Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/ty_python_semantic/resources/mdtest/del.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -783,8 +783,8 @@ class A: ...
```py
from subexporter import *

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

__all__.append("B")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
```
Expand Down
10 changes: 5 additions & 5 deletions crates/ty_python_semantic/resources/mdtest/import/star.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand All @@ -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

Expand All @@ -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"]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 _():
Expand Down Expand Up @@ -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 _():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
6 changes: 3 additions & 3 deletions crates/ty_python_semantic/resources/mdtest/scopes/eager.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down Expand Up @@ -283,7 +283,7 @@ x = 1

def _():
class C:
# revealed: Unknown | Literal[1]
# revealed: Literal[1]
[reveal_type(x) for _ in [1]]
x = 2
```
Expand Down Expand Up @@ -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
```
Expand Down
6 changes: 3 additions & 3 deletions crates/ty_python_semantic/resources/mdtest/scopes/global.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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"]
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
64 changes: 30 additions & 34 deletions crates/ty_python_semantic/src/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -822,18 +822,37 @@ 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.
//
// However, 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 undeclared places in stubs and do not union with `Unknown`.
let in_stub_file = scope.file(db).is_stub(db);
Comment on lines +843 to +844
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that we can now remove this exception here, but we'll evaluate that in a separate PR, to make sure it does not have any effects on the ecosystem.


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()
}
}
Expand Down Expand Up @@ -1585,29 +1604,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::*;
Expand Down
Loading
Loading