Skip to content

Commit 71d7112

Browse files
authored
[ty] No union with Unknown for module-global symbols (#20664)
## Summary Quoting from the newly added comment: Module-level globals can be mutated externally. A `MY_CONSTANT = 1` global might be changed to `"some string"` from code outside of the module that we're looking at, and so from a gradual-guarantee perspective, it makes sense to infer a type of `Literal[1] | Unknown` for global symbols. This allows the code that does the mutation to type check correctly, and for code that uses the global, it accurately reflects the lack of knowledge about the type. External modifications (or modifications through `global` statements) that would require a wider type are relatively rare. From a practical perspective, we can therefore achieve a better user experience by trusting the inferred type. Users who need the external mutation to work can always annotate the global with the wider type. And everyone else benefits from more precise type inference. I initially implemented this by applying literal promotion to the type of the unannotated module globals (as suggested in astral-sh/ty#1069), but the ecosystem impact showed a lot of problems (#20643). I fixed/patched some of these problems, but this PR seems like a good first step, and it seems sensible to apply the literal promotion change in a second step that can be evaluated separately. closes astral-sh/ty#1069 ## Ecosystem impact This seems like an (unexpectedly large) net positive with 650 fewer diagnostics overall.. even though this change will certainly catch more true positives. * There are 666 removed `type-assertion-failure` diagnostics, where we were previously used the correct type already, but removing the `Unknown` now leads to an "exact" match. * 1464 of the 1805 total new diagnostics are `unresolved-attribute` errors, most (1365) of which were previously `possibly-missing-attribute` errors. So they could also be counted as "changed" diagnostics. * For code that uses constants like ```py IS_PYTHON_AT_LEAST_3_10 = sys.version_info >= (3, 10) ``` where we would have previously inferred a type of `Literal[True/False] | Unknown`, removing the `Unknown` now allows us to do reachability analysis on branches that use these constants, and so we get a lot of favorable ecosystem changes because of that. * There is code like the following, where we previously emitted `conflicting-argument-forms` diagnostics on calls to the aliased `assert_type`, because its type was `Unknown | def …` (and the call to `Unknown` "used" the type form argument in a non type-form way): ```py if sys.version_info >= (3, 11): import typing assert_type = typing.assert_type else: import typing_extensions assert_type = typing_extensions.assert_type ``` * ~100 new `invalid-argument-type` false positives, due to missing `**kwargs` support (astral-sh/ty#247) ## Typing conformance ```diff +protocols_modules.py:25:1: error[invalid-assignment] Object of type `<module '_protocols_modules1'>` is not assignable to `Options1` ``` This diagnostic should apparently not be there, but it looks like we also fail other tests in that file, so it seems to be a limitation that was previously hidden by `Unknown` somehow. ## Test Plan Updated tests and relatively thorough ecosystem analysis.
1 parent eb34d12 commit 71d7112

File tree

16 files changed

+83
-93
lines changed

16 files changed

+83
-93
lines changed

crates/ty_python_semantic/resources/mdtest/del.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ def foo():
108108
global x
109109
def bar():
110110
# allowed, refers to `x` in the global scope
111-
reveal_type(x) # revealed: Unknown | Literal[1]
111+
reveal_type(x) # revealed: Literal[1]
112112
bar()
113113
del x # allowed, deletes `x` in the global scope (though we don't track that)
114114
```

crates/ty_python_semantic/resources/mdtest/import/conditional.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ reveal_type(y)
2525
# error: [possibly-missing-import] "Member `y` of module `maybe_unbound` may be missing"
2626
from maybe_unbound import x, y
2727

28-
reveal_type(x) # revealed: Unknown | Literal[3]
29-
reveal_type(y) # revealed: Unknown | Literal[3]
28+
reveal_type(x) # revealed: Literal[3]
29+
reveal_type(y) # revealed: Literal[3]
3030
```
3131

3232
## Maybe unbound annotated
@@ -56,7 +56,7 @@ Importing an annotated name prefers the declared type over the inferred type:
5656
# error: [possibly-missing-import] "Member `y` of module `maybe_unbound_annotated` may be missing"
5757
from maybe_unbound_annotated import x, y
5858

59-
reveal_type(x) # revealed: Unknown | Literal[3]
59+
reveal_type(x) # revealed: Literal[3]
6060
reveal_type(y) # revealed: int
6161
```
6262

crates/ty_python_semantic/resources/mdtest/import/dunder_all.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -783,8 +783,8 @@ class A: ...
783783
```py
784784
from subexporter import *
785785

786-
# TODO: Should we avoid including `Unknown` for this case?
787-
reveal_type(__all__) # revealed: Unknown | list[Unknown | str]
786+
# TODO: we could potentially infer `list[str] | tuple[str, ...]` here
787+
reveal_type(__all__) # revealed: list[Unknown | str]
788788

789789
__all__.append("B")
790790

crates/ty_python_semantic/resources/mdtest/import/module_getattr.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def __getattr__(name: str) -> int:
4040
import mixed_module
4141

4242
# Explicit attribute should take precedence
43-
reveal_type(mixed_module.explicit_attr) # revealed: Unknown | Literal["explicit"]
43+
reveal_type(mixed_module.explicit_attr) # revealed: Literal["explicit"]
4444

4545
# `__getattr__` should handle unknown attributes
4646
reveal_type(mixed_module.dynamic_attr) # revealed: str

crates/ty_python_semantic/resources/mdtest/import/namespace.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ x = "namespace"
103103
```py
104104
from foo import x
105105

106-
reveal_type(x) # revealed: Unknown | Literal["module"]
106+
reveal_type(x) # revealed: Literal["module"]
107107

108108
import foo.bar # error: [unresolved-import]
109109
```

crates/ty_python_semantic/resources/mdtest/import/star.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,8 @@ X = (Y := 3) + 4
144144
```py
145145
from exporter import *
146146

147-
reveal_type(X) # revealed: Unknown | Literal[7]
148-
reveal_type(Y) # revealed: Unknown | Literal[3]
147+
reveal_type(X) # revealed: Literal[7]
148+
reveal_type(Y) # revealed: Literal[3]
149149
```
150150

151151
### Global-scope symbols defined in many other ways
@@ -781,9 +781,9 @@ else:
781781
from exporter import *
782782

783783
# error: [possibly-unresolved-reference]
784-
reveal_type(A) # revealed: Unknown | Literal[1]
784+
reveal_type(A) # revealed: Literal[1]
785785

786-
reveal_type(B) # revealed: Unknown | Literal[2, 3]
786+
reveal_type(B) # revealed: Literal[2, 3]
787787
```
788788

789789
### Reachability constraints in the importing module
@@ -804,7 +804,7 @@ if coinflip():
804804
from exporter import *
805805

806806
# error: [possibly-unresolved-reference]
807-
reveal_type(A) # revealed: Unknown | Literal[1]
807+
reveal_type(A) # revealed: Literal[1]
808808
```
809809

810810
### Reachability constraints in the exporting module *and* the importing module

crates/ty_python_semantic/resources/mdtest/narrow/assignment.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class _:
3434
[reveal_type(a.z) for _ in range(1)] # revealed: Literal[0]
3535

3636
def _():
37-
reveal_type(a.x) # revealed: Unknown | int | None
37+
reveal_type(a.x) # revealed: int | None
3838
reveal_type(a.y) # revealed: Unknown | None
3939
reveal_type(a.z) # revealed: Unknown | None
4040

@@ -75,7 +75,7 @@ class _:
7575

7676
if cond():
7777
a = A()
78-
reveal_type(a.x) # revealed: int | None | Unknown
78+
reveal_type(a.x) # revealed: int | None
7979
reveal_type(a.y) # revealed: Unknown | None
8080
reveal_type(a.z) # revealed: Unknown | None
8181

@@ -295,10 +295,10 @@ class C:
295295

296296
def _():
297297
# error: [possibly-missing-attribute]
298-
reveal_type(b.a.x[0]) # revealed: Unknown | int | None
298+
reveal_type(b.a.x[0]) # revealed: int | None
299299
# error: [possibly-missing-attribute]
300-
reveal_type(b.a.x) # revealed: Unknown | list[int | None]
301-
reveal_type(b.a) # revealed: Unknown | A | None
300+
reveal_type(b.a.x) # revealed: list[int | None]
301+
reveal_type(b.a) # revealed: A | None
302302
```
303303

304304
## Invalid assignments are not used for narrowing

crates/ty_python_semantic/resources/mdtest/narrow/complex_target.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,11 @@ if c.x is not None:
167167

168168
if c.x is not None:
169169
def _():
170-
reveal_type(c.x) # revealed: Unknown | int | None
170+
reveal_type(c.x) # revealed: int | None
171171

172172
def _():
173173
if c.x is not None:
174-
reveal_type(c.x) # revealed: (Unknown & ~None) | int
174+
reveal_type(c.x) # revealed: int
175175
```
176176

177177
## Subscript narrowing

crates/ty_python_semantic/resources/mdtest/narrow/conditionals/nested.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class B:
8686
reveal_type(a.x) # revealed: Literal["a"]
8787

8888
def f():
89-
reveal_type(a.x) # revealed: Unknown | str | None
89+
reveal_type(a.x) # revealed: str | None
9090

9191
[reveal_type(a.x) for _ in range(1)] # revealed: Literal["a"]
9292

@@ -96,7 +96,7 @@ class C:
9696
reveal_type(a.x) # revealed: str | None
9797

9898
def g():
99-
reveal_type(a.x) # revealed: Unknown | str | None
99+
reveal_type(a.x) # revealed: str | None
100100

101101
[reveal_type(a.x) for _ in range(1)] # revealed: str | None
102102

@@ -109,7 +109,7 @@ class D:
109109
reveal_type(a.x) # revealed: Literal["a"]
110110

111111
def h():
112-
reveal_type(a.x) # revealed: Unknown | str | None
112+
reveal_type(a.x) # revealed: str | None
113113

114114
# TODO: should be `str | None`
115115
[reveal_type(a.x) for _ in range(1)] # revealed: Literal["a"]
@@ -190,7 +190,7 @@ def f(x: str | None):
190190
reveal_type(g) # revealed: str
191191

192192
if a.x is not None:
193-
reveal_type(a.x) # revealed: (Unknown & ~None) | str
193+
reveal_type(a.x) # revealed: str
194194

195195
if l[0] is not None:
196196
reveal_type(l[0]) # revealed: str
@@ -206,7 +206,7 @@ def f(x: str | None):
206206
reveal_type(g) # revealed: str
207207

208208
if a.x is not None:
209-
reveal_type(a.x) # revealed: (Unknown & ~None) | str
209+
reveal_type(a.x) # revealed: str
210210

211211
if l[0] is not None:
212212
reveal_type(l[0]) # revealed: str
@@ -382,12 +382,12 @@ def f():
382382
if a.x is not None:
383383
def _():
384384
# Lazy nested scope narrowing is not performed on attributes/subscripts because it's difficult to track their changes.
385-
reveal_type(a.x) # revealed: Unknown | str | None
385+
reveal_type(a.x) # revealed: str | None
386386

387387
class D:
388-
reveal_type(a.x) # revealed: (Unknown & ~None) | str
388+
reveal_type(a.x) # revealed: str
389389

390-
[reveal_type(a.x) for _ in range(1)] # revealed: (Unknown & ~None) | str
390+
[reveal_type(a.x) for _ in range(1)] # revealed: str
391391

392392
if l[0] is not None:
393393
def _():
@@ -473,11 +473,11 @@ def f():
473473
if a.x is not None:
474474
def _():
475475
if a.x != 1:
476-
reveal_type(a.x) # revealed: (Unknown & ~Literal[1]) | str | None
476+
reveal_type(a.x) # revealed: str | None
477477

478478
class D:
479479
if a.x != 1:
480-
reveal_type(a.x) # revealed: (Unknown & ~Literal[1] & ~None) | str
480+
reveal_type(a.x) # revealed: str
481481

482482
if l[0] is not None:
483483
def _():

crates/ty_python_semantic/resources/mdtest/public_types.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ if flag():
263263
x = 1
264264

265265
def f() -> None:
266-
reveal_type(x) # revealed: Unknown | Literal[1, 2]
266+
reveal_type(x) # revealed: Literal[1, 2]
267267
# Function only used inside this branch
268268
f()
269269

0 commit comments

Comments
 (0)