Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] Remove Type::Unbound #13980

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ b: tuple[int] = (42,)
c: tuple[str, int] = ("42", 42)
d: tuple[tuple[str, str], tuple[int, int]] = (("foo", "foo"), (42, 42))
e: tuple[str, ...] = ()
# TODO: we should not emit this error
# error: [call-potentially-unbound-method] "Method `__class_getitem__` of type `Literal[tuple]` is potentially unbound"
f: tuple[str, *tuple[int, ...], bytes] = ("42", b"42")
g: tuple[str, Unpack[tuple[int, ...]], bytes] = ("42", b"42")
h: tuple[list[int], list[int]] = ([], [])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ class Foo:
return 42

f = Foo()
# error: [call-potentially-unbound-method] "Call to potentially unbound method `__iadd__` on object of type `Foo`"
f += "Hello, world!"

# TODO should emit a diagnostic warning that `Foo` might not have an `__iadd__` method
reveal_type(f) # revealed: int
```

Expand All @@ -100,6 +100,7 @@ class Foo:
return 42

f = Foo()
# error: [call-potentially-unbound-method] "Call to potentially unbound method `__iadd__` on object of type `Foo`"
f += "Hello, world!"

# TODO(charlie): This should be `int | str`, since `__iadd__` may be unbound.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,20 @@
x = foo # error: [unresolved-reference] "Name `foo` used when not defined"
foo = 1

# error: [unresolved-reference]
# revealed: Unbound
# No error `unresolved-reference` diagnostic is reported for `x`. This is
# desirable because we would get a lot of cascading errors even though there
# is only one root cause (the unbound variable `foo`).

# revealed: Unknown
reveal_type(x)
```

Note: in this particular example, one could argue that the most likely error would
be a wrong order of the `x`/`foo` definitions, and so it could be desirable to infer
`Literal[1]` for the type of `x`. On the other hand, there might be a variable `fob`
a little higher up in this file, and the actual error might have been just a typo.
Inferring `Unknown` thus seems like the safest option.

## Unbound class variable

Name lookups within a class scope fall back to globals, but lookups of class attributes don't.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ def bool_instance() -> bool:

if bool_instance() or (x := 1):
# error: [possibly-unresolved-reference]
reveal_type(x) # revealed: Unbound | Literal[1]
reveal_type(x) # revealed: Literal[1]

if bool_instance() and (x := 1):
# error: [possibly-unresolved-reference]
reveal_type(x) # revealed: Unbound | Literal[1]
reveal_type(x) # revealed: Literal[1]
```

## First expression is always evaluated
Expand All @@ -36,14 +36,14 @@ if (x := 1) and bool_instance():

```py
if True or (x := 1):
# TODO: infer that the second arm is never executed so type should be just "Unbound".
# TODO: infer that the second arm is never executed, and raise `unresolved-reference`.
# error: [possibly-unresolved-reference]
reveal_type(x) # revealed: Unbound | Literal[1]
reveal_type(x) # revealed: Literal[1]

if True and (x := 1):
# TODO: infer that the second arm is always executed so type should be just "Literal[1]".
# TODO: infer that the second arm is always executed, do not raise a diagnostic
# error: [possibly-unresolved-reference]
reveal_type(x) # revealed: Unbound | Literal[1]
reveal_type(x) # revealed: Literal[1]
```

## Later expressions can always use variables from earlier expressions
Expand All @@ -55,7 +55,7 @@ def bool_instance() -> bool:
bool_instance() or (x := 1) or reveal_type(x) # revealed: Literal[1]

# error: [unresolved-reference]
bool_instance() or reveal_type(y) or (y := 1) # revealed: Unbound
bool_instance() or reveal_type(y) or (y := 1) # revealed: Unknown
```

## Nested expressions
Expand All @@ -66,13 +66,13 @@ def bool_instance() -> bool:

if bool_instance() or ((x := 1) and bool_instance()):
# error: "Name `x` used when possibly not defined"
reveal_type(x) # revealed: Unbound | Literal[1]
reveal_type(x) # revealed: Literal[1]

if ((y := 1) and bool_instance()) or bool_instance():
reveal_type(y) # revealed: Literal[1]

# error: [possibly-unresolved-reference]
if (bool_instance() and (z := 1)) or reveal_type(z): # revealed: Unbound | Literal[1]
if (bool_instance() and (z := 1)) or reveal_type(z): # revealed: Literal[1]
# error: [possibly-unresolved-reference]
reveal_type(z) # revealed: Unbound | Literal[1]
reveal_type(z) # revealed: Literal[1]
```
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ x = y

reveal_type(x) # revealed: Literal[3, 4, 5]

# revealed: Unbound | Literal[2]
# revealed: Literal[2]
# error: [possibly-unresolved-reference]
reveal_type(r)

# revealed: Unbound | Literal[5]
# revealed: Literal[5]
# error: [possibly-unresolved-reference]
reveal_type(s)
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ match 0:
case 2:
y = 3

# revealed: Unbound | Literal[2, 3]
# revealed: Literal[2, 3]
# error: [possibly-unresolved-reference]
reveal_type(y)
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ except EXCEPTIONS as f:
## Dynamic exception types

```py
def foo(x: type[AttributeError], y: tuple[type[OSError], type[RuntimeError]], z: tuple[type[BaseException], ...]):
# TODO: we should not emit those errors `call-potentially-unbound-method` errors
def foo(
x: type[AttributeError],
y: tuple[type[OSError], type[RuntimeError]], # error: [call-potentially-unbound-method]
z: tuple[type[BaseException], ...], # error: [call-potentially-unbound-method]
):
try:
help()
except x as e:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ if flag:

x = y # error: [possibly-unresolved-reference]

# revealed: Unbound | Literal[3]
# error: [possibly-unresolved-reference]
# revealed: Literal[3]
reveal_type(x)

# revealed: Unbound | Literal[3]
# revealed: Literal[3]
# error: [possibly-unresolved-reference]
reveal_type(y)
```
Expand All @@ -40,11 +39,10 @@ if flag:
y: int = 3
x = y # error: [possibly-unresolved-reference]

# revealed: Unbound | Literal[3]
# error: [possibly-unresolved-reference]
# revealed: Literal[3]
reveal_type(x)

# revealed: Unbound | Literal[3]
# revealed: Literal[3]
# error: [possibly-unresolved-reference]
reveal_type(y)
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ async def foo():
async for x in Iterator():
pass

# TODO: should reveal `Unbound | Unknown` because `__aiter__` is not defined
# revealed: Unbound | @Todo
# TODO: should reveal `Unknown` because `__aiter__` is not defined
# revealed: @Todo
# error: [possibly-unresolved-reference]
reveal_type(x)
```
Expand All @@ -40,6 +40,6 @@ async def foo():
pass

# error: [possibly-unresolved-reference]
# revealed: Unbound | @Todo
# revealed: @Todo
reveal_type(x)
```
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class IntIterable:
for x in IntIterable():
pass

# revealed: Unbound | int
# revealed: int
# error: [possibly-unresolved-reference]
reveal_type(x)
```
Expand Down Expand Up @@ -87,7 +87,7 @@ class OldStyleIterable:
for x in OldStyleIterable():
pass

# revealed: Unbound | int
# revealed: int
# error: [possibly-unresolved-reference]
reveal_type(x)
```
Expand All @@ -98,7 +98,7 @@ reveal_type(x)
for x in (1, "a", b"foo"):
pass

# revealed: Unbound | Literal[1] | Literal["a"] | Literal[b"foo"]
# revealed: Literal[1] | Literal["a"] | Literal[b"foo"]
# error: [possibly-unresolved-reference]
reveal_type(x)
```
Expand All @@ -120,7 +120,7 @@ class NotIterable:
for x in NotIterable(): # error: "Object of type `NotIterable` is not iterable"
pass

# revealed: Unbound | Unknown
# revealed: Unknown
# error: [possibly-unresolved-reference]
reveal_type(x)
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ reveal_type(__path__) # revealed: @Todo

# TODO: this should probably be added to typeshed; not sure why it isn't?
# error: [unresolved-reference]
# revealed: Unbound
# revealed: Unknown
reveal_type(__doc__)

class X:
Expand All @@ -34,15 +34,15 @@ module globals; these are excluded:

```py path=unbound_dunders.py
# error: [unresolved-reference]
# revealed: Unbound
# revealed: Unknown
reveal_type(__getattr__)

# error: [unresolved-reference]
# revealed: Unbound
# revealed: Unknown
reveal_type(__dict__)

# error: [unresolved-reference]
# revealed: Unbound
# revealed: Unknown
reveal_type(__init__)
```

Expand All @@ -61,10 +61,10 @@ reveal_type(typing.__init__) # revealed: Literal[__init__]

# These come from `builtins.object`, not `types.ModuleType`:
# TODO: we don't currently understand `types.ModuleType` as inheriting from `object`;
# these should not reveal `Unbound`:
reveal_type(typing.__eq__) # revealed: Unbound
reveal_type(typing.__class__) # revealed: Unbound
reveal_type(typing.__module__) # revealed: Unbound
# these should not reveal `Unknown`:
reveal_type(typing.__eq__) # revealed: Unknown
reveal_type(typing.__class__) # revealed: Unknown
reveal_type(typing.__module__) # revealed: Unknown

# TODO: needs support for attribute access on instances, properties and generics;
# should be `dict[str, Any]`
Expand All @@ -78,7 +78,7 @@ where we know exactly which module we're dealing with:
```py path=__getattr__.py
import typing

reveal_type(typing.__getattr__) # revealed: Unbound
reveal_type(typing.__getattr__) # revealed: Unknown
```

## `types.ModuleType.__dict__` takes precedence over global variable `__dict__`
Expand Down Expand Up @@ -120,7 +120,7 @@ if returns_bool():
__name__ = 1

reveal_type(__file__) # revealed: Literal[42]
reveal_type(__name__) # revealed: str | Literal[1]
reveal_type(__name__) # revealed: Literal[1] | str
Copy link
Contributor Author

@sharkdp sharkdp Oct 30, 2024

Choose a reason for hiding this comment

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

To do: restore original order?

```

## Conditionally global or `ModuleType` attribute, with annotation
Expand All @@ -137,5 +137,5 @@ if returns_bool():
__name__: int = 1

reveal_type(__file__) # revealed: Literal[42]
reveal_type(__name__) # revealed: str | Literal[1]
reveal_type(__name__) # revealed: Literal[1] | str
```
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ if flag:
else:
class Spam: ...

# error: [call-non-callable] "Method `__class_getitem__` of type `Literal[__class_getitem__] | Unbound` is not callable on object of type `Literal[Spam, Spam]`"
# revealed: str | Unknown
# error: [call-potentially-unbound-method] "Method `__class_getitem__` of type `Literal[Spam, Spam]` is potentially unbound"
# revealed: str
reveal_type(Spam[42])
```

Expand Down
4 changes: 1 addition & 3 deletions crates/red_knot_python_semantic/src/semantic_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ pub mod expression;
pub mod symbol;
mod use_def;

pub(crate) use self::use_def::{
BindingWithConstraints, BindingWithConstraintsIterator, DeclarationsIterator,
};
pub(crate) use self::use_def::{BindingWithConstraints, DeclarationsIterator};

type SymbolMap = hashbrown::HashMap<ScopedSymbolId, (), FxBuildHasher>;

Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_python_semantic/src/semantic_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::module_name::ModuleName;
use crate::module_resolver::{resolve_module, Module};
use crate::semantic_index::ast_ids::HasScopedAstId;
use crate::semantic_index::semantic_index;
use crate::types::{binding_ty, global_symbol_ty, infer_scope_types, Type};
use crate::types::{binding_ty, global_symbol_ty, infer_scope_types, SymbolLookupResult, Type};
use crate::Db;

pub struct SemanticModel<'db> {
Expand Down Expand Up @@ -39,7 +39,7 @@ impl<'db> SemanticModel<'db> {
resolve_module(self.db, module_name)
}

pub fn global_symbol_ty(&self, module: &Module, symbol_name: &str) -> Type<'db> {
pub fn global_symbol_ty(&self, module: &Module, symbol_name: &str) -> SymbolLookupResult<'db> {
global_symbol_ty(self.db, module.file(), symbol_name)
}
}
Expand Down
Loading
Loading