Skip to content

[ty] highlight the relevant argument in type assert error messages #19426

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

Merged
merged 4 commits into from
Jul 23, 2025
Merged

[ty] highlight the relevant argument in type assert error messages #19426

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4163fd8
highlight the relevant argument in type assert error messages
oconnor663 Jul 18, 2025
55e01da
revert changes to `assert_never`, `assert_type`, and `cast`
oconnor663 Jul 23, 2025
46591a9
make the `static_assert` error use a secondary annotation
oconnor663 Jul 23, 2025
6fb7eeb
move static_assert snapshots to type_api.md
oconnor663 Jul 23, 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
14 changes: 14 additions & 0 deletions crates/ty_python_semantic/resources/mdtest/directives/cast.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# `cast`

## Behavior

`cast()` takes two arguments, one type and one value, and returns a value of the given type.

The (inferred) type of the value and the given type do not need to have any correlation.
Expand Down Expand Up @@ -78,3 +80,15 @@ def f(x: Any, y: Unknown, z: Any | str | int):

e = cast(str | int | Any, z) # error: [redundant-cast]
```

## The redundant `cast` diagnostic

<!-- snapshot-diagnostics -->

```py
import secrets
from typing import cast

# error: [redundant-cast] "Value is already of type `int`"
cast(int, secrets.randbelow(10))
```
29 changes: 29 additions & 0 deletions crates/ty_python_semantic/resources/mdtest/directives/static_assert.md
View file
Open in desktop
Copy link
Contributor Author

@oconnor663 oconnor663 Jul 18, 2025

Choose a reason for hiding this comment

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

Is this the right place for this new mdtest file?

Copy link
Contributor

@carljm carljm Jul 18, 2025

Choose a reason for hiding this comment

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

I think it could go into type_api.md with the existing tests for static_assert, or if we want to keep it separate because it's all about the diagnostics rather than the functionality, then it could go somewhere under diagnostics?

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# `static_assert`

## Diagnostics

<!-- snapshot-diagnostics -->

```py
from ty_extensions import static_assert
import secrets

# a passing assert
static_assert(1 < 2)

# evaluates to False
# error: [static-assert-error]
static_assert(1 > 2)

# evaluates to False, with a message as the second argument
# error: [static-assert-error]
static_assert(1 > 2, "with a message")

# evaluates to something falsey
# error: [static-assert-error]
static_assert("")

# evaluates to something ambiguous
# error: [static-assert-error]
static_assert(secrets.randbelow(2))
```
54 changes: 18 additions & 36 deletions ...ert_never.md_-_`assert_never`_-_Basic_functionality_-_Diagnostics_(be8f5d8b0718ee54).snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,11 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/directives/assert_never.

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:5:5
--> src/mdtest_snippet.py:5:18
|
4 | def _():
5 | assert_never(0) # error: [type-assertion-failure]
| ^^^^^^^^^^^^^-^
| |
| Inferred type of argument is `Literal[0]`
| ^ Inferred type is `Literal[0]`
6 |
7 | def _():
|
Expand All @@ -64,13 +62,11 @@ info: rule `type-assertion-failure` is enabled by default

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:8:5
--> src/mdtest_snippet.py:8:18
|
7 | def _():
8 | assert_never("") # error: [type-assertion-failure]
| ^^^^^^^^^^^^^--^
| |
| Inferred type of argument is `Literal[""]`
| ^^ Inferred type is `Literal[""]`
9 |
10 | def _():
|
Expand All @@ -81,13 +77,11 @@ info: rule `type-assertion-failure` is enabled by default

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:11:5
--> src/mdtest_snippet.py:11:18
|
10 | def _():
11 | assert_never(None) # error: [type-assertion-failure]
| ^^^^^^^^^^^^^----^
| |
| Inferred type of argument is `None`
| ^^^^ Inferred type is `None`
12 |
13 | def _():
|
Expand All @@ -98,13 +92,11 @@ info: rule `type-assertion-failure` is enabled by default

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:14:5
--> src/mdtest_snippet.py:14:18
|
13 | def _():
14 | assert_never([]) # error: [type-assertion-failure]
| ^^^^^^^^^^^^^--^
| |
| Inferred type of argument is `list[Unknown]`
| ^^ Inferred type is `list[Unknown]`
15 |
16 | def _():
|
Expand All @@ -115,13 +107,11 @@ info: rule `type-assertion-failure` is enabled by default

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:17:5
--> src/mdtest_snippet.py:17:18
|
16 | def _():
17 | assert_never({}) # error: [type-assertion-failure]
| ^^^^^^^^^^^^^--^
| |
| Inferred type of argument is `dict[Unknown, Unknown]`
| ^^ Inferred type is `dict[Unknown, Unknown]`
18 |
19 | def _():
|
Expand All @@ -132,13 +122,11 @@ info: rule `type-assertion-failure` is enabled by default

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:20:5
--> src/mdtest_snippet.py:20:18
|
19 | def _():
20 | assert_never(()) # error: [type-assertion-failure]
| ^^^^^^^^^^^^^--^
| |
| Inferred type of argument is `tuple[()]`
| ^^ Inferred type is `tuple[()]`
21 |
22 | def _(flag: bool, never: Never):
|
Expand All @@ -149,13 +137,11 @@ info: rule `type-assertion-failure` is enabled by default

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:23:5
--> src/mdtest_snippet.py:23:18
|
22 | def _(flag: bool, never: Never):
23 | assert_never(1 if flag else never) # error: [type-assertion-failure]
| ^^^^^^^^^^^^^--------------------^
| |
| Inferred type of argument is `Literal[1]`
| ^^^^^^^^^^^^^^^^^^^^ Inferred type is `Literal[1]`
24 |
25 | def _(any_: Any):
|
Expand All @@ -166,13 +152,11 @@ info: rule `type-assertion-failure` is enabled by default

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:26:5
--> src/mdtest_snippet.py:26:18
|
25 | def _(any_: Any):
26 | assert_never(any_) # error: [type-assertion-failure]
| ^^^^^^^^^^^^^----^
| |
| Inferred type of argument is `Any`
| ^^^^ Inferred type is `Any`
27 |
28 | def _(unknown: Unknown):
|
Expand All @@ -183,13 +167,11 @@ info: rule `type-assertion-failure` is enabled by default

```
error[type-assertion-failure]: Argument does not have asserted type `Never`
--> src/mdtest_snippet.py:29:5
--> src/mdtest_snippet.py:29:18
|
28 | def _(unknown: Unknown):
29 | assert_never(unknown) # error: [type-assertion-failure]
| ^^^^^^^^^^^^^-------^
| |
| Inferred type of argument is `Unknown`
| ^^^^^^^ Inferred type is `Unknown`
|
info: `Never` and `Unknown` are not equivalent types
info: rule `type-assertion-failure` is enabled by default
Expand Down
6 changes: 2 additions & 4 deletions ...resources/mdtest/snapshots/assert_type.md_-_`assert_type`_-_Basic_(c507788da2659ec9).snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,12 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/directives/assert_type.m

```
error[type-assertion-failure]: Argument does not have asserted type `str`
--> src/mdtest_snippet.py:5:5
--> src/mdtest_snippet.py:5:17
|
3 | def _(x: int):
4 | assert_type(x, int) # fine
5 | assert_type(x, str) # error: [type-assertion-failure]
| ^^^^^^^^^^^^-^^^^^^
| |
| Inferred type of argument is `int`
| ^ Inferred type is `int`
|
info: `str` and `int` are not equivalent types
info: rule `type-assertion-failure` is enabled by default
Expand Down
34 changes: 34 additions & 0 deletions ...sources/mdtest/snapshots/cast.md_-_`cast`_-_The_redundant_`cast`…_(7746465d25c4280a).snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: cast.md - `cast` - The redundant `cast` diagnostic
mdtest path: crates/ty_python_semantic/resources/mdtest/directives/cast.md
---

# Python source files

## mdtest_snippet.py

```
1 | import secrets
2 | from typing import cast
3 |
4 | # error: [redundant-cast] "Value is already of type `int`"
5 | cast(int, secrets.randbelow(10))
```

# Diagnostics

```
warning[redundant-cast]: Value is already of type `int`
--> src/mdtest_snippet.py:5:11
|
4 | # error: [redundant-cast] "Value is already of type `int`"
Copy link
Member

@AlexWaygood AlexWaygood Jul 19, 2025

Choose a reason for hiding this comment

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

I think this means that for a multiline cast() call, you'd have to have a type: ignore comment very precisely located to suppress the redundant-cast diagnostic. E.g. this would suppress the diagnostic:

cast(
    int,
    very_long_variable_name_splitting_the_call_over_several_lines,  # type: ignore
)

But this wouldn't, since the range of the suppression comment wouldn't overlap with the primary range of the diagnostic:

cast(  # type: ignore
    int,
    very_long_variable_name_splitting_the_call_over_several_lines,
)

I think that would be very confusing for users, so for this reason, in #18050 (regarding the same issue, but for different diagnostics) I went with a slightly different approach of highlighting the call-arguments range with a secondary annotation but keeping the range of the entire call expression as the diagnostic's range. Some discussion in #18050 (comment)

In the long term, I think it would be great if we could disentangle the concepts of "primary range to be highlighted in diagnostics" and "range in which suppression comments are deemed to apply" so that our diagnostic model doesn't require them to always be the same. But that's not something for this PR!

Copy link
Member

@AlexWaygood AlexWaygood Jul 19, 2025

Choose a reason for hiding this comment

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

For the same reasons, I'm not sure we should make any changes to assert_type diagnostics in this PR: we already discussed this issue w.r.t. assert_type diagnostics in #18050 and the changes made in that PR were basically the best we thought we could do without reworking the diagnostics model to disentangle the concepts of "diagnostic suppression range" and "range displayed when the diagnostic is rendered"

5 | cast(int, secrets.randbelow(10))
| ^^^^^^^^^^^^^^^^^^^^^ Inferred type is already `int`
|
info: rule `redundant-cast` is enabled by default

```
96 changes: 96 additions & 0 deletions ...mdtest/snapshots/static_assert.md_-_`static_assert`_-_Diagnostics_(b195c06ae1c68ef3).snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: static_assert.md - `static_assert` - Diagnostics
mdtest path: crates/ty_python_semantic/resources/mdtest/directives/static_assert.md
---

# Python source files

## mdtest_snippet.py

```
1 | from ty_extensions import static_assert
2 | import secrets
3 |
4 | # a passing assert
5 | static_assert(1 < 2)
6 |
7 | # evaluates to False
8 | # error: [static-assert-error]
9 | static_assert(1 > 2)
10 |
11 | # evaluates to False, with a message as the second argument
12 | # error: [static-assert-error]
13 | static_assert(1 > 2, "with a message")
14 |
15 | # evaluates to something falsey
16 | # error: [static-assert-error]
17 | static_assert("")
18 |
19 | # evaluates to something ambiguous
20 | # error: [static-assert-error]
21 | static_assert(secrets.randbelow(2))
```

# Diagnostics

```
error[static-assert-error]: Static assertion error: argument evaluates to `False`
--> src/mdtest_snippet.py:9:15
|
7 | # evaluates to False
8 | # error: [static-assert-error]
9 | static_assert(1 > 2)
| ^^^^^ Inferred type is `Literal[False]`
10 |
11 | # evaluates to False, with a message as the second argument
|
info: rule `static-assert-error` is enabled by default

```

```
error[static-assert-error]: Static assertion error: with a message
--> src/mdtest_snippet.py:13:15
|
11 | # evaluates to False, with a message as the second argument
12 | # error: [static-assert-error]
13 | static_assert(1 > 2, "with a message")
| ^^^^^ Inferred type is `Literal[False]`
14 |
15 | # evaluates to something falsey
|
info: rule `static-assert-error` is enabled by default

```

```
error[static-assert-error]: Static assertion error: argument of type `Literal[""]` is statically known to be falsy
--> src/mdtest_snippet.py:17:15
|
15 | # evaluates to something falsey
16 | # error: [static-assert-error]
17 | static_assert("")
| ^^ Inferred type is `Literal[""]`
18 |
19 | # evaluates to something ambiguous
|
info: rule `static-assert-error` is enabled by default

```

```
error[static-assert-error]: Static assertion error: argument of type `int` has an ambiguous static truthiness
--> src/mdtest_snippet.py:21:15
|
19 | # evaluates to something ambiguous
20 | # error: [static-assert-error]
21 | static_assert(secrets.randbelow(2))
| ^^^^^^^^^^^^^^^^^^^^ Inferred type is `int`
|
info: rule `static-assert-error` is enabled by default

```
Loading
Loading