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 all 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
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]
sharkdp marked this conversation as resolved.
Show resolved Hide resolved
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
sharkdp marked this conversation as resolved.
Show resolved Hide resolved
```

## Integration test: numbers from typeshed
Expand Down
55 changes: 41 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` | `T_declared \| T_inferred` | `Unknown \| T_inferred` |
| possibly-unbound | `T_declared` | `T_declared \| T_inferred` | `Unknown \| T_inferred` |
| unbound | `T_declared` | `T_declared` | `Unknown` |

| **Diagnostic** | declared | possibly-undeclared | undeclared |
| ---------------- | -------- | ------------------------- | ------------------- |
Expand Down Expand Up @@ -97,17 +102,24 @@ def flag() -> bool: ...

x = 1
y = 2
z = 3
if flag():
x: Any
x: int
y: Any
# error: [invalid-declaration]
y: str
z: str
```

```py
from mod import x, y
from mod import x, y, z

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

# External modifications of `x` that violate the declared type are not allowed:
# error: [invalid-assignment]
x = None
```

### Possibly undeclared and possibly unbound
Expand All @@ -134,6 +146,10 @@ from mod import x, y

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

# External modifications of `y` that violate the declared type are not allowed:
# error: [invalid-assignment]
y = None
```

### Possibly undeclared and unbound
Expand All @@ -154,22 +170,27 @@ if flag():
from mod import x

reveal_type(x) # revealed: int

# External modifications to `x` that violate the declared type are not allowed:
# error: [invalid-assignment]
x = None
```

## 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]

# All external modifications of `x` are allowed:
x = None
```

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

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

# All external modifications of `x` are allowed:
x = None
```

### Undeclared and unbound
Expand All @@ -206,4 +230,7 @@ if False:
from mod import x

reveal_type(x) # revealed: Unknown

# Modifications allowed in this case:
x = None
```
2 changes: 1 addition & 1 deletion 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 @@ -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 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]
sharkdp marked this conversation as resolved.
Show resolved Hide resolved
[[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: 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
sharkdp marked this conversation as resolved.
Show resolved Hide resolved

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
sharkdp marked this conversation as resolved.
Show resolved Hide resolved
```

### 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
6 changes: 3 additions & 3 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,7 +52,7 @@ 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(x) # revealed: Unknown | Literal[3]
reveal_type(y) # revealed: int
```

Expand Down
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
sharkdp marked this conversation as resolved.
Show resolved Hide resolved

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

if alias_for_type(x) is A:
reveal_type(x) # revealed: A
```
Expand Down
10 changes: 5 additions & 5 deletions crates/red_knot_python_semantic/resources/mdtest/scopes/nonlocal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
def f():
x = 1
def g():
reveal_type(x) # revealed: Literal[1]
reveal_type(x) # revealed: Unknown | Literal[1]
```

## Two levels up
Expand All @@ -16,7 +16,7 @@ def f():
x = 1
def g():
def h():
reveal_type(x) # revealed: Literal[1]
reveal_type(x) # revealed: Unknown | Literal[1]
```

## Skips class scope
Expand All @@ -28,7 +28,7 @@ def f():
class C:
x = 2
def g():
reveal_type(x) # revealed: Literal[1]
reveal_type(x) # revealed: Unknown | Literal[1]
```

## Skips annotation-only assignment
Expand All @@ -41,7 +41,7 @@ def f():
# name is otherwise not defined; maybe should be an error?
x: int
def h():
reveal_type(x) # revealed: Literal[1]
reveal_type(x) # revealed: Unknown | Literal[1]
```

## Implicit global in function
Expand All @@ -52,5 +52,5 @@ A name reference to a never-defined symbol in a function is implicitly a global
x = 1

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