making changes directly will be easier than code review

Author: hauntsaninja

mypy/messages.py

@@ -42,7 +42,7 @@
 )
 from mypy.sametypes import is_same_type
 from mypy.typeops import separate_union_literals
-from mypy.util import unmangle, plural_s as plural_s
+from mypy.util import unmangle, plural_s
 from mypy.errorcodes import ErrorCode
 from mypy import message_registry, errorcodes as codes
 

mypy/stubtest.py

@@ -9,10 +9,12 @@
 import enum
 import importlib
 import inspect
+import os
 import re
 import sys
 import types
 import warnings
+from contextlib import redirect_stdout, redirect_stderr
 from functools import singledispatch
 from pathlib import Path
 from typing import Any, Dict, Generic, Iterator, List, Optional, Tuple, TypeVar, Union, cast
@@ -27,7 +29,7 @@
 from mypy import nodes
 from mypy.config_parser import parse_config_file
 from mypy.options import Options
-from mypy.util import FancyFormatter, bytes_to_human_readable_repr, is_dunder
+from mypy.util import FancyFormatter, bytes_to_human_readable_repr, plural_s, is_dunder
 
 
 class Missing:
@@ -51,6 +53,10 @@ def _style(message: str, **kwargs: Any) -> str:
     return _formatter.style(message, **kwargs)
 
 
+class StubtestFailure(Exception):
+    pass
+
+
 class Error:
     def __init__(
         self,
@@ -161,19 +167,20 @@ def test_module(module_name: str) -> Iterator[Error]:
     """
     stub = get_stub(module_name)
     if stub is None:
-        yield Error([module_name], "failed to find stubs", MISSING, None)
+        yield Error([module_name], "failed to find stubs", MISSING, None, runtime_desc="N/A")
         return
 
     try:
-        with warnings.catch_warnings():
-            warnings.simplefilter("ignore")
-            runtime = importlib.import_module(module_name)
-            # Also run the equivalent of `from module import *`
-            # This could have the additional effect of loading not-yet-loaded submodules
-            # mentioned in __all__
-            __import__(module_name, fromlist=["*"])
+        with open(os.devnull, "w") as devnull:
+            with warnings.catch_warnings(), redirect_stdout(devnull), redirect_stderr(devnull):
+                warnings.simplefilter("ignore")
+                runtime = importlib.import_module(module_name)
+                # Also run the equivalent of `from module import *`
+                # This could have the additional effect of loading not-yet-loaded submodules
+                # mentioned in __all__
+                __import__(module_name, fromlist=["*"])
     except Exception as e:
-        yield Error([module_name], f"failed to import: {e}", stub, MISSING)
+        yield Error([module_name], f"failed to import, {type(e).__name__}: {e}", stub, MISSING)
         return
 
     with warnings.catch_warnings():
@@ -918,7 +925,11 @@ def apply_decorator_to_funcitem(
         ) or decorator.fullname in mypy.types.OVERLOAD_NAMES:
             return func
         if decorator.fullname == "builtins.classmethod":
-            assert func.arguments[0].variable.name in ("cls", "metacls")
+            if func.arguments[0].variable.name not in ("cls", "mcs", "metacls"):
+                raise StubtestFailure(
+                    f"unexpected class argument name {func.arguments[0].variable.name!r} "
+                    f"in {dec.fullname}"
+                )
             # FuncItem is written so that copy.copy() actually works, even when compiled
             ret = copy.copy(func)
             # Remove the cls argument, since it's not present in inspect.signature of classmethods
@@ -1251,20 +1262,9 @@ def build_stubs(modules: List[str], options: Options, find_submodules: bool = Fa
     try:
         res = mypy.build.build(sources=sources, options=options)
     except mypy.errors.CompileError as e:
-        output = [
-            _style("error: ", color="red", bold=True),
-            "not checking stubs due to failed mypy compile:\n",
-            str(e),
-        ]
-        print("".join(output))
-        raise RuntimeError from e
+        raise StubtestFailure(f"failed mypy compile:\n{e}") from e
     if res.errors:
-        output = [
-            _style("error: ", color="red", bold=True),
-            "not checking stubs due to mypy build errors:\n",
-        ]
-        print("".join(output) + "\n".join(res.errors))
-        raise RuntimeError
+        raise StubtestFailure("mypy build errors:\n" + "\n".join(res.errors))
 
     global _all_stubs
     _all_stubs = res.files
@@ -1329,7 +1329,21 @@ def strip_comments(s: str) -> str:
                 yield entry
 
 
-def test_stubs(args: argparse.Namespace, use_builtins_fixtures: bool = False) -> int:
+class _Arguments:
+    modules: List[str]
+    concise: bool
+    ignore_missing_stub: bool
+    ignore_positional_only: bool
+    allowlist: List[str]
+    generate_allowlist: bool
+    ignore_unused_allowlist: bool
+    mypy_config_file: str
+    custom_typeshed_dir: str
+    check_typeshed: bool
+    version: str
+
+
+def test_stubs(args: _Arguments, use_builtins_fixtures: bool = False) -> int:
     """This is stubtest! It's time to test the stubs!"""
     # Load the allowlist. This is a series of strings corresponding to Error.object_desc
     # Values in the dict will store whether we used the allowlist entry or not.
@@ -1345,13 +1359,23 @@ def test_stubs(args: argparse.Namespace, use_builtins_fixtures: bool = False) ->
 
     modules = args.modules
     if args.check_typeshed:
-        assert not args.modules, "Cannot pass both --check-typeshed and a list of modules"
+        if args.modules:
+            print(
+                _style("error:", color="red", bold=True),
+                "cannot pass both --check-typeshed and a list of modules",
+            )
+            return 1
         modules = get_typeshed_stdlib_modules(args.custom_typeshed_dir)
         # typeshed added a stub for __main__, but that causes stubtest to check itself
         annoying_modules = {"antigravity", "this", "__main__"}
         modules = [m for m in modules if m not in annoying_modules]
 
-    assert modules, "No modules to check"
+    if not modules:
+        print(
+            _style("error:", color="red", bold=True),
+            "no modules to check",
+        )
+        return 1
 
     options = Options()
     options.incremental = False
@@ -1366,10 +1390,15 @@ def set_strict_flags() -> None:  # not needed yet
 
     try:
         modules = build_stubs(modules, options, find_submodules=not args.check_typeshed)
-    except RuntimeError:
+    except StubtestFailure as stubtest_failure:
+        print(
+            _style("error:", color="red", bold=True),
+            f"not checking stubs due to {stubtest_failure}",
+        )
         return 1
 
     exit_code = 0
+    error_count = 0
     for module in modules:
         for error in test_module(module):
             # Filter errors
@@ -1395,6 +1424,7 @@ def set_strict_flags() -> None:  # not needed yet
                 generated_allowlist.add(error.object_desc)
                 continue
             print(error.get_description(concise=args.concise))
+            error_count += 1
 
     # Print unused allowlist entries
     if not args.ignore_unused_allowlist:
@@ -1403,18 +1433,35 @@ def set_strict_flags() -> None:  # not needed yet
             # This lets us allowlist errors that don't manifest at all on some systems
             if not allowlist[w] and not allowlist_regexes[w].fullmatch(""):
                 exit_code = 1
+                error_count += 1
                 print(f"note: unused allowlist entry {w}")
 
     # Print the generated allowlist
     if args.generate_allowlist:
         for e in sorted(generated_allowlist):
             print(e)
         exit_code = 0
+    elif not args.concise:
+        if error_count:
+            print(
+                _style(
+                    f"Found {error_count} error{plural_s(error_count)}"
+                    f" (checked {len(modules)} module{plural_s(modules)})",
+                    color="red", bold=True
+                )
+            )
+        else:
+            print(
+                _style(
+                    f"Success: no issues found in {len(modules)} module{plural_s(modules)}",
+                    color="green", bold=True
+                )
+            )
 
     return exit_code
 
 
-def parse_options(args: List[str]) -> argparse.Namespace:
+def parse_options(args: List[str]) -> _Arguments:
     parser = argparse.ArgumentParser(
         description="Compares stubs to objects introspected from the runtime."
     )
@@ -1476,7 +1523,7 @@ def parse_options(args: List[str]) -> argparse.Namespace:
         "--version", action="version", version="%(prog)s " + mypy.version.__version__
     )
 
-    return parser.parse_args(args)
+    return parser.parse_args(args, namespace=_Arguments())
 
 
 def main() -> int:

mypy/test/teststubtest.py

@@ -806,7 +806,7 @@ def test_missing_no_runtime_all(self) -> Iterator[Case]:
     @collect_cases
     def test_non_public_1(self) -> Iterator[Case]:
         yield Case(
-            stub="__all__: list[str]", runtime="", error="test_module.__all__"
+            stub="__all__: list[str]", runtime="", error=f"{TEST_MODULE_NAME}.__all__"
         )  # dummy case
         yield Case(stub="_f: int", runtime="def _f(): ...", error="_f")
 
@@ -1055,9 +1055,10 @@ def test_output(self) -> None:
             options=[],
         )
         expected = (
-            'error: {0}.bad is inconsistent, stub argument "number" differs from runtime '
+            f'error: {TEST_MODULE_NAME}.bad is inconsistent, stub argument "number" differs from runtime '
             'argument "num"\nStub: at line 1\ndef (number: builtins.int, text: builtins.str)\n'
-            "Runtime: at line 1 in file {0}.py\ndef (num, text)\n\n".format(TEST_MODULE_NAME)
+            f"Runtime: at line 1 in file {TEST_MODULE_NAME}.py\ndef (num, text)\n\n"
+            'Found 1 error (checked 1 module)\n'
         )
         assert remove_color_code(output) == expected
 
@@ -1076,17 +1077,17 @@ def test_ignore_flags(self) -> None:
         output = run_stubtest(
             stub="", runtime="__all__ = ['f']\ndef f(): pass", options=["--ignore-missing-stub"]
         )
-        assert not output
+        assert output == 'Success: no issues found in 1 module\n'
 
         output = run_stubtest(
             stub="", runtime="def f(): pass", options=["--ignore-missing-stub"]
         )
-        assert not output
+        assert output == 'Success: no issues found in 1 module\n'
 
         output = run_stubtest(
             stub="def f(__a): ...", runtime="def f(a): pass", options=["--ignore-positional-only"]
         )
-        assert not output
+        assert output == 'Success: no issues found in 1 module\n'
 
     def test_allowlist(self) -> None:
         # Can't use this as a context because Windows
@@ -1100,18 +1101,21 @@ def test_allowlist(self) -> None:
                 runtime="def bad(asdf, text): pass",
                 options=["--allowlist", allowlist.name],
             )
-            assert not output
+            assert output == 'Success: no issues found in 1 module\n'
 
             # test unused entry detection
             output = run_stubtest(stub="", runtime="", options=["--allowlist", allowlist.name])
-            assert output == f"note: unused allowlist entry {TEST_MODULE_NAME}.bad\n"
+            assert output == (
+                f"note: unused allowlist entry {TEST_MODULE_NAME}.bad\n"
+                "Found 1 error (checked 1 module)\n"
+            )
 
             output = run_stubtest(
                 stub="",
                 runtime="",
                 options=["--allowlist", allowlist.name, "--ignore-unused-allowlist"],
             )
-            assert not output
+            assert output == 'Success: no issues found in 1 module\n'
 
             # test regex matching
             with open(allowlist.name, mode="w+") as f:
@@ -1136,8 +1140,9 @@ def also_bad(asdf): pass
                 ),
                 options=["--allowlist", allowlist.name, "--generate-allowlist"],
             )
-            assert output == "note: unused allowlist entry unused.*\n{}.also_bad\n".format(
-                TEST_MODULE_NAME
+            assert output == (
+                f"note: unused allowlist entry unused.*\n"
+                f"{TEST_MODULE_NAME}.also_bad\n"
             )
         finally:
             os.unlink(allowlist.name)
@@ -1193,8 +1198,23 @@ def test_config_file(self) -> None:
         )
         output = run_stubtest(stub=stub, runtime=runtime, options=[])
         assert remove_color_code(output) == (
-            "error: test_module.temp variable differs from runtime type Literal[5]\n"
+            f"error: {TEST_MODULE_NAME}.temp variable differs from runtime type Literal[5]\n"
             "Stub: at line 2\n_decimal.Decimal\nRuntime:\n5\n\n"
+            "Found 1 error (checked 1 module)\n"
         )
         output = run_stubtest(stub=stub, runtime=runtime, options=[], config_file=config_file)
-        assert output == ""
+        assert output == "Success: no issues found in 1 module\n"
+
+    def test_no_modules(self) -> None:
+        output = io.StringIO()
+        with contextlib.redirect_stdout(output):
+            test_stubs(parse_options([]))
+        assert remove_color_code(output.getvalue()) == "error: no modules to check\n"
+
+    def test_module_and_typeshed(self) -> None:
+        output = io.StringIO()
+        with contextlib.redirect_stdout(output):
+            test_stubs(parse_options(["--check-typeshed", "some_module"]))
+        assert remove_color_code(output.getvalue()) == (
+            "error: cannot pass both --check-typeshed and a list of modules\n"
+        )