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.

Sign up for GitHub

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] Use Unknown | T_inferred for undeclared public symbols #15674

Merged
merged 13 commits into from
Jan 24, 2025
Merged

[red-knot] Use Unknown | T_inferred for undeclared public symbols #15674

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f7f3d92
[red-knot] Use `Unknown | T_inferred` for undeclared public symbols
sharkdp Jan 22, 2025
9bf84e5
Extend description
sharkdp Jan 23, 2025
7a035b7
Adapt 'Public type' table
sharkdp Jan 23, 2025
5bf3c9b
Use helper to apply the same restrictions to instance_member
sharkdp Jan 23, 2025
49bcf53
Add TODO comment
sharkdp Jan 23, 2025
f29add5
Minor change in imports
sharkdp Jan 23, 2025
36b3af6
Add new test for '__slots__' modifications
sharkdp Jan 23, 2025
f4ca4b8
Union with Unknown for *possibly* undeclared types
sharkdp Jan 23, 2025
a1c5647
Revert change in symbol.rs
sharkdp Jan 23, 2025
1a1e08a
Revert possibly-undeclared changes
sharkdp Jan 24, 2025
edb295e
Add tests for modifications of external symbols
sharkdp Jan 24, 2025
96e8ae9
Add TODO
sharkdp Jan 24, 2025
f10e976
Clarifying comments
sharkdp Jan 24, 2025
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/red_knot_python_semantic/resources/mdtest/annotations/literal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def f():
reveal_type(a7) # revealed: None
reveal_type(a8) # revealed: Literal[1]
# TODO: This should be Color.RED
reveal_type(b1) # revealed: Literal[0]
reveal_type(b1) # revealed: Unknown | Literal[0]

# error: [invalid-type-form]
invalid1: Literal[3 + 4]
Expand Down
8 changes: 4 additions & 4 deletions crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def _(flag: bool):
# that `Foo.__iadd__` may be unbound as additional context.
f += "Hello, world!"

reveal_type(f) # revealed: int | Unknown
reveal_type(f) # revealed: Unknown | int
```

## Partially bound with `__add__`
Expand All @@ -96,7 +96,7 @@ def _(flag: bool):
f = Foo()
f += "Hello, world!"

reveal_type(f) # revealed: int | str
reveal_type(f) # revealed: Unknown | int | str
```

## Partially bound target union
Expand All @@ -116,7 +116,7 @@ def _(flag1: bool, flag2: bool):
f = 42.0
f += 12

reveal_type(f) # revealed: int | str | float
reveal_type(f) # revealed: Unknown | int | str | float
```

## Target union
Expand Down Expand Up @@ -160,5 +160,5 @@ def f(flag: bool, flag2: bool):
f = Bar()
f += 12

reveal_type(f) # revealed: int | str | float
reveal_type(f) # revealed: Unknown | int | str | float
```
17 changes: 8 additions & 9 deletions crates/red_knot_python_semantic/resources/mdtest/attributes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class C:

reveal_type(C.pure_class_variable1) # revealed: str

# TODO: this should be `Literal[1]`, or `Unknown | Literal[1]`.
# TODO: Should be `Unknown | Literal[1]`.
reveal_type(C.pure_class_variable2) # revealed: Unknown

c_instance = C()
Expand Down Expand Up @@ -252,8 +252,7 @@ class C:

reveal_type(C.variable_with_class_default1) # revealed: str

# TODO: this should be `Unknown | Literal[1]`.
reveal_type(C.variable_with_class_default2) # revealed: Literal[1]
reveal_type(C.variable_with_class_default2) # revealed: Unknown | Literal[1]

c_instance = C()

Expand Down Expand Up @@ -296,8 +295,8 @@ def _(flag: bool):
else:
x = 4

reveal_type(C1.x) # revealed: Literal[1, 2]
reveal_type(C2.x) # revealed: Literal[3, 4]
reveal_type(C1.x) # revealed: Unknown | Literal[1, 2]
reveal_type(C2.x) # revealed: Unknown | Literal[3, 4]
```

## Inherited class attributes
Expand All @@ -311,7 +310,7 @@ class A:
class B(A): ...
class C(B): ...

reveal_type(C.X) # revealed: Literal["foo"]
reveal_type(C.X) # revealed: Unknown | Literal["foo"]
```

### Multiple inheritance
Expand All @@ -334,7 +333,7 @@ class A(B, C): ...
reveal_type(A.__mro__)

# `E` is earlier in the MRO than `F`, so we should use the type of `E.X`
reveal_type(A.X) # revealed: Literal[42]
reveal_type(A.X) # revealed: Unknown | Literal[42]
```

## Unions with possibly unbound paths
Expand All @@ -356,7 +355,7 @@ def _(flag1: bool, flag2: bool):
C = C1 if flag1 else C2 if flag2 else C3

# error: [possibly-unbound-attribute] "Attribute `x` on type `Literal[C1, C2, C3]` is possibly unbound"
reveal_type(C.x) # revealed: Literal[1, 3]
reveal_type(C.x) # revealed: Unknown | Literal[1, 3]
```

### Possibly-unbound within a class
Expand All @@ -379,7 +378,7 @@ def _(flag: bool, flag1: bool, flag2: bool):
C = C1 if flag1 else C2 if flag2 else C3

# error: [possibly-unbound-attribute] "Attribute `x` on type `Literal[C1, C2, C3]` is possibly unbound"
reveal_type(C.x) # revealed: Literal[1, 2, 3]
reveal_type(C.x) # revealed: Unknown | Literal[1, 2, 3]
```

### Unions with all paths unbound
Expand Down
3 changes: 2 additions & 1 deletion crates/red_knot_python_semantic/resources/mdtest/binary/instances.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ class A:
class B:
__add__ = A()

reveal_type(B() + B()) # revealed: int
# TODO: this could be `int` if we declare `B.__add__` using a `Callable` type
reveal_type(B() + B()) # revealed: Unknown | int
```

## Integration test: numbers from typeshed
Expand Down
31 changes: 17 additions & 14 deletions crates/red_knot_python_semantic/resources/mdtest/boundness_declaredness/public.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,23 @@ that is, a use of a symbol from another scope. If a symbol has a declared type i
(e.g. `int`), we use that as the symbol's "public type" (the type of the symbol from the perspective
of other scopes) even if there is a more precise local inferred type for the symbol (`Literal[1]`).

If a symbol has no declared type, we use the union of `Unknown` with the inferred type as the public
type. If there is no declaration, then the symbol can be reassigned to any type from another scope;
the union with `Unknown` reflects that its type must at least be as large as the type of the
assigned value, but could be arbitrarily larger.

We test the whole matrix of possible boundness and declaredness states. The current behavior is
summarized in the following table, while the tests below demonstrate each case. Note that some of
this behavior is questionable and might change in the future. See the TODOs in `symbol_by_id`
(`types.rs`) and [this issue](https://github.com/astral-sh/ruff/issues/14297) for more information.
In particular, we should raise errors in the "possibly-undeclared-and-unbound" as well as the
"undeclared-and-possibly-unbound" cases (marked with a "?").

| **Public type** | declared | possibly-undeclared | undeclared |
| ---------------- | ------------ | -------------------------- | ------------ |
| bound | `T_declared` | `T_declared \| T_inferred` | `T_inferred` |
| possibly-unbound | `T_declared` | `T_declared \| T_inferred` | `T_inferred` |
| unbound | `T_declared` | `T_declared` | `Unknown` |
| **Public type** | declared | possibly-undeclared | undeclared |
| ---------------- | ------------ | ------------------------------------- | ----------------------- |
| bound | `T_declared` | `Unknown \| T_declared \| T_inferred` | `Unknown \| T_inferred` |
| possibly-unbound | `T_declared` | `Unknown \| T_declared \| T_inferred` | `Unknown \| T_inferred` |
| unbound | `T_declared` | `Unknown \| T_declared` | `Unknown` |

| **Diagnostic** | declared | possibly-undeclared | undeclared |
| ---------------- | -------- | ------------------------- | ------------------- |
Expand Down Expand Up @@ -106,8 +111,8 @@ if flag():
```py
from mod import x, y

reveal_type(x) # revealed: Literal[1] | Any
reveal_type(y) # revealed: Literal[2] | Unknown
reveal_type(x) # revealed: Unknown | Literal[1] | Any
reveal_type(y) # revealed: Unknown | Literal[2]
```

### Possibly undeclared and possibly unbound
Expand All @@ -132,8 +137,8 @@ else:
# error: [possibly-unbound-import]
from mod import x, y

reveal_type(x) # revealed: Literal[1] | Any
reveal_type(y) # revealed: Literal[2] | str
reveal_type(x) # revealed: Unknown | Literal[1] | Any
reveal_type(y) # revealed: Unknown | Literal[2] | str
```

### Possibly undeclared and unbound
Expand All @@ -153,23 +158,21 @@ if flag():
# on top of this document.
from mod import x

reveal_type(x) # revealed: int
reveal_type(x) # revealed: Unknown | int
```

## Undeclared

### Undeclared but bound

We use the inferred type as the public type, if a symbol has no declared type.

```py path=mod.py
x = 1
```

```py
from mod import x

reveal_type(x) # revealed: Literal[1]
reveal_type(x) # revealed: Unknown | Literal[1]
```

### Undeclared and possibly unbound
Expand All @@ -189,7 +192,7 @@ if flag:
# on top of this document.
from mod import x

reveal_type(x) # revealed: Literal[1]
reveal_type(x) # revealed: Unknown | Literal[1]
```

### Undeclared and unbound
Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/resources/mdtest/call/callable_instance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def _(flag: bool):

a = PossiblyNotCallable()
result = a() # error: "Object of type `PossiblyNotCallable` is not callable (possibly unbound `__call__` method)"
reveal_type(result) # revealed: int
reveal_type(result) # revealed: Unknown | int
```

## Possibly unbound callable
Expand All @@ -52,7 +52,7 @@ class NonCallable:
__call__ = 1

a = NonCallable()
# error: "Object of type `NonCallable` is not callable"
# error: "Object of type `Unknown | Literal[1]` is not callable (due to union element `Literal[1]`)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another instance of the callable problem. Notice how the usefulness of the error message degrades.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is really begging for nested diagnostics (so you get both "Object of type NonCallable is not callable" and nested details explaining why.)

We could easily adjust the handling so you still get "Object of type NonCallable is not callable" here, but then you lose the details of which union element failed callability.

Even without nested diagnostics we could still provide all of the information here in a new error, it just requires more and more bespoke error-cases handling in the __call__ lookup code. (We would need to match on the case that callability of __call__ failed due to a union, and then write a new custom error message for that case which mentions both the outer NonCallable type and the details of why its __call__ isn't callable.) Nested diagnostics would just let us handle this kind of situation more generically, with less special casing.

cc @BurntSushi @MichaReiser re diagnostics considerations

reveal_type(a()) # revealed: Unknown
```

Expand All @@ -67,7 +67,7 @@ def _(flag: bool):
def __call__(self) -> int: ...

a = NonCallable()
# error: "Object of type `Literal[1] | Literal[__call__]` is not callable (due to union element `Literal[1]`)"
# error: "Object of type `Unknown | Literal[1] | Literal[__call__]` is not callable (due to union element `Literal[1]`)"
reveal_type(a()) # revealed: Unknown | int
```

Expand Down
6 changes: 4 additions & 2 deletions crates/red_knot_python_semantic/resources/mdtest/comprehensions/basic.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class IntIterable:
def __iter__(self) -> IntIterator:
return IntIterator()

# revealed: tuple[int, int]
# TODO: This could be a `tuple[int, int]` if we model that `y` can not be modified in the outer comprehension scope
# revealed: tuple[int, Unknown | int]
[[reveal_type((x, y)) for x in IntIterable()] for y in IntIterable()]
```

Expand All @@ -66,7 +67,8 @@ class IterableOfIterables:
def __iter__(self) -> IteratorOfIterables:
return IteratorOfIterables()

# revealed: tuple[int, IntIterable]
# TODO: This could be a `tuple[int, int]` (see above)
# revealed: tuple[int, Unknown | IntIterable]
[[reveal_type((x, y)) for x in y] for y in IterableOfIterables()]
```

Expand Down
17 changes: 12 additions & 5 deletions crates/red_knot_python_semantic/resources/mdtest/expression/attribute.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,29 @@
```py
def _(flag: bool):
class A:
always_bound = 1
always_bound: int = 1

if flag:
union = 1
else:
union = "abc"

if flag:
possibly_unbound = "abc"
union_declared: int = 1
else:
union_declared: str = "abc"

if flag:
possibly_unbound: str = "abc"

reveal_type(A.always_bound) # revealed: int

reveal_type(A.always_bound) # revealed: Literal[1]
reveal_type(A.union) # revealed: Unknown | Literal[1, "abc"]

reveal_type(A.union) # revealed: Literal[1, "abc"]
reveal_type(A.union_declared) # revealed: int | str

# error: [possibly-unbound-attribute] "Attribute `possibly_unbound` on type `Literal[A]` is possibly unbound"
reveal_type(A.possibly_unbound) # revealed: Literal["abc"]
reveal_type(A.possibly_unbound) # revealed: Unknown | str

# error: [unresolved-attribute] "Type `Literal[A]` has no attribute `non_existent`"
reveal_type(A.non_existent) # revealed: Unknown
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/resources/mdtest/expression/boolean.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ reveal_type("x" or "y" and "") # revealed: Literal["x"]
## Evaluates to builtin

```py path=a.py
redefined_builtin_bool = bool
redefined_builtin_bool: type[bool] = bool

def my_bool(x) -> bool:
return True
Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_python_semantic/resources/mdtest/expression/len.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,10 @@ class IntUnion:
def __len__(self) -> Literal[SomeEnum.INT, SomeEnum.INT_2]: ...

reveal_type(len(Auto())) # revealed: int
reveal_type(len(Int())) # revealed: Literal[2]
reveal_type(len(Int())) # revealed: int
reveal_type(len(Str())) # revealed: int
reveal_type(len(Tuple())) # revealed: int
reveal_type(len(IntUnion())) # revealed: Literal[2, 32]
reveal_type(len(IntUnion())) # revealed: int
```

### Negative integers
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/resources/mdtest/generics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ wrong_innards: MyBox[int] = MyBox("five")
# TODO reveal int, do not leak the typevar
reveal_type(box.data) # revealed: T

reveal_type(MyBox.box_model_number) # revealed: Literal[695]
reveal_type(MyBox.box_model_number) # revealed: Unknown | Literal[695]
```

## Subclassing
Expand Down
12 changes: 6 additions & 6 deletions crates/red_knot_python_semantic/resources/mdtest/import/conditional.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ reveal_type(y)
# error: [possibly-unbound-import] "Member `y` of module `maybe_unbound` is possibly unbound"
from maybe_unbound import x, y

reveal_type(x) # revealed: Literal[3]
reveal_type(y) # revealed: Literal[3]
reveal_type(x) # revealed: Unknown | Literal[3]
reveal_type(y) # revealed: Unknown | Literal[3]
```

## Maybe unbound annotated
Expand Down Expand Up @@ -52,8 +52,8 @@ Importing an annotated name prefers the declared type over the inferred type:
# error: [possibly-unbound-import] "Member `y` of module `maybe_unbound_annotated` is possibly unbound"
from maybe_unbound_annotated import x, y

reveal_type(x) # revealed: Literal[3]
reveal_type(y) # revealed: int
reveal_type(x) # revealed: Unknown | Literal[3]
reveal_type(y) # revealed: Unknown | int
```

## Maybe undeclared
Expand All @@ -71,7 +71,7 @@ if coinflip():
```py
from maybe_undeclared import x

reveal_type(x) # revealed: int
reveal_type(x) # revealed: Unknown | int
```

## Reimport
Expand Down Expand Up @@ -119,5 +119,5 @@ else:
```py
from b import x

reveal_type(x) # revealed: int
reveal_type(x) # revealed: Unknown | int
```
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/resources/mdtest/loops/for.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ reveal_type(x)
def _(flag: bool):
class NotIterable:
if flag:
__iter__ = 1
__iter__: int = 1
else:
__iter__ = None
__iter__: None = None

for x in NotIterable(): # error: "Object of type `NotIterable` is not iterable"
pass
Expand All @@ -135,7 +135,7 @@ for x in nonsense: # error: "Object of type `Literal[123]` is not iterable"
class NotIterable:
def __getitem__(self, key: int) -> int:
return 42
__iter__ = None
__iter__: None = None

for x in NotIterable(): # error: "Object of type `NotIterable` is not iterable"
pass
Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_python_semantic/resources/mdtest/narrow/type.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ def _(x: str | int):
class A: ...
class B: ...

alias_for_type = type

def _(x: A | B):
alias_for_type = type

if alias_for_type(x) is A:
reveal_type(x) # revealed: A
```
Expand Down
Loading
Loading