[ty] No union with Unknown for module-global symbols (no promotion)

sharkdp · sharkdp · commit 72c90480ed35 · 2025-10-01T14:26:27.000+02:00
diff --git a/crates/ty_python_semantic/resources/mdtest/del.md b/crates/ty_python_semantic/resources/mdtest/del.md
@@ -108,7 +108,7 @@ def foo():
     global x
     def bar():
         # allowed, refers to `x` in the global scope
-        reveal_type(x)  # revealed: Unknown | Literal[1]
+        reveal_type(x)  # revealed: Literal[1]
     bar()
     del x  # allowed, deletes `x` in the global scope (though we don't track that)
 ```
diff --git a/crates/ty_python_semantic/resources/mdtest/import/conditional.md b/crates/ty_python_semantic/resources/mdtest/import/conditional.md
@@ -25,8 +25,8 @@ reveal_type(y)
 # error: [possibly-missing-import] "Member `y` of module `maybe_unbound` may be missing"
 from maybe_unbound import x, y
 
-reveal_type(x)  # revealed: Unknown | Literal[3]
-reveal_type(y)  # revealed: Unknown | Literal[3]
+reveal_type(x)  # revealed: Literal[3]
+reveal_type(y)  # revealed: Literal[3]
 ```
 
 ## Maybe unbound annotated
@@ -56,7 +56,7 @@ Importing an annotated name prefers the declared type over the inferred type:
 # error: [possibly-missing-import] "Member `y` of module `maybe_unbound_annotated` may be missing"
 from maybe_unbound_annotated import x, y
 
-reveal_type(x)  # revealed: Unknown | Literal[3]
+reveal_type(x)  # revealed: Literal[3]
 reveal_type(y)  # revealed: int
 ```
 
diff --git a/crates/ty_python_semantic/resources/mdtest/import/dunder_all.md b/crates/ty_python_semantic/resources/mdtest/import/dunder_all.md
@@ -783,8 +783,7 @@ class A: ...
 ```py
 from subexporter import *
 
-# TODO: Should we avoid including `Unknown` for this case?
-reveal_type(__all__)  # revealed: Unknown | list[Unknown | str]
+reveal_type(__all__)  # revealed: list[Unknown | str]
 
 __all__.append("B")
 
diff --git a/crates/ty_python_semantic/resources/mdtest/import/module_getattr.md b/crates/ty_python_semantic/resources/mdtest/import/module_getattr.md
@@ -40,7 +40,7 @@ def __getattr__(name: str) -> int:
 import mixed_module
 
 # Explicit attribute should take precedence
-reveal_type(mixed_module.explicit_attr)  # revealed: Unknown | Literal["explicit"]
+reveal_type(mixed_module.explicit_attr)  # revealed: Literal["explicit"]
 
 # `__getattr__` should handle unknown attributes
 reveal_type(mixed_module.dynamic_attr)  # revealed: str
diff --git a/crates/ty_python_semantic/resources/mdtest/import/namespace.md b/crates/ty_python_semantic/resources/mdtest/import/namespace.md
@@ -103,7 +103,7 @@ x = "namespace"
 ```py
 from foo import x
 
-reveal_type(x)  # revealed: Unknown | Literal["module"]
+reveal_type(x)  # revealed: Literal["module"]
 
 import foo.bar  # error: [unresolved-import]
 ```
diff --git a/crates/ty_python_semantic/resources/mdtest/import/star.md b/crates/ty_python_semantic/resources/mdtest/import/star.md
@@ -144,8 +144,8 @@ X = (Y := 3) + 4
 ```py
 from exporter import *
 
-reveal_type(X)  # revealed: Unknown | Literal[7]
-reveal_type(Y)  # revealed: Unknown | Literal[3]
+reveal_type(X)  # revealed: Literal[7]
+reveal_type(Y)  # revealed: Literal[3]
 ```
 
 ### Global-scope symbols defined in many other ways
@@ -781,9 +781,9 @@ else:
 from exporter import *
 
 # error: [possibly-unresolved-reference]
-reveal_type(A)  # revealed: Unknown | Literal[1]
+reveal_type(A)  # revealed: Literal[1]
 
-reveal_type(B)  # revealed: Unknown | Literal[2, 3]
+reveal_type(B)  # revealed: Literal[2, 3]
 ```
 
 ### Reachability constraints in the importing module
@@ -804,7 +804,7 @@ if coinflip():
     from exporter import *
 
 # error: [possibly-unresolved-reference]
-reveal_type(A)  # revealed: Unknown | Literal[1]
+reveal_type(A)  # revealed: Literal[1]
 ```
 
 ### Reachability constraints in the exporting module *and* the importing module
diff --git a/crates/ty_python_semantic/resources/mdtest/narrow/assignment.md b/crates/ty_python_semantic/resources/mdtest/narrow/assignment.md
@@ -34,7 +34,7 @@ class _:
 [reveal_type(a.z) for _ in range(1)]  # revealed: Literal[0]
 
 def _():
-    reveal_type(a.x)  # revealed: Unknown | int | None
+    reveal_type(a.x)  # revealed: int | None
     reveal_type(a.y)  # revealed: Unknown | None
     reveal_type(a.z)  # revealed: Unknown | None
 
@@ -75,7 +75,7 @@ class _:
 
     if cond():
         a = A()
-    reveal_type(a.x)  # revealed: int | None | Unknown
+    reveal_type(a.x)  # revealed: int | None
     reveal_type(a.y)  # revealed: Unknown | None
     reveal_type(a.z)  # revealed: Unknown | None
 
@@ -295,10 +295,10 @@ class C:
 
 def _():
     # error: [possibly-missing-attribute]
-    reveal_type(b.a.x[0])  # revealed: Unknown | int | None
+    reveal_type(b.a.x[0])  # revealed: int | None
     # error: [possibly-missing-attribute]
-    reveal_type(b.a.x)  # revealed: Unknown | list[int | None]
-    reveal_type(b.a)  # revealed: Unknown | A | None
+    reveal_type(b.a.x)  # revealed: list[int | None]
+    reveal_type(b.a)  # revealed: A | None
 ```
 
 ## Invalid assignments are not used for narrowing
diff --git a/crates/ty_python_semantic/resources/mdtest/narrow/complex_target.md b/crates/ty_python_semantic/resources/mdtest/narrow/complex_target.md
@@ -167,11 +167,11 @@ if c.x is not None:
 
 if c.x is not None:
     def _():
-        reveal_type(c.x)  # revealed: Unknown | int | None
+        reveal_type(c.x)  # revealed: int | None
 
 def _():
     if c.x is not None:
-        reveal_type(c.x)  # revealed: (Unknown & ~None) | int
+        reveal_type(c.x)  # revealed: int
 ```
 
 ## Subscript narrowing
diff --git a/crates/ty_python_semantic/resources/mdtest/narrow/conditionals/nested.md b/crates/ty_python_semantic/resources/mdtest/narrow/conditionals/nested.md
@@ -86,7 +86,7 @@ class B:
     reveal_type(a.x)  # revealed: Literal["a"]
 
 def f():
-    reveal_type(a.x)  # revealed: Unknown | str | None
+    reveal_type(a.x)  # revealed: str | None
 
 [reveal_type(a.x) for _ in range(1)]  # revealed: Literal["a"]
 
@@ -96,7 +96,7 @@ class C:
     reveal_type(a.x)  # revealed: str | None
 
 def g():
-    reveal_type(a.x)  # revealed: Unknown | str | None
+    reveal_type(a.x)  # revealed: str | None
 
 [reveal_type(a.x) for _ in range(1)]  # revealed: str | None
 
@@ -109,7 +109,7 @@ class D:
     reveal_type(a.x)  # revealed: Literal["a"]
 
 def h():
-    reveal_type(a.x)  # revealed: Unknown | str | None
+    reveal_type(a.x)  # revealed: str | None
 
 # TODO: should be `str | None`
 [reveal_type(a.x) for _ in range(1)]  # revealed: Literal["a"]
@@ -190,7 +190,7 @@ def f(x: str | None):
             reveal_type(g)  # revealed: str
 
         if a.x is not None:
-            reveal_type(a.x)  # revealed: (Unknown & ~None) | str
+            reveal_type(a.x)  # revealed: str
 
         if l[0] is not None:
             reveal_type(l[0])  # revealed: str
@@ -206,7 +206,7 @@ def f(x: str | None):
             reveal_type(g)  # revealed: str
 
         if a.x is not None:
-            reveal_type(a.x)  # revealed: (Unknown & ~None) | str
+            reveal_type(a.x)  # revealed: str
 
         if l[0] is not None:
             reveal_type(l[0])  # revealed: str
@@ -382,12 +382,12 @@ def f():
     if a.x is not None:
         def _():
             # Lazy nested scope narrowing is not performed on attributes/subscripts because it's difficult to track their changes.
-            reveal_type(a.x)  # revealed: Unknown | str | None
+            reveal_type(a.x)  # revealed: str | None
 
         class D:
-            reveal_type(a.x)  # revealed: (Unknown & ~None) | str
+            reveal_type(a.x)  # revealed: str
 
-        [reveal_type(a.x) for _ in range(1)]  # revealed: (Unknown & ~None) | str
+        [reveal_type(a.x) for _ in range(1)]  # revealed: str
 
     if l[0] is not None:
         def _():
@@ -473,11 +473,11 @@ def f():
         if a.x is not None:
             def _():
                 if a.x != 1:
-                    reveal_type(a.x)  # revealed: (Unknown & ~Literal[1]) | str | None
+                    reveal_type(a.x)  # revealed: str | None
 
             class D:
                 if a.x != 1:
-                    reveal_type(a.x)  # revealed: (Unknown & ~Literal[1] & ~None) | str
+                    reveal_type(a.x)  # revealed: str
 
         if l[0] is not None:
             def _():
diff --git a/crates/ty_python_semantic/resources/mdtest/public_types.md b/crates/ty_python_semantic/resources/mdtest/public_types.md
@@ -263,7 +263,7 @@ if flag():
     x = 1
 
     def f() -> None:
-        reveal_type(x)  # revealed: Unknown | Literal[1, 2]
+        reveal_type(x)  # revealed: Literal[1, 2]
     # Function only used inside this branch
     f()
 
diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/builtin.md b/crates/ty_python_semantic/resources/mdtest/scopes/builtin.md
@@ -29,8 +29,8 @@ if flag():
     chr: int = 1
 
 def _():
-    # TODO: Should ideally be `Unknown | Literal[1] | (def abs(x: SupportsAbs[_T], /) -> _T)`
-    reveal_type(abs)  # revealed: Unknown | Literal[1]
+    # TODO: Should ideally be `Literal[1] | (def abs(x: SupportsAbs[_T], /) -> _T)`
+    reveal_type(abs)  # revealed: Literal[1]
     # TODO: Should ideally be `int | (def chr(i: SupportsIndex, /) -> str)`
     reveal_type(chr)  # revealed: int
 ```
diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/eager.md b/crates/ty_python_semantic/resources/mdtest/scopes/eager.md
@@ -12,7 +12,7 @@ Function definitions are evaluated lazily.
 x = 1
 
 def f():
-    reveal_type(x)  # revealed: Unknown | Literal[1, 2]
+    reveal_type(x)  # revealed: Literal[1, 2]
 
 x = 2
 ```
@@ -283,7 +283,7 @@ x = 1
 
 def _():
     class C:
-        # revealed: Unknown | Literal[1]
+        # revealed: Literal[1]
         [reveal_type(x) for _ in [1]]
         x = 2
 ```
@@ -389,7 +389,7 @@ x = int
 class C:
     var: ClassVar[x]
 
-reveal_type(C.var)  # revealed: Unknown | int | str
+reveal_type(C.var)  # revealed: int | str
 
 x = str
 ```
diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/global.md b/crates/ty_python_semantic/resources/mdtest/scopes/global.md
@@ -8,7 +8,7 @@ A name reference to a never-defined symbol in a function is implicitly a global
 x = 1
 
 def f():
-    reveal_type(x)  # revealed: Unknown | Literal[1]
+    reveal_type(x)  # revealed: Literal[1]
 ```
 
 ## Explicit global in function
@@ -18,7 +18,7 @@ x = 1
 
 def f():
     global x
-    reveal_type(x)  # revealed: Unknown | Literal[1]
+    reveal_type(x)  # revealed: Literal[1]
 ```
 
 ## Unassignable type in function
@@ -201,7 +201,7 @@ x = 42
 
 def f():
     global x
-    reveal_type(x)  # revealed: Unknown | Literal[42]
+    reveal_type(x)  # revealed: Literal[42]
     x = "56"
     reveal_type(x)  # revealed: Literal["56"]
 ```
diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md b/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md
@@ -73,10 +73,10 @@ __spec__ = 42  # error: [invalid-assignment] "Object of type `Literal[42]` is no
 ```py
 import module
 
-reveal_type(module.__file__)  # revealed: Unknown | None
+reveal_type(module.__file__)  # revealed: None
 reveal_type(module.__path__)  # revealed: list[str]
 reveal_type(module.__doc__)  # revealed: Unknown
-reveal_type(module.__spec__)  # revealed: Unknown | ModuleSpec | None
+reveal_type(module.__spec__)  # revealed: ModuleSpec | None
 # error: [unresolved-attribute]
 reveal_type(module.__warningregistry__)  # revealed: Unknown
 
diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs
@@ -810,7 +810,7 @@ fn place_by_id<'db>(
             // modified externally, but those changes do not take effect. We therefore issue
             // a diagnostic if we see it being modified externally. In type inference, we
             // can assign a "narrow" type to it even if it is not *declared*. This means, we
-            // do not have to call [`widen_type_for_undeclared_public_symbol`].
+            // do not have to union with `Unknown`.
             //
             // `TYPE_CHECKING` is a special variable that should only be assigned `False`
             // at runtime, but is always considered `True` in type checking.
@@ -822,18 +822,38 @@ fn place_by_id<'db>(
                 )
             });
 
-            if scope.file(db).is_stub(db) || scope.scope(db).visibility().is_private() {
-                // We generally trust module-level undeclared places in stubs and do not union
-                // with `Unknown`. If we don't do this, simple aliases like `IOError = OSError` in
-                // stubs would result in `IOError` being a union of `OSError` and `Unknown`, which
-                // leads to all sorts of downstream problems. Similarly, type variables are often
-                // defined as `_T = TypeVar("_T")`, without being declared.
-                // Also, if the scope is private, such as a function scope,
-                // meaning that the place cannot be rewritten from elsewhere, we do not union with `Unknown`.
-
+            // 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.
+            let is_module_global = scope.node(db).scope_kind().is_module();
+
+            // If the visibility of the scope is private (like for a function scope), we also do
+            // not union with `Unknown`, because the symbol cannot be modified externally.
+            let scope_has_private_visibility = scope.scope(db).visibility().is_private();
+
+            // We generally trust module-level undeclared places in stubs and do not union with
+            // `Unknown`.
+            let in_stub_file = scope.file(db).is_stub(db);
+
+            if is_considered_non_modifiable
+                || is_module_global
+                || scope_has_private_visibility
+                || in_stub_file
+            {
                 inferred.into()
             } else {
-                widen_type_for_undeclared_public_symbol(db, inferred, is_considered_non_modifiable)
+                // Widen the inferred type of undeclared public symbols by unioning with `Unknown`
+                inferred
+                    .map_type(|ty| UnionType::from_elements(db, [Type::unknown(), ty]))
                     .into()
             }
         }
@@ -1585,29 +1605,6 @@ pub(crate) enum BoundnessAnalysis {
     BasedOnUnboundVisibility,
 }
 
-/// Computes a possibly-widened type `Unknown | T_inferred` from the inferred type `T_inferred`
-/// of a symbol, unless the type is a known-instance type (e.g. `typing.Any`) or the symbol is
-/// considered non-modifiable (e.g. when the symbol is `@Final`). We need this for public uses
-/// of symbols that have no declared type.
-fn widen_type_for_undeclared_public_symbol<'db>(
-    db: &'db dyn Db,
-    inferred: Place<'db>,
-    is_considered_non_modifiable: bool,
-) -> Place<'db> {
-    // We special-case known-instance types here since symbols like `typing.Any` are typically
-    // not declared in the stubs (e.g. `Any = object()`), but we still want to treat them as
-    // such.
-    let is_known_instance = inferred
-        .ignore_possibly_unbound()
-        .is_some_and(|ty| matches!(ty, Type::SpecialForm(_) | Type::KnownInstance(_)));
-
-    if is_considered_non_modifiable || is_known_instance {
-        inferred
-    } else {
-        inferred.map_type(|ty| UnionType::from_elements(db, [Type::unknown(), ty]))
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
