From 4ad39979bc62d1f33aae217c84575e0007ed682c Mon Sep 17 00:00:00 2001 From: mdemello Date: Wed, 20 Dec 2023 12:15:15 -0800 Subject: [PATCH 1/9] Do not crash if we encounter a malformed splat in a constant list. Now correctly raises a python-compiler-error for code like `[*42]`. PiperOrigin-RevId: 592625727 --- pytype/constant_folding.py | 13 +++++++++---- pytype/constant_folding_test.py | 5 +++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pytype/constant_folding.py b/pytype/constant_folding.py index 7e6d46bdb..88c6b7d21 100644 --- a/pytype/constant_folding.py +++ b/pytype/constant_folding.py @@ -336,10 +336,15 @@ def build_tuple(tup): other_elts = tuple(_Constant(('prim', e), v, None, other.op) for (_, e), v in zip(other_et, other.value)) elif other_tag == 'prim': - assert other_et == str - other_et = {other.typ} - other_elts = tuple(_Constant(('prim', str), v, None, other.op) - for v in other.value) + if other_et == str: + other_et = {other.typ} + other_elts = tuple(_Constant(('prim', str), v, None, other.op) + for v in other.value) + else: + # We have some malformed code, e.g. [*42] + name = other_et.__name__ + msg = f'Value after * must be an iterable, not {name}' + raise ConstantError(msg, op) else: other_elts = other.elements typ = (tag, et | set(other_et)) diff --git a/pytype/constant_folding_test.py b/pytype/constant_folding_test.py index 629e99cde..81f72186a 100644 --- a/pytype/constant_folding_test.py +++ b/pytype/constant_folding_test.py @@ -84,6 +84,11 @@ def test_str_to_list(self): (1, ("list", str), ["a", "b", "c"], [str, str, str]) ]) + @test_utils.skipBeforePy((3, 9), "Test for new LIST_EXTEND opcode in 3.9") + def test_bad_extend(self): + with self.assertRaises(constant_folding.ConstantError): + self._process("a = [1, 2, *3]") + def test_map(self): actual = self._process("a = {'x': 1, 'y': '2'}") self.assertCountEqual(actual, [ From 565688236f8652d5b3b82186c3db758058d6a055 Mon Sep 17 00:00:00 2001 From: tsudol Date: Thu, 21 Dec 2023 11:47:25 -0800 Subject: [PATCH 2/9] Detect bad calls to `dataclasses.replace` - Ensure the object being replaced is actually an instance of a dataclass. - Ensure all the kwargs are actually fields of that dataclass. PiperOrigin-RevId: 592915704 --- pytype/overlays/dataclass_overlay.py | 58 ++++++++++++++++++++++++++++ pytype/tests/test_dataclasses.py | 33 ++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/pytype/overlays/dataclass_overlay.py b/pytype/overlays/dataclass_overlay.py index fd94a986e..8d45f2ad0 100644 --- a/pytype/overlays/dataclass_overlay.py +++ b/pytype/overlays/dataclass_overlay.py @@ -22,6 +22,7 @@ def __init__(self, ctx): member_map = { "dataclass": Dataclass.make, "field": FieldFunction.make, + "replace": Replace.make, } ast = ctx.loader.import_name("dataclasses") super().__init__(ctx, "dataclasses", member_map, ast) @@ -220,3 +221,60 @@ def match_initvar(var): def match_classvar(var): """Unpack the type parameter from ClassVar[T].""" return abstract_utils.match_type_container(var, "typing.ClassVar") + + +class Replace(abstract.PyTDFunction): + """Implements dataclasses.replace.""" + + @classmethod + def make(cls, ctx, module="dataclasses"): + return super().make("replace", ctx, module) + + def _match_args_sequentially(self, node, args, alias_map, match_all_views): + ret = super()._match_args_sequentially( + node, args, alias_map, match_all_views + ) + # _match_args_sequentially has succeeded, so we know we have 1 posarg (the + # object) and some number of named args (the new fields). + (obj,) = args.posargs + if len(obj.data) != 1: + return ret + obj = abstract_utils.get_atomic_value(obj) + if obj.cls == self.ctx.convert.unsolvable: + return ret + if not abstract_utils.is_dataclass(obj.cls): + bad = abstract_utils.BadType("__obj", obj.cls) + sig = self.signatures[0].signature + raise function.WrongArgTypes(sig, args, self.ctx, bad) + invalid_names = tuple( + name for name in args.namedargs.keys() if name not in obj.cls + ) + if invalid_names: + # pylint: disable=line-too-long + # If we use the signature of replace() in the error message, it will be + # very confusing to users: + # Invalid keyword arguments (y, z) to function dataclasses.replace [wrong-keyword-args] + # Expected: (__obj, **changes) + # Actually passed: (__obj, y, z) + # Instead, we construct a fake signature that shows the expected fields: + # Invalid keyword arguments (y, z) to function dataclasses.replace [wrong-keyword-args] + # Expected: (__obj: Test, *, x) + # Actually passed: (__obj: Test, y, z) + # We also cheat a little bit by making sure the type of the object is + # included in the signature, pointing users towards more info. + # pylint: enable=line-too-long + fields = obj.cls.metadata["__dataclass_fields__"] + s = self.signatures[0].signature + sig = function.Signature( + name=s.name, + param_names=(f"__obj: {obj.cls.full_name}",), + posonly_count=s.posonly_count, + varargs_name=s.varargs_name, + kwonly_params=tuple(f.name for f in fields), + defaults=s.defaults, + kwargs_name=None, + annotations={}, # not used when printing errors. + postprocess_annotations=False, + ) + raise function.WrongKeywordArgs(sig, args, self.ctx, invalid_names) + return ret diff --git a/pytype/tests/test_dataclasses.py b/pytype/tests/test_dataclasses.py index abb30f7b0..04fc67f2d 100644 --- a/pytype/tests/test_dataclasses.py +++ b/pytype/tests/test_dataclasses.py @@ -784,6 +784,24 @@ class X: def __init__(self, b: int, a: str = ...) -> None: ... """) + def test_replace_wrong_keyword_args(self): + self.CheckWithErrors(""" + import dataclasses + @dataclasses.dataclass + class Test: + x: int + x = Test(1) + dataclasses.replace(x, y=1, z=2) # wrong-keyword-args + """) + + def test_replace_not_a_dataclass(self): + self.CheckWithErrors(""" + import dataclasses + class Test: + pass + dataclasses.replace(Test(), y=1, z=2) # wrong-arg-types + """) + class TestPyiDataclass(test_base.BaseTest): """Tests for @dataclasses in pyi files.""" @@ -1283,6 +1301,21 @@ class B(foo.A): b = B(1, '1') """) + def test_replace_wrong_keyword_args(self): + with self.DepTree([("foo.pyi", """ + import dataclasses + @dataclasses.dataclass + class Test: + x: int + def __init__(self, x: int) -> None: ... + """)]): + self.CheckWithErrors(""" + import dataclasses + import foo + x = foo.Test(1) + dataclasses.replace(x, y=1, z=2) # wrong-keyword-args + """) + if __name__ == "__main__": test_base.main() From e79894a8eec6d2de373d60dfa246e0a0c4f89e4e Mon Sep 17 00:00:00 2001 From: mdemello Date: Tue, 2 Jan 2024 13:58:43 -0800 Subject: [PATCH 3/9] Allow ParamSpecs as generic class parameters. PiperOrigin-RevId: 595203342 --- pytype/abstract/abstract_utils.py | 10 +++++++--- pytype/tests/test_paramspec.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/pytype/abstract/abstract_utils.py b/pytype/abstract/abstract_utils.py index 15b753362..9e2ec3da8 100644 --- a/pytype/abstract/abstract_utils.py +++ b/pytype/abstract/abstract_utils.py @@ -727,16 +727,20 @@ def get_type_parameter_substitutions( return subst +def is_type_variable(val: _BaseValueType): + """Check if a value is a type variable (TypeVar or ParamSpec).""" + return _isinstance(val, ("TypeParameter", "ParamSpec")) + + def build_generic_template( type_params: Sequence[_BaseValueType], base_type: _BaseValueType ) -> Tuple[Sequence[str], Sequence[_TypeParamType]]: """Build a typing.Generic template from a sequence of type parameters.""" - if not all(_isinstance(item, "TypeParameter") for item in type_params): + if not all(is_type_variable(item) for item in type_params): base_type.ctx.errorlog.invalid_annotation( base_type.ctx.vm.frames, base_type, "Parameters to Generic[...] must all be type variables") - type_params = [item for item in type_params - if _isinstance(item, "TypeParameter")] + type_params = [item for item in type_params if is_type_variable(item)] template = [item.name for item in type_params] diff --git a/pytype/tests/test_paramspec.py b/pytype/tests/test_paramspec.py index 0289c6ebe..7fbcbbb2d 100644 --- a/pytype/tests/test_paramspec.py +++ b/pytype/tests/test_paramspec.py @@ -187,6 +187,18 @@ def g(x: int, y: str) -> bool: assert_type(b, bool) """) + def test_use_as_protocol_parameter(self): + self.Check(""" + from typing import ParamSpec, Protocol, TypeVar + + P = ParamSpec('P') + T = TypeVar('T') + + class CallLogger(Protocol[P, T]): + def args(self, *args: P.args, **kwargs: P.kwargs) -> None: + pass + """) + _DECORATOR_PYI = """ from typing import TypeVar, ParamSpec, Callable, List From e987b873d9d0da0459d39cd54c9f11c01fa18267 Mon Sep 17 00:00:00 2001 From: tsudol Date: Tue, 2 Jan 2024 14:17:03 -0800 Subject: [PATCH 4/9] Allow resolved LateTypes to behave just a little more like their underlying types. InterepreterClass and PytdClass both implement `__contains__`, so resolved LateAnnotations should too. PiperOrigin-RevId: 595207992 --- pytype/abstract/_typing.py | 3 +++ pytype/tests/test_dataclasses.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/pytype/abstract/_typing.py b/pytype/abstract/_typing.py index c7c301d36..19da49e39 100644 --- a/pytype/abstract/_typing.py +++ b/pytype/abstract/_typing.py @@ -736,6 +736,9 @@ def __setattr__(self, name, value): return super().__setattr__(name, value) return self._type.__setattr__(name, value) + def __contains__(self, name): + return self.resolved and name in self._type + def resolve(self, node, f_globals, f_locals): """Resolve the late annotation.""" if self.resolved: diff --git a/pytype/tests/test_dataclasses.py b/pytype/tests/test_dataclasses.py index 04fc67f2d..30120c797 100644 --- a/pytype/tests/test_dataclasses.py +++ b/pytype/tests/test_dataclasses.py @@ -1316,6 +1316,22 @@ def __init__(self, x: int) -> None: ... dataclasses.replace(x, y=1, z=2) # wrong-keyword-args """) + def test_replace_late_annotation(self): + # Regression test: LateAnnotations (like `z: Z`) should behave + # like their underlying types once resolved. The dataclass overlay + # relies on this behavior. + self.Check(""" + from __future__ import annotations + import dataclasses + @dataclasses.dataclass + class A: + z: Z + def do(self): + return dataclasses.replace(self.z, name="A") + @dataclasses.dataclass + class Z: + name: str + """) if __name__ == "__main__": test_base.main() From 56f876e90748b21751f49746d5cb7d20407de88b Mon Sep 17 00:00:00 2001 From: rechen Date: Tue, 2 Jan 2024 14:58:32 -0800 Subject: [PATCH 5/9] Mark PEP 655 (typing.Required, typing.NotRequired) as supported. I added support for this feature back in December, but I figured we shouldn't mark it as supported until we're close to resuming releases. PiperOrigin-RevId: 595218047 --- docs/support.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/support.md b/docs/support.md index 7302843b9..4893c5f70 100644 --- a/docs/support.md +++ b/docs/support.md @@ -17,7 +17,7 @@ of pytype. * [Third-Party Libraries](#third-party-libraries) - + @@ -79,7 +79,7 @@ Feature [PEP 613 -- Explicit Type Aliases][613] | 3.10 | ✅ | [PEP 647 -- User-Defined Type Guards][647] | 3.10 | ✅ | [PEP 646 -- Variadic Generics][646] | 3.11 | ❌ | [#1525][variadic-generics] -[PEP 655 -- Marking individual TypedDict items as required or potentially-missing][655] | 3.11 | ❌ | [#1551][typed-dict-requirements] +[PEP 655 -- Marking individual TypedDict items as required or potentially-missing][655] | 3.11 | ✅ | [PEP 673 -- Self Type][673] | 3.11 | ✅ | [PEP 675 -- Arbitrary Literal String Type][675] | 3.11 | ❌ | [#1552][literal-string] [PEP 681 -- Data Class Transforms][681] | 3.11 | 🟡 | [#1553][dataclass-transform] From e7fcd3c1202343c4f82e2fdb3f54b620efb0816e Mon Sep 17 00:00:00 2001 From: tsudol Date: Wed, 3 Jan 2024 11:22:35 -0800 Subject: [PATCH 6/9] Make sure dataclasses.replace actually has args to unpack. PiperOrigin-RevId: 595455184 --- pytype/overlays/dataclass_overlay.py | 12 ++++++++++++ pytype/tests/test_dataclasses.py | 15 +++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/pytype/overlays/dataclass_overlay.py b/pytype/overlays/dataclass_overlay.py index 8d45f2ad0..e046d99e2 100644 --- a/pytype/overlays/dataclass_overlay.py +++ b/pytype/overlays/dataclass_overlay.py @@ -234,6 +234,18 @@ def _match_args_sequentially(self, node, args, alias_map, match_all_views): ret = super()._match_args_sequentially( node, args, alias_map, match_all_views ) + if not args.posargs: + # This is a weird case where pytype thinks the call can succeed, but + # there's no concrete `__obj` in the posargs. + # This can happen when `dataclasses.replace` is called with **kwargs: + # @dataclasses.dataclass + # class A: + # replace = dataclasses.replace + # def do(self, **kwargs): + # return self.replace(**kwargs) + # (Yes, this is a simplified example of real code.) + # Since **kwargs is opaque magic, we can't do more type checking. + return ret # _match_args_sequentially has succeeded, so we know we have 1 posarg (the # object) and some number of named args (the new fields). (obj,) = args.posargs diff --git a/pytype/tests/test_dataclasses.py b/pytype/tests/test_dataclasses.py index 30120c797..f5d6e847d 100644 --- a/pytype/tests/test_dataclasses.py +++ b/pytype/tests/test_dataclasses.py @@ -1333,5 +1333,20 @@ class Z: name: str """) + def test_replace_as_method_with_kwargs(self): + # This is a weird case where replace is added as a method, then called + # with kwargs. This makes pytype unable to see that `self` is the object + # being modified, and also caused a crash when the dataclass overlay tries + # to unpack the object being modified from the args. + self.Check(""" + import dataclasses + @dataclasses.dataclass + class WithKwargs: + replace = dataclasses.replace + def do(self, **kwargs): + return self.replace(**kwargs) + """) + + if __name__ == "__main__": test_base.main() From 7b3039762515cbd55980f3109f4a8e3bd2d1735b Mon Sep 17 00:00:00 2001 From: rechen Date: Wed, 3 Jan 2024 14:25:32 -0800 Subject: [PATCH 7/9] Fix crash caused by incorrect assumption about the type of an ast node. Fixes https://github.com/google/pytype/issues/1556. PiperOrigin-RevId: 595503542 --- pytype/pyi/parser.py | 6 +++++- pytype/pyi/parser_test.py | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/pytype/pyi/parser.py b/pytype/pyi/parser.py index e6bab8bc9..92d4a64b4 100644 --- a/pytype/pyi/parser.py +++ b/pytype/pyi/parser.py @@ -279,7 +279,11 @@ class _MetadataVisitor(visitor.BaseVisitor): def visit_Call(self, node): posargs = tuple(evaluator.literal_eval(x) for x in node.args) kwargs = {x.arg: evaluator.literal_eval(x.value) for x in node.keywords} - return (node.func.id, posargs, kwargs) + if isinstance(node.func, astlib.Attribute): + func_name = _attribute_to_name(node.func) + else: + func_name = node.func + return (func_name.id, posargs, kwargs) def visit_Dict(self, node): return evaluator.literal_eval(node) diff --git a/pytype/pyi/parser_test.py b/pytype/pyi/parser_test.py index 3404a1949..10bc90a4c 100644 --- a/pytype/pyi/parser_test.py +++ b/pytype/pyi/parser_test.py @@ -2885,6 +2885,19 @@ class Foo: x: Annotated[int, Signal] """) + def test_attribute_access_and_call(self): + self.check(""" + from typing import Annotated, Any + a: Any + def f() -> Annotated[list[int], a.b.C(3)]: ... + """, """ + from typing import Annotated, Any, List + + a: Any + + def f() -> Annotated[List[int], {'tag': 'call', 'fn': 'a.b.C', 'posargs': (3,), 'kwargs': {}}]: ... + """) + class ErrorTest(test_base.UnitTest): """Test parser errors.""" From afaa4962385cd0cacd283fe4cc59073a6d77cf9d Mon Sep 17 00:00:00 2001 From: rechen Date: Wed, 3 Jan 2024 14:33:16 -0800 Subject: [PATCH 8/9] Add PEP 692 to support.md. PiperOrigin-RevId: 595505755 --- docs/support.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/support.md b/docs/support.md index 4893c5f70..f98f8c96d 100644 --- a/docs/support.md +++ b/docs/support.md @@ -17,7 +17,7 @@ of pytype. * [Third-Party Libraries](#third-party-libraries) - + @@ -83,6 +83,7 @@ Feature [PEP 673 -- Self Type][673] | 3.11 | ✅ | [PEP 675 -- Arbitrary Literal String Type][675] | 3.11 | ❌ | [#1552][literal-string] [PEP 681 -- Data Class Transforms][681] | 3.11 | 🟡 | [#1553][dataclass-transform] +[PEP 692 -- Using TypedDict for more precise **kwargs typing][692] | 3.12 | ❌ | [#1558][typeddict-unpack] [PEP 695 -- Type Parameter Syntax][695] | 3.12 | ❌ | [PEP 698 -- Override Decorator for Static Typing][698] | 3.12 | ✅ | Custom Recursive Types | 3.6 | ✅ | @@ -186,6 +187,7 @@ Tensorflow | 🟡 | Minimal, Google-internal [673]: https://www.python.org/dev/peps/pep-0673 [675]: https://peps.python.org/pep-0675/ [681]: https://peps.python.org/pep-0681/ +[692]: https://peps.python.org/pep-0692/ [695]: https://peps.python.org/pep-0695/ [698]: https://peps.python.org/pep-0698/ [annotated]: https://github.com/google/pytype/issues/791 @@ -205,5 +207,5 @@ Tensorflow | 🟡 | Minimal, Google-internal [pytype-typing-faq]: https://google.github.io/pytype/typing_faq.html [self]: https://github.com/google/pytype/issues/1283 [type-guards]: https://github.com/google/pytype/issues/916 -[typed-dict-requirements]: https://github.com/google/pytype/issues/1551 +[typeddict-unpack]: https://github.com/google/pytype/issues/1558 [variadic-generics]: https://github.com/google/pytype/issues/1525 From d827002b1bee392b0a4348844e1337e5d75dab2b Mon Sep 17 00:00:00 2001 From: tsudol Date: Wed, 3 Jan 2024 16:19:35 -0800 Subject: [PATCH 9/9] Loosen the check on the first arg to `dataclasses.replace()`. It turns out this check is actually very noisy and can be impacted by CFG complexity, possibly-empty containers, generic dataclasses, etc. PiperOrigin-RevId: 595531087 --- pytype/overlays/dataclass_overlay.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pytype/overlays/dataclass_overlay.py b/pytype/overlays/dataclass_overlay.py index e046d99e2..5d792f594 100644 --- a/pytype/overlays/dataclass_overlay.py +++ b/pytype/overlays/dataclass_overlay.py @@ -252,12 +252,15 @@ def _match_args_sequentially(self, node, args, alias_map, match_all_views): if len(obj.data) != 1: return ret obj = abstract_utils.get_atomic_value(obj) - if obj.cls == self.ctx.convert.unsolvable: + # There are some cases where the user knows that obj will be a dataclass + # instance, but we don't. These instances are commonly false positives, so + # we should ignore them. + # (Consider a generic function where an `obj: T` is passed to replace().) + if ( + obj.cls == self.ctx.convert.unsolvable + or not abstract_utils.is_dataclass(obj.cls) + ): return ret - if not abstract_utils.is_dataclass(obj.cls): - bad = abstract_utils.BadType("__obj", obj.cls) - sig = self.signatures[0].signature - raise function.WrongArgTypes(sig, args, self.ctx, bad) invalid_names = tuple( name for name in args.namedargs.keys() if name not in obj.cls )