Colorize daemon output and add summary line (#7441)

Fixes #7438

This PR makes the daemon behave the same way as the normal run (i.e. use colorized output and write a summary line). I also fix a minor bug that the summary line was colorized even if `--no-color-output` was passed.

The idea is quite simple, client passes to the server info about whether we are in tty and if yes server returns nicely formatted output. I tried running various commands on Linux and piping it through `grep`.

Author: ilevkivskyi

mypy/dmypy/client.py

@@ -466,6 +466,9 @@ def request(status_file: str, command: str, *, timeout: Optional[int] = None,
     response = {}  # type: Dict[str, str]
     args = dict(kwds)
     args['command'] = command
+    # Tell the server whether this request was initiated from a human-facing terminal,
+    # so that it can format the type checking output accordingly.
+    args['is_tty'] = sys.stdout.isatty()
     bdata = json.dumps(args).encode('utf8')
     _, name = get_status(status_file)
     try:

mypy/dmypy_server.py

@@ -16,7 +16,7 @@
 import traceback
 from contextlib import redirect_stderr, redirect_stdout
 
-from typing import AbstractSet, Any, Callable, Dict, List, Mapping, Optional, Sequence, Tuple
+from typing import AbstractSet, Any, Callable, Dict, List, Optional, Sequence, Tuple
 from typing_extensions import Final
 
 import mypy.build
@@ -33,6 +33,7 @@
 from mypy.suggestions import SuggestionFailure, SuggestionEngine
 from mypy.typestate import reset_global_state
 from mypy.version import __version__
+from mypy.util import FancyFormatter, count_stats
 
 MEM_PROFILE = False  # type: Final  # If True, dump memory profile after initialization
 
@@ -188,6 +189,10 @@ def __init__(self, options: Options,
         options.local_partial_types = True
         self.status_file = status_file
 
+        # Since the object is created in the parent process we can check
+        # the output terminal options here.
+        self.formatter = FancyFormatter(sys.stdout, sys.stderr, options.show_error_codes)
+
     def _response_metadata(self) -> Dict[str, str]:
         py_version = '{}_{}'.format(self.options.python_version[0], self.options.python_version[1])
         return {
@@ -248,13 +253,18 @@ def serve(self) -> None:
             if exc_info[0] and exc_info[0] is not SystemExit:
                 traceback.print_exception(*exc_info)
 
-    def run_command(self, command: str, data: Mapping[str, object]) -> Dict[str, object]:
+    def run_command(self, command: str, data: Dict[str, object]) -> Dict[str, object]:
         """Run a specific command from the registry."""
         key = 'cmd_' + command
         method = getattr(self.__class__, key, None)
         if method is None:
             return {'error': "Unrecognized command '%s'" % command}
         else:
+            if command not in {'check', 'recheck', 'run'}:
+                # Only the above commands use some error formatting.
+                del data['is_tty']
+            elif int(os.getenv('MYPY_FORCE_COLOR', '0')):
+                data['is_tty'] = True
             return method(self, **data)
 
     # Command functions (run in the server via RPC).
@@ -280,7 +290,7 @@ def cmd_stop(self) -> Dict[str, object]:
         os.unlink(self.status_file)
         return {}
 
-    def cmd_run(self, version: str, args: Sequence[str]) -> Dict[str, object]:
+    def cmd_run(self, version: str, args: Sequence[str], is_tty: bool) -> Dict[str, object]:
         """Check a list of files, triggering a restart if needed."""
         try:
             # Process options can exit on improper arguments, so we need to catch that and
@@ -313,17 +323,18 @@ def cmd_run(self, version: str, args: Sequence[str]) -> Dict[str, object]:
             return {'out': '', 'err': str(err), 'status': 2}
         except SystemExit as e:
             return {'out': stdout.getvalue(), 'err': stderr.getvalue(), 'status': e.code}
-        return self.check(sources)
+        return self.check(sources, is_tty)
 
-    def cmd_check(self, files: Sequence[str]) -> Dict[str, object]:
+    def cmd_check(self, files: Sequence[str], is_tty: bool) -> Dict[str, object]:
         """Check a list of files."""
         try:
             sources = create_source_list(files, self.options, self.fscache)
         except InvalidSourceList as err:
             return {'out': '', 'err': str(err), 'status': 2}
-        return self.check(sources)
+        return self.check(sources, is_tty)
 
     def cmd_recheck(self,
+                    is_tty: bool,
                     remove: Optional[List[str]] = None,
                     update: Optional[List[str]] = None) -> Dict[str, object]:
         """Check the same list of files we checked most recently.
@@ -349,17 +360,21 @@ def cmd_recheck(self,
         t1 = time.time()
         manager = self.fine_grained_manager.manager
         manager.log("fine-grained increment: cmd_recheck: {:.3f}s".format(t1 - t0))
-        res = self.fine_grained_increment(sources, remove, update)
+        res = self.fine_grained_increment(sources, is_tty, remove, update)
         self.fscache.flush()
         self.update_stats(res)
         return res
 
-    def check(self, sources: List[BuildSource]) -> Dict[str, Any]:
-        """Check using fine-grained incremental mode."""
+    def check(self, sources: List[BuildSource], is_tty: bool) -> Dict[str, Any]:
+        """Check using fine-grained incremental mode.
+
+        If is_tty is True format the output nicely with colors and summary line
+        (unless disabled in self.options).
+        """
         if not self.fine_grained_manager:
-            res = self.initialize_fine_grained(sources)
+            res = self.initialize_fine_grained(sources, is_tty)
         else:
-            res = self.fine_grained_increment(sources)
+            res = self.fine_grained_increment(sources, is_tty)
         self.fscache.flush()
         self.update_stats(res)
         return res
@@ -371,7 +386,8 @@ def update_stats(self, res: Dict[str, Any]) -> None:
             res['stats'] = manager.stats
             manager.stats = {}
 
-    def initialize_fine_grained(self, sources: List[BuildSource]) -> Dict[str, Any]:
+    def initialize_fine_grained(self, sources: List[BuildSource],
+                                is_tty: bool) -> Dict[str, Any]:
         self.fswatcher = FileSystemWatcher(self.fscache)
         t0 = time.time()
         self.update_sources(sources)
@@ -433,10 +449,12 @@ def initialize_fine_grained(self, sources: List[BuildSource]) -> Dict[str, Any]:
             print_memory_profile(run_gc=False)
 
         status = 1 if messages else 0
+        messages = self.pretty_messages(messages, len(sources), is_tty)
         return {'out': ''.join(s + '\n' for s in messages), 'err': '', 'status': status}
 
     def fine_grained_increment(self,
                                sources: List[BuildSource],
+                               is_tty: bool,
                                remove: Optional[List[str]] = None,
                                update: Optional[List[str]] = None,
                                ) -> Dict[str, Any]:
@@ -466,8 +484,28 @@ def fine_grained_increment(self,
 
         status = 1 if messages else 0
         self.previous_sources = sources
+        messages = self.pretty_messages(messages, len(sources), is_tty)
         return {'out': ''.join(s + '\n' for s in messages), 'err': '', 'status': status}
 
+    def pretty_messages(self, messages: List[str], n_sources: int,
+                        is_tty: bool = False) -> List[str]:
+        use_color = self.options.color_output and is_tty
+        if self.options.error_summary:
+            summary = None  # type: Optional[str]
+            if messages:
+                n_errors, n_files = count_stats(messages)
+                if n_errors:
+                    summary = self.formatter.format_error(n_errors, n_files, n_sources,
+                                                          use_color)
+            else:
+                summary = self.formatter.format_success(n_sources, use_color)
+            if summary:
+                # Create new list to avoid appending multiple summaries on successive runs.
+                messages = messages + [summary]
+        if use_color:
+            messages = [self.formatter.colorize(m) for m in messages]
+        return messages
+
     def update_sources(self, sources: List[BuildSource]) -> None:
         paths = [source.path for source in sources if source.path is not None]
         self.fswatcher.add_watched_paths(paths)

mypy/main.py

@@ -112,9 +112,11 @@ def flush_errors(new_messages: List[str], serious: bool) -> None:
         if messages:
             n_errors, n_files = util.count_stats(messages)
             if n_errors:
-                stdout.write(formatter.format_error(n_errors, n_files, len(sources)) + '\n')
+                stdout.write(formatter.format_error(n_errors, n_files, len(sources),
+                                                    options.color_output) + '\n')
         else:
-            stdout.write(formatter.format_success(len(sources)) + '\n')
+            stdout.write(formatter.format_success(len(sources),
+                                                  options.color_output) + '\n')
         stdout.flush()
     if options.fast_exit:
         # Exit without freeing objects -- it's faster.

mypy/test/testfinegrained.py

@@ -181,6 +181,7 @@ def get_options(self,
         options.incremental = True
         options.use_builtins_fixtures = True
         options.show_traceback = True
+        options.error_summary = False
         options.fine_grained_incremental = not build_cache
         options.use_fine_grained_cache = self.use_cache and not build_cache
         options.cache_fine_grained = self.use_cache
@@ -196,7 +197,7 @@ def get_options(self,
         return options
 
     def run_check(self, server: Server, sources: List[BuildSource]) -> List[str]:
-        response = server.check(sources)
+        response = server.check(sources, is_tty=False)
         out = cast(str, response['out'] or response['err'])
         return out.splitlines()
 

mypy/util.py

@@ -427,14 +427,19 @@ def underline_link(self, note: str) -> str:
                 self.style(note[start:end], 'none', underline=True) +
                 note[end:])
 
-    def format_success(self, n_sources: int) -> str:
-        return self.style('Success: no issues found in {}'
-                          ' source file{}'.format(n_sources, 's' if n_sources != 1 else ''),
-                          'green', bold=True)
+    def format_success(self, n_sources: int, use_color: bool = True) -> str:
+        msg = 'Success: no issues found in {}' \
+              ' source file{}'.format(n_sources, 's' if n_sources != 1 else '')
+        if not use_color:
+            return msg
+        return self.style(msg, 'green', bold=True)
 
-    def format_error(self, n_errors: int, n_files: int, n_sources: int) -> str:
+    def format_error(self, n_errors: int, n_files: int, n_sources: int,
+                     use_color: bool = True) -> str:
         msg = 'Found {} error{} in {} file{}' \
               ' (checked {} source file{})'.format(n_errors, 's' if n_errors != 1 else '',
                                                    n_files, 's' if n_files != 1 else '',
                                                    n_sources, 's' if n_sources != 1 else '')
+        if not use_color:
+            return msg
         return self.style(msg, 'red', bold=True)

test-data/unit/daemon.test

@@ -11,7 +11,9 @@ Daemon stopped
 $ dmypy start -- --follow-imports=error
 Daemon started
 $ dmypy check -- foo.py
+Success: no issues found in 1 source file
 $ dmypy recheck
+Success: no issues found in 1 source file
 $ dmypy stop
 Daemon stopped
 [file foo.py]
@@ -20,6 +22,7 @@ def f(): pass
 [case testDaemonRun]
 $ dmypy run -- foo.py --follow-imports=error
 Daemon started
+Success: no issues found in 1 source file
 $ dmypy stop
 Daemon stopped
 [file foo.py]
@@ -28,7 +31,9 @@ def f(): pass
 [case testDaemonRunRestart]
 $ dmypy run -- foo.py --follow-imports=error
 Daemon started
+Success: no issues found in 1 source file
 $ dmypy run -- foo.py --follow-imports=error
+Success: no issues found in 1 source file
 $ {python} -c "print('[mypy]')" >mypy.ini
 $ {python} -c "print('disallow_untyped_defs = True')" >>mypy.ini
 $ dmypy run -- foo.py --follow-imports=error
@@ -37,19 +42,21 @@ Daemon stopped
 Daemon started
 foo.py:1: error: Function is missing a return type annotation
 foo.py:1: note: Use "-> None" if function does not return a value
+Found 1 error in 1 file (checked 1 source file)
 == Return code: 1
 $ {python} -c "print('def f() -> None: pass')" >foo.py
 $ dmypy run -- foo.py --follow-imports=error
+Success: no issues found in 1 source file
 $ dmypy stop
 Daemon stopped
 [file foo.py]
 def f(): pass
 
 [case testDaemonRunRestartPluginVersion]
-$ dmypy run -- foo.py
+$ dmypy run -- foo.py --no-error-summary
 Daemon started
 $ {python} -c "print(' ')" >> plug.py
-$ dmypy run -- foo.py
+$ dmypy run -- foo.py --no-error-summary
 Restarting: plugins changed
 Daemon stopped
 Daemon started
@@ -79,22 +86,22 @@ No status file found
 $ dmypy recheck
 No status file found
 == Return code: 2
-$ dmypy start --  --follow-imports=error
+$ dmypy start --  --follow-imports=error --no-error-summary
 Daemon started
 $ dmypy status
 Daemon is up and running
 $ dmypy start
 Daemon is still alive
 == Return code: 2
-$ dmypy restart --  --follow-imports=error
+$ dmypy restart --  --follow-imports=error --no-error-summary
 Daemon stopped
 Daemon started
 $ dmypy stop
 Daemon stopped
 $ dmypy status
 No status file found
 == Return code: 2
-$ dmypy restart --  --follow-imports=error
+$ dmypy restart --  --follow-imports=error --no-error-summary
 Daemon started
 $ dmypy recheck
 Command 'recheck' is only valid after a 'check' command
@@ -106,7 +113,7 @@ Daemon has died
 == Return code: 2
 
 [case testDaemonRecheck]
-$ dmypy start -- --follow-imports=error
+$ dmypy start -- --follow-imports=error --no-error-summary
 Daemon started
 $ dmypy check foo.py bar.py
 $ dmypy recheck
@@ -153,6 +160,7 @@ $ {python} -c "import time;time.sleep(1)"
 $ {python} -c "print('x=1')" >bar.py
 $ dmypy run -- foo.py bar.py --follow-imports=error --use-fine-grained-cache --no-sqlite-cache --python-version=3.6
 Daemon started
+Success: no issues found in 2 source files
 $ dmypy status --fswatcher-dump-file test.json
 Daemon is up and running
 $ dmypy stop
@@ -165,6 +173,7 @@ $ {python} -c "print('lol')" >foo.py
 $ dmypy run --log-file=log -- foo.py bar.py --follow-imports=error --use-fine-grained-cache --no-sqlite-cache --python-version=3.6 --quickstart-file test.json
 Daemon started
 foo.py:1: error: Name 'lol' is not defined
+Found 1 error in 1 file (checked 2 source files)
 == Return code: 1
 -- make sure no errors made it to the log file
 $ {python} -c "import sys; sys.stdout.write(open('log').read())"
@@ -173,7 +182,7 @@ $ {python} -c "import sys; sys.stdout.write(open('log').read())"
 $ {python} -c "x = open('.mypy_cache/3.6/bar.meta.json').read(); y = open('asdf.json').read(); assert x == y"
 
 [case testDaemonSuggest]
-$ dmypy start --log-file log.txt -- --follow-imports=error
+$ dmypy start --log-file log.txt -- --follow-imports=error --no-error-summary
 Daemon started
 $ dmypy suggest foo:foo
 Command 'suggest' is only valid after a 'check' command