Skip to content

Commit

Permalink
Add a new pytype flag, --check-variable-types.
Browse files Browse the repository at this point in the history
When this mode is enabled, pytype checks variable values against annotations.
Implemented by calling bad_matches every time an annotated value is stored and
using annotated_locals to look up annotations.

Since dataclasses is part of the stdlib, I modified the matcher to handle
dataclasses.field so dataclass type-checking can (mostly) be handled like
normal variable checking. (None is still special.) I kept all logic for the
third party attrs library in its overlay.

I enabled check_variable_types by default for our tests, to help avoid any
behavior regressions during the (likely lengthy) release process. The attrs and
dataclasses tests have check_variable_types disabled in order to test that the
checking for these modules continues to work as expected even without it.

I punted on a couple of things:
* Class attributes are still not type-checked.
* In some cases, annotations should probably replace inferred values. For
  example, in:
    class Foo:
      v: float
      def __init__(self):
        self.v = 0
  Foo.v should get a type of float, not int, in the type stub. Or in:
    x: Any = None
    for ...:
      x = <some complicated thing pytype infers the wrong type for>
  x should be treated as having type Any, not Union[Any, <some wrong thing>].

This change increases the type-checking time for my sample file,
blobstore2/access/signature/testdata/gen_v4_sig_from_canonical_request.py, from
8s to 10s.

For #323.

PiperOrigin-RevId: 311001938
  • Loading branch information
rchen152 committed May 11, 2020
1 parent 69cf1ad commit 835cf17
Show file tree
Hide file tree
Showing 17 changed files with 145 additions and 54 deletions.
8 changes: 6 additions & 2 deletions pytype/abstract_utils.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# Lint as: python3
"""Utilities for abstract.py."""

import collections
import hashlib
import logging
from typing import Tuple, Union

from pytype import compat
from pytype import datatypes
Expand Down Expand Up @@ -605,12 +607,14 @@ def check_classes(var, check):
v.cls.isinstance_Class() and check(v.cls) for v in var.data if v.cls)


def match_type_container(typ, container_type_name):
def match_type_container(typ, container_type_name: Union[str, Tuple[str, ...]]):
"""Unpack the type parameter from ContainerType[T]."""
if typ is None:
return None
if isinstance(container_type_name, str):
container_type_name = (container_type_name,)
if not (typ.isinstance_ParameterizedClass() and
typ.full_name == container_type_name):
typ.full_name in container_type_name):
return None
param = typ.get_formal_type_parameter(T)
return param
Expand Down
4 changes: 4 additions & 0 deletions pytype/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ def add_basic_options(o):
"--strict-import", action="store_true",
dest="strict_import", default=False,
help="Experimental: Only load submodules that are explicitly imported.")
o.add_argument(
"--check-variable-types", action="store_true",
dest="check_variable_types", default=False,
help="Experimental: Check variable values against their annotations.")
o.add_argument(
"--precise-return", action="store_true", dest="precise_return",
default=False, help=("Experimental: Infer precise return types even for "
Expand Down
10 changes: 10 additions & 0 deletions pytype/matcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from pytype import mixin
from pytype import special_builtins
from pytype import utils
from pytype.overlays import dataclass_overlay
from pytype.overlays import typing_overlay
from pytype.pytd import pep484
from pytype.pytd import pytd
Expand Down Expand Up @@ -102,6 +103,11 @@ def bad_matches(self, var, other_type, node):
A list of all the views of var that didn't match.
"""
bad = []
if (var.data == [self.vm.convert.unsolvable] or
other_type == self.vm.convert.unsolvable):
# An unsolvable matches everything. Since bad_matches doesn't need to
# compute substitutions, we can return immediately.
return bad
views = abstract_utils.get_views([var], node)
skip_future = None
while True:
Expand Down Expand Up @@ -397,6 +403,10 @@ def _match_type_against_type(self, left, other_type, subst, node, view):
if new_subst is not None:
return new_subst
return None
elif isinstance(left, dataclass_overlay.FieldInstance) and left.default:
default = self.vm.merge_values(left.default.data)
return self._match_type_against_type(
default, other_type, subst, node, view)
elif isinstance(left, abstract.SimpleAbstractValue):
return self._match_instance_against_type(
left, other_type, subst, node, view)
Expand Down
7 changes: 4 additions & 3 deletions pytype/overlays/attr_overlay.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,13 @@ def decorate(self, node, cls):
typ=typ,
init=orig.data[0].init,
default=orig.data[0].default)
self.check_default(node, attr.name, attr.typ, attr.default, local.stack,
allow_none=True)
self.vm.check_annotation_type_mismatch(
node, attr.name, attr.typ, attr.default, local.stack,
allow_none=True)
own_attrs.append(attr)
elif self.args[cls]["auto_attribs"]:
if not match_classvar(typ):
self.check_default(
self.vm.check_annotation_type_mismatch(
node, name, typ, orig, local.stack, allow_none=True)
attr = Attribute(name=name, typ=typ, init=True, default=orig)
if not orig:
Expand Down
23 changes: 0 additions & 23 deletions pytype/overlays/classgen.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,29 +179,6 @@ def get_base_class_attrs(self, cls, cls_attrs, metadata_key):
base_attrs.append(a)
return base_attrs

def check_default(self, node, name, typ, default, stack, allow_none=False):
"""Check that the type annotation and the default value are consistent.
Args:
node: node
name: variable name
typ: variable annotation
default: variable assignment or default value
stack: a frame stack for error reporting
allow_none: whether a default of None is allowed for any type
"""
if not typ or not default:
return
# Check for permitted uses of x: T = None
if (allow_none and
len(default.data) == 1 and
default.data[0].cls == self.vm.convert.none_type):
return
bad = self.vm.matcher.bad_matches(default, typ, node)
if bad:
binding = bad[0][default]
self.vm.errorlog.annotation_type_mismatch(stack, typ, binding, name)

def call(self, node, func, args):
"""Construct a decorator, and call it on the class."""
self.match_args(node, args)
Expand Down
9 changes: 7 additions & 2 deletions pytype/overlays/dataclass_overlay.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,13 @@ def decorate(self, node, cls):
else:
init = True

# Check that default matches the declared type
self.check_default(node, name, typ, orig, local.stack)
if (not self.vm.options.check_variable_types or
orig and orig.data == [self.vm.convert.none]):
# vm._apply_annotation mostly takes care of checking that the default
# matches the declared type. However, it allows None defaults, and
# dataclasses do not.
self.vm.check_annotation_type_mismatch(
node, name, typ, orig, local.stack, allow_none=False)

attr = classgen.Attribute(name=name, typ=typ, init=init, default=orig)
own_attrs.append(attr)
Expand Down
16 changes: 8 additions & 8 deletions pytype/tests/py2/test_six_overlay.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ def test_string_types(self):
b = [b]
c = '' # type: Text
if isinstance(c, six.string_types):
c = len(c)
d = len(c)
""")
self.assertTypesMatchPytd(
ty, """
from typing import List
six: module = ...
a: List[str] = ...
b: List[unicode] = ...
c: int = ...
self.assertTypesMatchPytd(ty, """
from typing import List, Union
six: module
a: List[str]
b: List[unicode]
c: Union[str, unicode]
d: int
""")


Expand Down
19 changes: 19 additions & 0 deletions pytype/tests/py3/test_attr.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# Lint as: python3
"""Tests for attrs library in attr_overlay.py."""

from pytype import file_utils
Expand All @@ -7,6 +8,12 @@
class TestAttrib(test_base.TargetPython3BasicTest):
"""Tests for attr.ib."""

def setUp(self):
super().setUp()
# Checking field defaults against their types should work even when general
# variable checking is disabled.
self.options.tweak(check_variable_types=False)

def test_factory_function(self):
ty = self.Infer("""
import attr
Expand All @@ -31,6 +38,12 @@ def __init__(self, x: CustomClass = ...) -> None: ...
class TestAttribPy3(test_base.TargetPython3FeatureTest):
"""Tests for attr.ib using PEP526 syntax."""

def setUp(self):
super().setUp()
# Checking field defaults against their types should work even when general
# variable checking is disabled.
self.options.tweak(check_variable_types=False)

def test_variable_annotations(self):
ty = self.Infer("""
import attr
Expand Down Expand Up @@ -151,6 +164,12 @@ class Bar(object):
class TestAttrs(test_base.TargetPython3FeatureTest):
"""Tests for attr.s."""

def setUp(self):
super().setUp()
# Checking field defaults against their types should work even when general
# variable checking is disabled.
self.options.tweak(check_variable_types=False)

def test_kw_only(self):
ty = self.Infer("""
import attr
Expand Down
7 changes: 7 additions & 0 deletions pytype/tests/py3/test_dataclasses.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# Lint as: python3
"""Tests for the dataclasses overlay."""

from pytype.tests import test_base
Expand All @@ -6,6 +7,12 @@
class TestDataclass(test_base.TargetPython3FeatureTest):
"""Tests for @dataclass."""

def setUp(self):
super().setUp()
# Checking field defaults against their types should work even when general
# variable checking is disabled.
self.options.tweak(check_variable_types=False)

def test_basic(self):
ty = self.Infer("""
import dataclasses
Expand Down
12 changes: 6 additions & 6 deletions pytype/tests/py3/test_six_overlay.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ def test_string_types(self):
a = [a]
b = '' # type: Text
if isinstance(b, six.string_types):
b = len(b)
c = len(b)
""")
self.assertTypesMatchPytd(
ty, """
self.assertTypesMatchPytd(ty, """
from typing import List
six: module = ...
a: List[str] = ...
b: int = ...
six: module
a: List[str]
b: str
c: int
""")

def test_integer_types(self):
Expand Down
6 changes: 4 additions & 2 deletions pytype/tests/py3/test_type_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,21 @@ def foo(x: int) -> float:
""")

def test_list_comprehension_comments(self):
ty = self.Infer("""
ty, errors = self.InferWithErrors("""
from typing import List
def f(x: str):
pass
def g(xs: List[str]) -> List[str]:
ys = [f(x) for x in xs] # type: List[str]
ys = [f(x) for x in xs] # type: List[str] # annotation-type-mismatch[e]
return ys
""")
self.assertTypesMatchPytd(ty, """
from typing import List
def f(x: str) -> None: ...
def g(xs: List[str]) -> List[str]: ...
""")
self.assertErrorRegexes(
errors, {"e": r"Annotation: List\[str\].*Assignment: List\[None\]"})


class Py3TypeCommentTest(test_base.TargetPython3FeatureTest):
Expand Down
16 changes: 12 additions & 4 deletions pytype/tests/py3/test_variable_annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,22 +117,24 @@ def test_shadow_none(self):
""")

def test_overwrite_annotation(self):
ty = self.Infer("""
ty, errors = self.InferWithErrors("""
x: int
x = ""
x = "" # annotation-type-mismatch[e]
""")
self.assertTypesMatchPytd(ty, "x: str")
self.assertErrorRegexes(errors, {"e": r"Annotation: int.*Assignment: str"})

def test_overwrite_annotation_in_class(self):
ty = self.Infer("""
ty, errors = self.InferWithErrors("""
class Foo:
x: int
x = ""
x = "" # annotation-type-mismatch[e]
""")
self.assertTypesMatchPytd(ty, """
class Foo:
x: str
""")
self.assertErrorRegexes(errors, {"e": r"Annotation: int.*Assignment: str"})

def test_class_variable_forward_reference(self):
ty = self.Infer("""
Expand Down Expand Up @@ -227,5 +229,11 @@ def f():
def f() -> Dict[str, Dict[str, bool]]: ...
""")

def test_none_or_ellipsis_assignment(self):
self.Check("""
v1: int = None
v2: str = ...
""")


test_base.main(globals(), __name__ == "__main__")
13 changes: 13 additions & 0 deletions pytype/tests/test_attr.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# Lint as: python3
"""Tests for attrs library in attr_overlay.py."""

from pytype.tests import test_base
Expand All @@ -6,6 +7,12 @@
class TestAttrib(test_base.TargetIndependentTest):
"""Tests for attr.ib."""

def setUp(self):
super().setUp()
# Checking field defaults against their types should work even when general
# variable checking is disabled.
self.options.tweak(check_variable_types=False)

def test_basic(self):
ty = self.Infer("""
import attr
Expand Down Expand Up @@ -647,6 +654,12 @@ def __init__(self, x = ...) -> None: ...
class TestAttrs(test_base.TargetIndependentTest):
"""Tests for attr.s."""

def setUp(self):
super().setUp()
# Checking field defaults against their types should work even when general
# variable checking is disabled.
self.options.tweak(check_variable_types=False)

def test_basic(self):
ty = self.Infer("""
import attr
Expand Down
3 changes: 2 additions & 1 deletion pytype/tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ def t(name): # pylint: disable=invalid-name

def setUp(self):
super(BaseTest, self).setUp()
self.options = config.Options.create(python_version=self.python_version)
self.options = config.Options.create(python_version=self.python_version,
check_variable_types=True)

@property
def loader(self):
Expand Down
8 changes: 5 additions & 3 deletions pytype/tests/test_type_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ class A(object):

def test_none_to_none_type(self):
ty = self.Infer("""
x = 0 # type: None
x = ... # type: None
""", deep=False)
self.assertTypesMatchPytd(ty, """
x = ... # type: None
Expand Down Expand Up @@ -581,21 +581,23 @@ class A(object):
""")

def test_list_comprehension_comments(self):
ty = self.Infer("""
ty, errors = self.InferWithErrors("""
from typing import List
def f(x):
# type: (str) -> None
pass
def g(xs):
# type: (List[str]) -> List[str]
ys = [f(x) for x in xs] # type: List[str]
ys = [f(x) for x in xs] # type: List[str] # annotation-type-mismatch[e]
return ys
""")
self.assertTypesMatchPytd(ty, """
from typing import List
def f(x: str) -> None: ...
def g(xs: List[str]) -> List[str]: ...
""")
self.assertErrorRegexes(
errors, {"e": r"Annotation: List\[str\].*Assignment: List\[None\]"})

def test_multiple_assignments(self):
ty = self.Infer("""
Expand Down
2 changes: 2 additions & 0 deletions pytype/tools/analyze_project/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@

# The missing fields will be filled in by generate_sample_config_or_die.
_PYTYPE_SINGLE_ITEMS = {
'check_variable_types': Item(
None, 'False', ArgInfo('--check-variable-types', None), None),
'disable': Item(
None, 'pyi-error', ArgInfo('--disable', ','.join),
'Comma or space separated list of error names to ignore.'),
Expand Down
Loading

0 comments on commit 835cf17

Please sign in to comment.