Skip to content

Commit e870866

Browse files
shannonzhufacebook-github-bot
authored andcommitted
Store temporary annotations for defined attribute assignments
Summary: Extend temporary annotation setting to attribute assignments. All temporary annotations are still cleared whenever we see a call. This diff already makes sure that any property attributes are excluded from temporary refinement, but added a TODO to handle checking for the presence of getattr on the parent class to exclude those, too. Will incrementally add temporary refinement + testing for asserts and other AST pieces that can trigger local type changes. Reviewed By: grievejia Differential Revision: D30400325 fbshipit-source-id: 4e3749c8bd0899c875985c0c3a0b73fb80e2553d
1 parent b0a7de9 commit e870866

File tree

5 files changed

+92
-12
lines changed

5 files changed

+92
-12
lines changed

source/analysis/test/integration/annotationTest.ml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,6 @@ let test_check_immutable_annotations context =
897897
|}
898898
[
899899
"Missing attribute annotation [4]: Attribute `attribute` of class `Foo` has no type specified.";
900-
"Incompatible return type [7]: Expected `int` but got `unknown`.";
901900
];
902901
assert_type_errors
903902
{|

source/analysis/test/integration/attributeTest.ml

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ let test_check_attributes context =
126126
|}
127127
[
128128
"Undefined attribute [16]: `Foo` has no attribute `bar`.";
129-
"Incompatible return type [7]: Expected `int` but got `unknown`.";
129+
"Incompatible return type [7]: Expected `int` but got `str`.";
130130
];
131131
assert_type_errors
132132
{|
@@ -248,7 +248,7 @@ let test_check_attributes context =
248248
|}
249249
[
250250
"Undefined attribute [16]: `Foo` has no attribute `bar`.";
251-
"Incompatible return type [7]: Expected `int` but got `unknown`.";
251+
"Incompatible return type [7]: Expected `int` but got `str`.";
252252
];
253253
assert_type_errors
254254
{|
@@ -298,7 +298,6 @@ let test_check_attributes context =
298298
[
299299
"Uninitialized attribute [13]: Attribute `bar` is declared in class `Foo` "
300300
^ "to have type `typing.Optional[int]` but is never initialized.";
301-
"Incompatible return type [7]: Expected `int` but got `typing.Optional[int]`.";
302301
];
303302
assert_type_errors
304303
{|
@@ -355,7 +354,6 @@ let test_check_attributes context =
355354
^ "has type `int` but no type is specified.";
356355
"Missing attribute annotation [4]: Attribute `baz` of class `Foo` "
357356
^ "has type `int` but no type is specified.";
358-
"Incompatible return type [7]: Expected `int` but got `unknown`.";
359357
];
360358
assert_type_errors
361359
{|
@@ -369,7 +367,7 @@ let test_check_attributes context =
369367
|}
370368
[
371369
"Undefined attribute [16]: `Foo` has no attribute `baz`.";
372-
"Incompatible return type [7]: Expected `int` but got `unknown`.";
370+
"Incompatible return type [7]: Expected `int` but got `typing.Optional[int]`.";
373371
];
374372

375373
(* Ensure synthetic attribute accesses don't mask errors on real ones. *)
@@ -617,7 +615,24 @@ let test_check_attributes context =
617615
self.attribute = not_annotated()
618616
a = self.attribute.something
619617
|}
620-
["Undefined attribute [16]: `int` has no attribute `something`."];
618+
[];
619+
620+
assert_type_errors
621+
{|
622+
def returns_string() -> str:
623+
return ""
624+
625+
class Foo:
626+
attribute: int = 1
627+
def foo(self) -> None:
628+
self.attribute = returns_string()
629+
a = self.attribute.something
630+
|}
631+
[
632+
"Incompatible attribute type [8]: Attribute `attribute` declared in class `Foo` has type \
633+
`int` but is used as type `str`.";
634+
"Undefined attribute [16]: `int` has no attribute `something`.";
635+
];
621636

622637
(* Do not resolve optional attributes to the optional type. *)
623638
assert_type_errors

source/analysis/test/integration/errorMessageTest.ml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,6 @@ let test_show_error_traces context =
126126
test.py:7:4.";
127127
"Unbound name [10]: Name `x` is used but not defined in the current scope. Did you forget to \
128128
import it or assign to it?";
129-
"Incompatible return type [7]: Expected `str` but got `unknown`. Type `str` expected on line \
130-
8, specified on line 5.";
131129
];
132130
assert_type_errors
133131
{|

source/analysis/test/integration/refinementTest.ml

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,6 +1044,71 @@ let test_check_temporary_refinement context =
10441044
`typing_extensions.Literal[1]`).";
10451045
"Revealed type [-1]: Revealed type for `MY_GLOBAL` is `float`.";
10461046
];
1047+
assert_type_errors
1048+
{|
1049+
from typing import Optional
1050+
1051+
class Foo:
1052+
attribute: Optional[int] = 1
1053+
1054+
def takes_non_optional_int(input: int) -> None:
1055+
pass
1056+
1057+
def test(foo: Foo) -> None:
1058+
reveal_type(foo.attribute)
1059+
foo.attribute = 1
1060+
reveal_type(foo.attribute)
1061+
takes_non_optional_int(foo.attribute)
1062+
reveal_type(foo.attribute)
1063+
|}
1064+
[
1065+
"Revealed type [-1]: Revealed type for `foo.attribute` is `Optional[int]`.";
1066+
"Revealed type [-1]: Revealed type for `foo.attribute` is `Optional[int]` (inferred: \
1067+
`typing_extensions.Literal[1]`).";
1068+
"Revealed type [-1]: Revealed type for `foo.attribute` is `Optional[int]`.";
1069+
];
1070+
assert_type_errors
1071+
{|
1072+
from typing import Optional
1073+
1074+
class Foo:
1075+
@property
1076+
def attribute(self) -> Optional[int]:
1077+
pass
1078+
@attribute.setter
1079+
def attribute(self, value: Optional[int]) -> None:
1080+
pass
1081+
1082+
def test(foo: Foo) -> None:
1083+
reveal_type(foo.attribute)
1084+
foo.attribute = 1
1085+
reveal_type(foo.attribute)
1086+
|}
1087+
[
1088+
"Revealed type [-1]: Revealed type for `foo.attribute` is `Optional[int]`.";
1089+
"Revealed type [-1]: Revealed type for `foo.attribute` is `Optional[int]`.";
1090+
];
1091+
(* TODO(T70545381): Don't allow refinement if __getattr__ override exists. *)
1092+
assert_type_errors
1093+
{|
1094+
from typing import Optional
1095+
1096+
class Foo:
1097+
attribute: Optional[int] = 1
1098+
1099+
def __getattr__(self, value: str) -> int:
1100+
return 1
1101+
1102+
def test(foo: Foo) -> None:
1103+
reveal_type(foo.attribute)
1104+
foo.attribute = 1
1105+
reveal_type(foo.attribute)
1106+
|}
1107+
[
1108+
"Revealed type [-1]: Revealed type for `foo.attribute` is `Optional[int]`.";
1109+
"Revealed type [-1]: Revealed type for `foo.attribute` is `Optional[int]` (inferred: \
1110+
`typing_extensions.Literal[1]`).";
1111+
];
10471112
()
10481113

10491114

source/analysis/typeCheck.ml

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4834,9 +4834,12 @@ module State (Context : Context) = struct
48344834
| Attribute _ as name when is_simple_name name -> (
48354835
match resolved_base, attribute with
48364836
| `Attribute (_, parent), Some (attribute, _)
4837-
when not
4838-
(Annotated.Attribute.defined attribute
4839-
|| is_undefined_attribute parent) ->
4837+
when not (Annotated.Attribute.property attribute) ->
4838+
let is_temporary_refinement =
4839+
is_temporary_refinement
4840+
|| Annotated.Attribute.defined attribute
4841+
|| is_undefined_attribute parent
4842+
in
48404843
Resolution.set_local_with_attributes
48414844
~temporary:is_temporary_refinement
48424845
resolution

0 commit comments

Comments
 (0)