Skip to content

Commit

Permalink
[red-knot] Meta data for Type::Todo (#14500)
Browse files Browse the repository at this point in the history
## Summary

Adds meta information to `Type::Todo`, allowing developers to easily
trace back the origin of a particular `@Todo` type they encounter.

Instead of `Type::Todo`, we now write either `type_todo!()` which
creates a `@Todo[path/to/source.rs:123]` type with file and line
information, or using `type_todo!("PEP 604 unions not supported")`,
which creates a variant with a custom message.

`Type::Todo` now contains a `TodoType` field. In release mode, this is
just a zero-sized struct, in order not to create any overhead. In debug
mode, this is an `enum` that contains the meta information.

`Type` implements `Copy`, which means that `TodoType` also needs to be
copyable. This limits the design space. We could intern `TodoType`, but
I discarded this option, as it would require us to have access to the
salsa DB everywhere we want to use `Type::Todo`. And it would have made
the macro invocations less ergonomic (requiring us to pass `db`).

So for now, the meta information is simply a `&'static str` / `u32` for
the file/line variant, or a `&'static str` for the custom message.
Anything involving a chain/backtrace of several `@Todo`s or similar is
therefore currently not implemented. Also because we currently don't see
any direct use cases for this, and because all of this will eventually
go away.

Note that the size of `Type` increases from 16 to 24 bytes, but only in
debug mode.

## Test Plan

- Observed the changes in Markdown tests.
- Added custom messages for all `Type::Todo`s that were revealed in the
tests
- Ran red knot in release and debug mode on the following Python file:
  ```py
  def f(x: int) -> int:
      reveal_type(x)
  ```
Prints `@Todo` in release mode and `@Todo(function parameter type)` in
debug mode.
  • Loading branch information
sharkdp authored Nov 21, 2024
1 parent aecdb8c commit 47f39ed
Show file tree
Hide file tree
Showing 24 changed files with 280 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ Ts = TypeVarTuple("Ts")

def append_int(*args: *Ts) -> tuple[*Ts, int]:
# TODO: should show some representation of the variadic generic type
reveal_type(args) # revealed: @Todo
reveal_type(args) # revealed: @Todo(function parameter type)

return (*args, 1)

# TODO should be tuple[Literal[True], Literal["a"], int]
reveal_type(append_int(True, "a")) # revealed: @Todo
reveal_type(append_int(True, "a")) # revealed: @Todo(full tuple[...] support)
```
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ reveal_type(c) # revealed: tuple[str, int]
reveal_type(d) # revealed: tuple[tuple[str, str], tuple[int, int]]

# TODO: homogenous tuples, PEP-646 tuples
reveal_type(e) # revealed: @Todo
reveal_type(f) # revealed: @Todo
reveal_type(g) # revealed: @Todo
reveal_type(e) # revealed: @Todo(full tuple[...] support)
reveal_type(f) # revealed: @Todo(full tuple[...] support)
reveal_type(g) # revealed: @Todo(full tuple[...] support)

# TODO: support more kinds of type expressions in annotations
reveal_type(h) # revealed: @Todo
reveal_type(h) # revealed: @Todo(full tuple[...] support)

reveal_type(i) # revealed: tuple[str | int, str | int]
reveal_type(j) # revealed: tuple[str | int]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,15 +317,15 @@ reveal_type(1 + A()) # revealed: int
reveal_type(A() + "foo") # revealed: A
# TODO should be `A` since `str.__add__` doesn't support `A` instances
# TODO overloads
reveal_type("foo" + A()) # revealed: @Todo
reveal_type("foo" + A()) # revealed: @Todo(return type)

reveal_type(A() + b"foo") # revealed: A
# TODO should be `A` since `bytes.__add__` doesn't support `A` instances
reveal_type(b"foo" + A()) # revealed: bytes

reveal_type(A() + ()) # revealed: A
# TODO this should be `A`, since `tuple.__add__` doesn't support `A` instances
reveal_type(() + A()) # revealed: @Todo
reveal_type(() + A()) # revealed: @Todo(return type)

literal_string_instance = "foo" * 1_000_000_000
# the test is not testing what it's meant to be testing if this isn't a `LiteralString`:
Expand All @@ -334,7 +334,7 @@ reveal_type(literal_string_instance) # revealed: LiteralString
reveal_type(A() + literal_string_instance) # revealed: A
# TODO should be `A` since `str.__add__` doesn't support `A` instances
# TODO overloads
reveal_type(literal_string_instance + A()) # revealed: @Todo
reveal_type(literal_string_instance + A()) # revealed: @Todo(return type)
```

## Operations involving instances of classes inheriting from `Any`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async def get_int_async() -> int:
return 42

# TODO: we don't yet support `types.CoroutineType`, should be generic `Coroutine[Any, Any, int]`
reveal_type(get_int_async()) # revealed: @Todo
reveal_type(get_int_async()) # revealed: @Todo(generic types.CoroutineType)
```

## Generic
Expand Down Expand Up @@ -44,7 +44,7 @@ def bar() -> str:
return "bar"

# TODO: should reveal `int`, as the decorator replaces `bar` with `foo`
reveal_type(bar()) # revealed: @Todo
reveal_type(bar()) # revealed: @Todo(return type)
```

## Invalid callable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ def foo(
help()
except x as e:
# TODO: should be `AttributeError`
reveal_type(e) # revealed: @Todo
reveal_type(e) # revealed: @Todo(exception type)
except y as f:
# TODO: should be `OSError | RuntimeError`
reveal_type(f) # revealed: @Todo
reveal_type(f) # revealed: @Todo(exception type)
except z as g:
# TODO: should be `BaseException`
reveal_type(g) # revealed: @Todo
reveal_type(g) # revealed: @Todo(exception type)
```
4 changes: 2 additions & 2 deletions crates/red_knot_python_semantic/resources/mdtest/generics.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ box: MyBox[int] = MyBox(5)
wrong_innards: MyBox[int] = MyBox("five")

# TODO reveal int
reveal_type(box.data) # revealed: @Todo
reveal_type(box.data) # revealed: @Todo(instance attributes)

reveal_type(MyBox.box_model_number) # revealed: Literal[695]
```
Expand All @@ -39,7 +39,7 @@ class MySecureBox[T](MyBox[T]): ...
secure_box: MySecureBox[int] = MySecureBox(5)
reveal_type(secure_box) # revealed: MySecureBox
# TODO reveal int
reveal_type(secure_box.data) # revealed: @Todo
reveal_type(secure_box.data) # revealed: @Todo(instance attributes)
```

## Cyclical class definition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ from other import Literal
a1: Literal[26]

def f():
reveal_type(a1) # revealed: @Todo
reveal_type(a1) # revealed: @Todo(generics)
```

## Detecting typing_extensions.Literal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ async def foo():
pass

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

# error: [possibly-unresolved-reference]
# revealed: @Todo
# revealed: @Todo(async iterables/iterators)
reveal_type(x)
```
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def f(*args, **kwargs) -> int: ...
class A(metaclass=f): ...

# TODO should be `type[int]`
reveal_type(A.__class__) # revealed: @Todo
reveal_type(A.__class__) # revealed: @Todo(metaclass not a class)
```

## Cyclic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ reveal_type(__doc__) # revealed: str | None
# (needs support for `*` imports)
reveal_type(__spec__) # revealed: Unknown | None

# TODO: generics
reveal_type(__path__) # revealed: @Todo
reveal_type(__path__) # revealed: @Todo(generics)

class X:
reveal_type(__name__) # revealed: str
Expand Down Expand Up @@ -64,7 +63,7 @@ reveal_type(typing.__class__) # revealed: Literal[type]

# TODO: needs support for attribute access on instances, properties and generics;
# should be `dict[str, Any]`
reveal_type(typing.__dict__) # revealed: @Todo
reveal_type(typing.__dict__) # revealed: @Todo(instance attributes)
```

Typeshed includes a fake `__getattr__` method in the stub for `types.ModuleType` to help out with
Expand Down Expand Up @@ -96,8 +95,8 @@ from foo import __dict__ as foo_dict

# TODO: needs support for attribute access on instances, properties, and generics;
# should be `dict[str, Any]` for both of these:
reveal_type(foo.__dict__) # revealed: @Todo
reveal_type(foo_dict) # revealed: @Todo
reveal_type(foo.__dict__) # revealed: @Todo(instance attributes)
reveal_type(foo_dict) # revealed: @Todo(instance attributes)
```

## Conditionally global or `ModuleType` attribute
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def int_instance() -> int:

a = b"abcde"[int_instance()]
# TODO: Support overloads... Should be `bytes`
reveal_type(a) # revealed: @Todo
reveal_type(a) # revealed: @Todo(return type)
```

## Slices
Expand All @@ -47,11 +47,11 @@ def int_instance() -> int: ...

byte_slice1 = b[int_instance() : int_instance()]
# TODO: Support overloads... Should be `bytes`
reveal_type(byte_slice1) # revealed: @Todo
reveal_type(byte_slice1) # revealed: @Todo(return type)

def bytes_instance() -> bytes: ...

byte_slice2 = bytes_instance()[0:5]
# TODO: Support overloads... Should be `bytes`
reveal_type(byte_slice2) # revealed: @Todo
reveal_type(byte_slice2) # revealed: @Todo(return type)
```
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ x = [1, 2, 3]
reveal_type(x) # revealed: list

# TODO reveal int
reveal_type(x[0]) # revealed: @Todo
reveal_type(x[0]) # revealed: @Todo(return type)

# TODO reveal list
reveal_type(x[0:1]) # revealed: @Todo
reveal_type(x[0:1]) # revealed: @Todo(return type)

# TODO error
reveal_type(x["a"]) # revealed: @Todo
reveal_type(x["a"]) # revealed: @Todo(return type)
```

## Assignments within list assignment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def int_instance() -> int: ...

a = "abcde"[int_instance()]
# TODO: Support overloads... Should be `str`
reveal_type(a) # revealed: @Todo
reveal_type(a) # revealed: @Todo(return type)
```

## Slices
Expand Down Expand Up @@ -78,13 +78,13 @@ def int_instance() -> int: ...

substring1 = s[int_instance() : int_instance()]
# TODO: Support overloads... Should be `LiteralString`
reveal_type(substring1) # revealed: @Todo
reveal_type(substring1) # revealed: @Todo(return type)

def str_instance() -> str: ...

substring2 = str_instance()[0:5]
# TODO: Support overloads... Should be `str`
reveal_type(substring2) # revealed: @Todo
reveal_type(substring2) # revealed: @Todo(return type)
```

## Unsupported slice types
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,5 @@ def int_instance() -> int: ...

tuple_slice = t[int_instance() : int_instance()]
# TODO: Support overloads... Should be `tuple[Literal[1, 'a', b"b"] | None, ...]`
reveal_type(tuple_slice) # revealed: @Todo
reveal_type(tuple_slice) # revealed: @Todo(return type)
```
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ properties on instance types:
```py path=b.py
import sys

reveal_type(sys.version_info.micro) # revealed: @Todo
reveal_type(sys.version_info.releaselevel) # revealed: @Todo
reveal_type(sys.version_info.serial) # revealed: @Todo
reveal_type(sys.version_info.micro) # revealed: @Todo(instance attributes)
reveal_type(sys.version_info.releaselevel) # revealed: @Todo(instance attributes)
reveal_type(sys.version_info.serial) # revealed: @Todo(instance attributes)
```

## Accessing fields by index/slice
Expand Down
22 changes: 11 additions & 11 deletions crates/red_knot_python_semantic/resources/mdtest/unpacking.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ reveal_type(b) # revealed: Literal[2]
[a, *b, c, d] = (1, 2)
reveal_type(a) # revealed: Literal[1]
# TODO: Should be list[Any] once support for assigning to starred expression is added
reveal_type(b) # revealed: @Todo
reveal_type(b) # revealed: @Todo(starred unpacking)
reveal_type(c) # revealed: Literal[2]
reveal_type(d) # revealed: Unknown
```
Expand All @@ -95,7 +95,7 @@ reveal_type(d) # revealed: Unknown
[a, *b, c] = (1, 2)
reveal_type(a) # revealed: Literal[1]
# TODO: Should be list[Any] once support for assigning to starred expression is added
reveal_type(b) # revealed: @Todo
reveal_type(b) # revealed: @Todo(starred unpacking)
reveal_type(c) # revealed: Literal[2]
```

Expand All @@ -105,7 +105,7 @@ reveal_type(c) # revealed: Literal[2]
[a, *b, c] = (1, 2, 3)
reveal_type(a) # revealed: Literal[1]
# TODO: Should be list[int] once support for assigning to starred expression is added
reveal_type(b) # revealed: @Todo
reveal_type(b) # revealed: @Todo(starred unpacking)
reveal_type(c) # revealed: Literal[3]
```

Expand All @@ -115,7 +115,7 @@ reveal_type(c) # revealed: Literal[3]
[a, *b, c, d] = (1, 2, 3, 4, 5, 6)
reveal_type(a) # revealed: Literal[1]
# TODO: Should be list[int] once support for assigning to starred expression is added
reveal_type(b) # revealed: @Todo
reveal_type(b) # revealed: @Todo(starred unpacking)
reveal_type(c) # revealed: Literal[5]
reveal_type(d) # revealed: Literal[6]
```
Expand All @@ -127,7 +127,7 @@ reveal_type(d) # revealed: Literal[6]
reveal_type(a) # revealed: Literal[1]
reveal_type(b) # revealed: Literal[2]
# TODO: Should be list[int] once support for assigning to starred expression is added
reveal_type(c) # revealed: @Todo
reveal_type(c) # revealed: @Todo(starred unpacking)
```

### Starred expression (6)
Expand All @@ -138,7 +138,7 @@ reveal_type(c) # revealed: @Todo
reveal_type(a) # revealed: Literal[1]
reveal_type(b) # revealed: Unknown
reveal_type(c) # revealed: Unknown
reveal_type(d) # revealed: @Todo
reveal_type(d) # revealed: @Todo(starred unpacking)
reveal_type(e) # revealed: Unknown
reveal_type(f) # revealed: Unknown
```
Expand Down Expand Up @@ -222,7 +222,7 @@ reveal_type(b) # revealed: LiteralString
(a, *b, c, d) = "ab"
reveal_type(a) # revealed: LiteralString
# TODO: Should be list[LiteralString] once support for assigning to starred expression is added
reveal_type(b) # revealed: @Todo
reveal_type(b) # revealed: @Todo(starred unpacking)
reveal_type(c) # revealed: LiteralString
reveal_type(d) # revealed: Unknown
```
Expand All @@ -233,7 +233,7 @@ reveal_type(d) # revealed: Unknown
(a, *b, c) = "ab"
reveal_type(a) # revealed: LiteralString
# TODO: Should be list[Any] once support for assigning to starred expression is added
reveal_type(b) # revealed: @Todo
reveal_type(b) # revealed: @Todo(starred unpacking)
reveal_type(c) # revealed: LiteralString
```

Expand All @@ -243,7 +243,7 @@ reveal_type(c) # revealed: LiteralString
(a, *b, c) = "abc"
reveal_type(a) # revealed: LiteralString
# TODO: Should be list[LiteralString] once support for assigning to starred expression is added
reveal_type(b) # revealed: @Todo
reveal_type(b) # revealed: @Todo(starred unpacking)
reveal_type(c) # revealed: LiteralString
```

Expand All @@ -253,7 +253,7 @@ reveal_type(c) # revealed: LiteralString
(a, *b, c, d) = "abcdef"
reveal_type(a) # revealed: LiteralString
# TODO: Should be list[LiteralString] once support for assigning to starred expression is added
reveal_type(b) # revealed: @Todo
reveal_type(b) # revealed: @Todo(starred unpacking)
reveal_type(c) # revealed: LiteralString
reveal_type(d) # revealed: LiteralString
```
Expand All @@ -265,5 +265,5 @@ reveal_type(d) # revealed: LiteralString
reveal_type(a) # revealed: LiteralString
reveal_type(b) # revealed: LiteralString
# TODO: Should be list[int] once support for assigning to starred expression is added
reveal_type(c) # revealed: @Todo
reveal_type(c) # revealed: @Todo(starred unpacking)
```
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ class Manager:

async def test():
async with Manager() as f:
reveal_type(f) # revealed: @Todo
reveal_type(f) # revealed: @Todo(async with statement)
```
Loading

0 comments on commit 47f39ed

Please sign in to comment.