Skip to content

Commit

Permalink
stubtest: various improvements (python#8502)
Browse files Browse the repository at this point in the history
* stubtest: refine verify_signature

If runtime takes a keyword-only argument, we now unconditionally check
that the stub also does. This leads to good outcomes on typeshed.
Also further clarify the comments.

* stubtest: ignore "this", another annoying module

* stubtest: support using regexes in whitelist

* stubtest: add --ignore-unused-whitelist

* stubtest: handle name mangling
  • Loading branch information
hauntsaninja authored Mar 7, 2020
1 parent f2ead85 commit 0a05e61
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 21 deletions.
68 changes: 50 additions & 18 deletions mypy/stubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import enum
import importlib
import inspect
import re
import sys
import types
import warnings
Expand Down Expand Up @@ -240,9 +241,12 @@ def verify_typeinfo(
to_check.update(m for m in cast(Any, vars)(runtime) if not m.startswith("_"))

for entry in sorted(to_check):
mangled_entry = entry
if entry.startswith("__") and not entry.endswith("__"):
mangled_entry = "_{}{}".format(stub.name, entry)
yield from verify(
next((t.names[entry].node for t in stub.mro if entry in t.names), MISSING),
getattr(runtime, entry, MISSING),
getattr(runtime, mangled_entry, MISSING),
object_path + [entry],
)

Expand All @@ -266,7 +270,13 @@ def _verify_static_class_methods(
# Look the object up statically, to avoid binding by the descriptor protocol
static_runtime = importlib.import_module(object_path[0])
for entry in object_path[1:]:
static_runtime = inspect.getattr_static(static_runtime, entry)
try:
static_runtime = inspect.getattr_static(static_runtime, entry)
except AttributeError:
# This can happen with mangled names, ignore for now.
# TODO: pass more information about ancestors of nodes/objects to verify, so we don't
# have to do this hacky lookup. Would be useful in a couple other places too.
return

if isinstance(static_runtime, classmethod) and not stub.is_class:
yield "runtime is a classmethod but stub is not"
Expand Down Expand Up @@ -582,21 +592,24 @@ def _verify_signature(

# Check unmatched keyword-only args
if runtime.varkw is None or not set(runtime.kwonly).issubset(set(stub.kwonly)):
# There are cases where the stub exhaustively lists out the extra parameters the function
# would take through *kwargs. Hence, a) we only check if the runtime actually takes those
# parameters when the above condition holds and b) below, we don't enforce that the stub
# takes *kwargs, since runtime logic may prevent additional arguments from actually being
# accepted.
for arg in sorted(set(stub.kwonly) - set(runtime.kwonly)):
yield 'runtime does not have argument "{}"'.format(arg)
if stub.varkw is None or not set(stub.kwonly).issubset(set(runtime.kwonly)):
for arg in sorted(set(runtime.kwonly) - set(stub.kwonly)):
if arg in set(stub_arg.variable.name for stub_arg in stub.pos):
# Don't report this if we've reported it before
if len(stub.pos) > len(runtime.pos) and runtime.varpos is not None:
yield 'stub argument "{}" is not keyword-only'.format(arg)
else:
yield 'stub does not have argument "{}"'.format(arg)
for arg in sorted(set(runtime.kwonly) - set(stub.kwonly)):
if arg in set(stub_arg.variable.name for stub_arg in stub.pos):
# Don't report this if we've reported it before
if len(stub.pos) > len(runtime.pos) and runtime.varpos is not None:
yield 'stub argument "{}" is not keyword-only'.format(arg)
else:
yield 'stub does not have argument "{}"'.format(arg)

# Checks involving **kwargs
if stub.varkw is None and runtime.varkw is not None:
# There are cases where the stub exhaustively lists out the extra parameters the function
# would take through **kwargs, so we don't enforce that the stub takes **kwargs.
# As mentioned above, don't enforce that the stub takes **kwargs.
# Also check against positional parameters, to avoid a nitpicky message when an argument
# isn't marked as keyword-only
stub_pos_names = set(stub_arg.variable.name for stub_arg in stub.pos)
Expand Down Expand Up @@ -1016,6 +1029,7 @@ def test_stubs(args: argparse.Namespace) -> int:
for whitelist_file in args.whitelist
for entry in get_whitelist_entries(whitelist_file)
}
whitelist_regexes = {entry: re.compile(entry) for entry in whitelist}

# If we need to generate a whitelist, we store Error.object_desc for each error here.
generated_whitelist = set()
Expand All @@ -1024,7 +1038,8 @@ def test_stubs(args: argparse.Namespace) -> int:
if args.check_typeshed:
assert not args.modules, "Cannot pass both --check-typeshed and a list of modules"
modules = get_typeshed_stdlib_modules(args.custom_typeshed_dir)
modules.remove("antigravity") # it's super annoying
annoying_modules = {"antigravity", "this"}
modules = [m for m in modules if m not in annoying_modules]

assert modules, "No modules to check"

Expand All @@ -1048,6 +1063,14 @@ def test_stubs(args: argparse.Namespace) -> int:
if error.object_desc in whitelist:
whitelist[error.object_desc] = True
continue
is_whitelisted = False
for w in whitelist:
if whitelist_regexes[w].fullmatch(error.object_desc):
whitelist[w] = True
is_whitelisted = True
break
if is_whitelisted:
continue

# We have errors, so change exit code, and output whatever necessary
exit_code = 1
Expand All @@ -1057,10 +1080,13 @@ def test_stubs(args: argparse.Namespace) -> int:
print(error.get_description(concise=args.concise))

# Print unused whitelist entries
for w in whitelist:
if not whitelist[w]:
exit_code = 1
print("note: unused whitelist entry {}".format(w))
if not args.ignore_unused_whitelist:
for w in whitelist:
# Don't consider an entry unused if it regex-matches the empty string
# This allows us to whitelist errors that don't manifest at all on some systems
if not whitelist[w] and not whitelist_regexes[w].fullmatch(""):
exit_code = 1
print("note: unused whitelist entry {}".format(w))

# Print the generated whitelist
if args.generate_whitelist:
Expand Down Expand Up @@ -1100,14 +1126,20 @@ def parse_options(args: List[str]) -> argparse.Namespace:
default=[],
help=(
"Use file as a whitelist. Can be passed multiple times to combine multiple "
"whitelists. Whitelist can be created with --generate-whitelist"
"whitelists. Whitelists can be created with --generate-whitelist"
),
)
parser.add_argument(
"--generate-whitelist",
action="store_true",
help="Print a whitelist (to stdout) to be used with --whitelist",
)
parser.add_argument(
"--ignore-unused-whitelist",
action="store_true",
help="Ignore unused whitelist entries",
)

return parser.parse_args(args)


Expand Down
62 changes: 59 additions & 3 deletions mypy/test/teststubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,11 @@ def test_varargs_varkwargs(self) -> Iterator[Case]:
runtime="def k5(a, *, b, c, **kwargs): pass",
error="k5",
)
yield Case(
stub="def k6(a, *, b, **kwargs) -> None: ...",
runtime="def k6(a, *, b, c, **kwargs): pass",
error="k6",
)

@collect_cases
def test_overload(self) -> Iterator[Case]:
Expand Down Expand Up @@ -567,6 +572,22 @@ def h(x: str): ...
yield Case(stub="", runtime="__all__ += ['y']\ny = 5", error="y")
yield Case(stub="", runtime="__all__ += ['g']\ndef g(): pass", error="g")

@collect_cases
def test_name_mangling(self) -> Iterator[Case]:
yield Case(
stub="""
class X:
def __mangle_good(self, text: str) -> None: ...
def __mangle_bad(self, number: int) -> None: ...
""",
runtime="""
class X:
def __mangle_good(self, text): pass
def __mangle_bad(self, text): pass
""",
error="X.__mangle_bad"
)


def remove_color_code(s: str) -> str:
return re.sub("\\x1b.*?m", "", s) # this works!
Expand Down Expand Up @@ -610,20 +631,55 @@ def test_ignore_flags(self) -> None:

def test_whitelist(self) -> None:
# Can't use this as a context because Windows
whitelist = tempfile.NamedTemporaryFile(mode="w", delete=False)
whitelist = tempfile.NamedTemporaryFile(mode="w+", delete=False)
try:
with whitelist:
whitelist.write("{}.bad\n# a comment".format(TEST_MODULE_NAME))
whitelist.write("{}.bad # comment\n# comment".format(TEST_MODULE_NAME))

output = run_stubtest(
stub="def bad(number: int, text: str) -> None: ...",
runtime="def bad(num, text) -> None: pass",
runtime="def bad(asdf, text): pass",
options=["--whitelist", whitelist.name],
)
assert not output

# test unused entry detection
output = run_stubtest(stub="", runtime="", options=["--whitelist", whitelist.name])
assert output == "note: unused whitelist entry {}.bad\n".format(TEST_MODULE_NAME)

output = run_stubtest(
stub="",
runtime="",
options=["--whitelist", whitelist.name, "--ignore-unused-whitelist"],
)
assert not output

# test regex matching
with open(whitelist.name, mode="w+") as f:
f.write("{}.b.*\n".format(TEST_MODULE_NAME))
f.write("(unused_missing)?\n")
f.write("unused.*\n")

output = run_stubtest(
stub=textwrap.dedent(
"""
def good() -> None: ...
def bad(number: int) -> None: ...
def also_bad(number: int) -> None: ...
""".lstrip("\n")
),
runtime=textwrap.dedent(
"""
def good(): pass
def bad(asdf): pass
def also_bad(asdf): pass
""".lstrip("\n")
),
options=["--whitelist", whitelist.name, "--generate-whitelist"],
)
assert output == "note: unused whitelist entry unused.*\n{}.also_bad\n".format(
TEST_MODULE_NAME
)
finally:
os.unlink(whitelist.name)

Expand Down

0 comments on commit 0a05e61

Please sign in to comment.