[ty] Improve diagnostics for invalid exceptions (#21475)
Some checks are pending
CI / cargo fmt (push) Waiting to run
CI / Determine changes (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (${{ github.repository == 'astral-sh/ruff' && 'depot-windows-2022-16' || 'windows-latest' }}) (push) Blocked by required conditions
CI / cargo test (macos-latest) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / ty completion evaluation (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks instrumented (ruff) (push) Blocked by required conditions
CI / benchmarks instrumented (ty) (push) Blocked by required conditions
CI / benchmarks walltime (medium|multithreaded) (push) Blocked by required conditions
CI / benchmarks walltime (small|large) (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run

## Summary

Not a high-priority task... but it _is_ a weekend :P

This PR improves our diagnostics for invalid exceptions. Specifically:
- We now give a special-cased ``help: Did you mean
`NotImplementedError`` subdiagnostic for `except NotImplemented`, `raise
NotImplemented` and `raise <EXCEPTION> from NotImplemented`
- If the user catches a tuple of exceptions (`except (foo, bar, baz):`)
and multiple elements in the tuple are invalid, we now collect these
into a single diagnostic rather than emitting a separate diagnostic for
each tuple element
- The explanation of why the `except`/`raise` was invalid ("must be a
`BaseException` instance or `BaseException` subclass", etc.) is
relegated to a subdiagnostic. This makes the top-level diagnostic
summary much more concise.

## Test Plan

Lots of snapshots. And here's some screenshots:

<details>
<summary>Screenshots</summary>

<img width="1770" height="1520" alt="image"
src="https://github.com/user-attachments/assets/7f27fd61-c74d-4ddf-ad97-ea4fd24d06fd"
/>

<img width="1916" height="1392" alt="image"
src="https://github.com/user-attachments/assets/83e5027c-8798-48a6-a0ec-1babfc134000"
/>

<img width="1696" height="588" alt="image"
src="https://github.com/user-attachments/assets/1bc16048-6eb4-4dfa-9ace-dd271074530f"
/>

</details>
This commit is contained in:
Alex Waygood 2025-11-15 22:12:00 +00:00 committed by GitHub
parent fb5b8c3653
commit 3065f8dbbc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 374 additions and 26 deletions

View file

@ -116,17 +116,18 @@ def silence2[T: (
## Invalid exception handlers ## Invalid exception handlers
<!-- snapshot-diagnostics -->
```py ```py
try: try:
pass pass
# error: [invalid-exception-caught] "Cannot catch object of type `Literal[3]` in an exception handler (must be a `BaseException` subclass or a tuple of `BaseException` subclasses)" # error: [invalid-exception-caught]
except 3 as e: except 3 as e:
reveal_type(e) # revealed: Unknown reveal_type(e) # revealed: Unknown
try: try:
pass pass
# error: [invalid-exception-caught] "Cannot catch object of type `Literal["foo"]` in an exception handler (must be a `BaseException` subclass or a tuple of `BaseException` subclasses)" # error: [invalid-exception-caught]
# error: [invalid-exception-caught] "Cannot catch object of type `Literal[b"bar"]` in an exception handler (must be a `BaseException` subclass or a tuple of `BaseException` subclasses)"
except (ValueError, OSError, "foo", b"bar") as e: except (ValueError, OSError, "foo", b"bar") as e:
reveal_type(e) # revealed: ValueError | OSError | Unknown reveal_type(e) # revealed: ValueError | OSError | Unknown
@ -278,3 +279,20 @@ def f(x: type[Exception]):
# error: [possibly-unresolved-reference] # error: [possibly-unresolved-reference]
reveal_type(e) # revealed: None reveal_type(e) # revealed: None
``` ```
## Special-cased diagnostics for `NotImplemented`
<!-- snapshot-diagnostics -->
```py
try:
# error: [invalid-raise]
# error: [invalid-raise]
raise NotImplemented from NotImplemented
# error: [invalid-exception-caught]
except NotImplemented:
pass
# error: [invalid-exception-caught]
except (TypeError, NotImplemented):
pass
```

View file

@ -0,0 +1,146 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: basic.md - Exception Handling - Invalid exception handlers
mdtest path: crates/ty_python_semantic/resources/mdtest/exception/basic.md
---
# Python source files
## mdtest_snippet.py
```
1 | try:
2 | pass
3 | # error: [invalid-exception-caught]
4 | except 3 as e:
5 | reveal_type(e) # revealed: Unknown
6 |
7 | try:
8 | pass
9 | # error: [invalid-exception-caught]
10 | except (ValueError, OSError, "foo", b"bar") as e:
11 | reveal_type(e) # revealed: ValueError | OSError | Unknown
12 |
13 | def foo(
14 | x: type[str],
15 | y: tuple[type[OSError], type[RuntimeError], int],
16 | z: tuple[type[str], ...],
17 | ):
18 | try:
19 | help()
20 | # error: [invalid-exception-caught]
21 | except x as e:
22 | reveal_type(e) # revealed: Unknown
23 | # error: [invalid-exception-caught]
24 | except y as f:
25 | reveal_type(f) # revealed: OSError | RuntimeError | Unknown
26 | # error: [invalid-exception-caught]
27 | except z as g:
28 | reveal_type(g) # revealed: Unknown
29 |
30 | try:
31 | {}.get("foo")
32 | # error: [invalid-exception-caught]
33 | except int:
34 | pass
```
# Diagnostics
```
error[invalid-exception-caught]: Invalid object caught in an exception handler
--> src/mdtest_snippet.py:4:8
|
2 | pass
3 | # error: [invalid-exception-caught]
4 | except 3 as e:
| ^ Object has type `Literal[3]`
5 | reveal_type(e) # revealed: Unknown
|
info: Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses
info: rule `invalid-exception-caught` is enabled by default
```
```
error[invalid-exception-caught]: Invalid tuple caught in an exception handler
--> src/mdtest_snippet.py:10:8
|
8 | pass
9 | # error: [invalid-exception-caught]
10 | except (ValueError, OSError, "foo", b"bar") as e:
| ^^^^^^^^^^^^^^^^^^^^^^-----^^------^
| | |
| | Invalid element of type `Literal[b"bar"]`
| Invalid element of type `Literal["foo"]`
11 | reveal_type(e) # revealed: ValueError | OSError | Unknown
|
info: Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses
info: rule `invalid-exception-caught` is enabled by default
```
```
error[invalid-exception-caught]: Invalid object caught in an exception handler
--> src/mdtest_snippet.py:21:12
|
19 | help()
20 | # error: [invalid-exception-caught]
21 | except x as e:
| ^ Object has type `type[str]`
22 | reveal_type(e) # revealed: Unknown
23 | # error: [invalid-exception-caught]
|
info: Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses
info: rule `invalid-exception-caught` is enabled by default
```
```
error[invalid-exception-caught]: Invalid tuple caught in an exception handler
--> src/mdtest_snippet.py:24:12
|
22 | reveal_type(e) # revealed: Unknown
23 | # error: [invalid-exception-caught]
24 | except y as f:
| ^ Object has type `tuple[type[OSError], type[RuntimeError], int]`
25 | reveal_type(f) # revealed: OSError | RuntimeError | Unknown
26 | # error: [invalid-exception-caught]
|
info: Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses
info: rule `invalid-exception-caught` is enabled by default
```
```
error[invalid-exception-caught]: Invalid tuple caught in an exception handler
--> src/mdtest_snippet.py:27:12
|
25 | reveal_type(f) # revealed: OSError | RuntimeError | Unknown
26 | # error: [invalid-exception-caught]
27 | except z as g:
| ^ Object has type `tuple[type[str], ...]`
28 | reveal_type(g) # revealed: Unknown
|
info: Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses
info: rule `invalid-exception-caught` is enabled by default
```
```
error[invalid-exception-caught]: Invalid object caught in an exception handler
--> src/mdtest_snippet.py:33:8
|
31 | {}.get("foo")
32 | # error: [invalid-exception-caught]
33 | except int:
| ^^^ Object has type `<class 'int'>`
34 | pass
|
info: Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses
info: rule `invalid-exception-caught` is enabled by default
```

View file

@ -0,0 +1,93 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: basic.md - Exception Handling - Special-cased diagnostics for `NotImplemented`
mdtest path: crates/ty_python_semantic/resources/mdtest/exception/basic.md
---
# Python source files
## mdtest_snippet.py
```
1 | try:
2 | # error: [invalid-raise]
3 | # error: [invalid-raise]
4 | raise NotImplemented from NotImplemented
5 | # error: [invalid-exception-caught]
6 | except NotImplemented:
7 | pass
8 | # error: [invalid-exception-caught]
9 | except (TypeError, NotImplemented):
10 | pass
```
# Diagnostics
```
error[invalid-raise]: Cannot raise `NotImplemented`
--> src/mdtest_snippet.py:4:11
|
2 | # error: [invalid-raise]
3 | # error: [invalid-raise]
4 | raise NotImplemented from NotImplemented
| ^^^^^^^^^^^^^^ Did you mean `NotImplementedError`?
5 | # error: [invalid-exception-caught]
6 | except NotImplemented:
|
info: Can only raise an instance or subclass of `BaseException`
info: rule `invalid-raise` is enabled by default
```
```
error[invalid-raise]: Cannot use `NotImplemented` as an exception cause
--> src/mdtest_snippet.py:4:31
|
2 | # error: [invalid-raise]
3 | # error: [invalid-raise]
4 | raise NotImplemented from NotImplemented
| ^^^^^^^^^^^^^^ Did you mean `NotImplementedError`?
5 | # error: [invalid-exception-caught]
6 | except NotImplemented:
|
info: An exception cause must be an instance of `BaseException`, subclass of `BaseException`, or `None`
info: rule `invalid-raise` is enabled by default
```
```
error[invalid-exception-caught]: Cannot catch `NotImplemented` in an exception handler
--> src/mdtest_snippet.py:6:8
|
4 | raise NotImplemented from NotImplemented
5 | # error: [invalid-exception-caught]
6 | except NotImplemented:
| ^^^^^^^^^^^^^^ Did you mean `NotImplementedError`?
7 | pass
8 | # error: [invalid-exception-caught]
|
info: Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses
info: rule `invalid-exception-caught` is enabled by default
```
```
error[invalid-exception-caught]: Invalid tuple caught in an exception handler
--> src/mdtest_snippet.py:9:8
|
7 | pass
8 | # error: [invalid-exception-caught]
9 | except (TypeError, NotImplemented):
| ^^^^^^^^^^^^--------------^
| |
| Invalid element of type `NotImplementedType`
| Did you mean `NotImplementedError`?
10 | pass
|
info: Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses
info: rule `invalid-exception-caught` is enabled by default
```

View file

@ -2380,36 +2380,108 @@ pub(super) fn report_possibly_missing_attribute(
}; };
} }
pub(super) fn report_invalid_exception_tuple_caught<'db, 'ast>(
context: &InferContext<'db, 'ast>,
node: &'ast ast::ExprTuple,
invalid_tuple_nodes: impl IntoIterator<Item = (&'ast ast::Expr, Type<'db>)>,
) {
let Some(builder) = context.report_lint(&INVALID_EXCEPTION_CAUGHT, node) else {
return;
};
let mut diagnostic = builder.into_diagnostic("Invalid tuple caught in an exception handler");
for (sub_node, ty) in invalid_tuple_nodes {
let span = context.span(sub_node);
diagnostic.annotate(Annotation::secondary(span.clone()).message(format_args!(
"Invalid element of type `{}`",
ty.display(context.db())
)));
if ty.is_notimplemented(context.db()) {
diagnostic.annotate(
Annotation::secondary(span).message("Did you mean `NotImplementedError`?"),
);
}
}
diagnostic.info(
"Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses",
);
}
pub(super) fn report_invalid_exception_caught(context: &InferContext, node: &ast::Expr, ty: Type) { pub(super) fn report_invalid_exception_caught(context: &InferContext, node: &ast::Expr, ty: Type) {
let Some(builder) = context.report_lint(&INVALID_EXCEPTION_CAUGHT, node) else { let Some(builder) = context.report_lint(&INVALID_EXCEPTION_CAUGHT, node) else {
return; return;
}; };
builder.into_diagnostic(format_args!(
"Cannot catch object of type `{}` in an exception handler \ let mut diagnostic = if ty.is_notimplemented(context.db()) {
(must be a `BaseException` subclass or a tuple of `BaseException` subclasses)", let mut diag =
ty.display(context.db()) builder.into_diagnostic("Cannot catch `NotImplemented` in an exception handler");
)); diag.set_primary_message("Did you mean `NotImplementedError`?");
diag
} else {
let mut diag = builder.into_diagnostic(format_args!(
"Invalid {thing} caught in an exception handler",
thing = if ty.tuple_instance_spec(context.db()).is_some() {
"tuple"
} else {
"object"
},
));
diag.set_primary_message(format_args!(
"Object has type `{}`",
ty.display(context.db())
));
diag
};
diagnostic.info(
"Can only catch a subclass of `BaseException` or tuple of `BaseException` subclasses",
);
} }
pub(crate) fn report_invalid_exception_raised(context: &InferContext, node: &ast::Expr, ty: Type) { pub(crate) fn report_invalid_exception_raised(
let Some(builder) = context.report_lint(&INVALID_RAISE, node) else { context: &InferContext,
raised_node: &ast::Expr,
raise_type: Type,
) {
let Some(builder) = context.report_lint(&INVALID_RAISE, raised_node) else {
return; return;
}; };
builder.into_diagnostic(format_args!( if raise_type.is_notimplemented(context.db()) {
"Cannot raise object of type `{}` (must be a `BaseException` subclass or instance)", let mut diagnostic =
ty.display(context.db()) builder.into_diagnostic(format_args!("Cannot raise `NotImplemented`",));
)); diagnostic.set_primary_message("Did you mean `NotImplementedError`?");
diagnostic.info("Can only raise an instance or subclass of `BaseException`");
} else {
let mut diagnostic = builder.into_diagnostic(format_args!(
"Cannot raise object of type `{}`",
raise_type.display(context.db())
));
diagnostic.set_primary_message("Not an instance or subclass of `BaseException`");
}
} }
pub(crate) fn report_invalid_exception_cause(context: &InferContext, node: &ast::Expr, ty: Type) { pub(crate) fn report_invalid_exception_cause(context: &InferContext, node: &ast::Expr, ty: Type) {
let Some(builder) = context.report_lint(&INVALID_RAISE, node) else { let Some(builder) = context.report_lint(&INVALID_RAISE, node) else {
return; return;
}; };
builder.into_diagnostic(format_args!( let mut diagnostic = if ty.is_notimplemented(context.db()) {
"Cannot use object of type `{}` as exception cause \ let mut diag = builder.into_diagnostic(format_args!(
(must be a `BaseException` subclass or instance or `None`)", "Cannot use `NotImplemented` as an exception cause",
ty.display(context.db()) ));
)); diag.set_primary_message("Did you mean `NotImplementedError`?");
diag
} else {
builder.into_diagnostic(format_args!(
"Cannot use object of type `{}` as an exception cause",
ty.display(context.db())
))
};
diagnostic.info(
"An exception cause must be an instance of `BaseException`, \
subclass of `BaseException`, or `None`",
);
} }
pub(crate) fn report_instance_layout_conflict( pub(crate) fn report_instance_layout_conflict(

View file

@ -72,9 +72,10 @@ use crate::types::diagnostic::{
report_instance_layout_conflict, report_invalid_arguments_to_annotated, report_instance_layout_conflict, report_invalid_arguments_to_annotated,
report_invalid_assignment, report_invalid_attribute_assignment, report_invalid_assignment, report_invalid_attribute_assignment,
report_invalid_exception_caught, report_invalid_exception_cause, report_invalid_exception_caught, report_invalid_exception_cause,
report_invalid_exception_raised, report_invalid_generator_function_return_type, report_invalid_exception_raised, report_invalid_exception_tuple_caught,
report_invalid_key_on_typed_dict, report_invalid_or_unsupported_base, report_invalid_generator_function_return_type, report_invalid_key_on_typed_dict,
report_invalid_return_type, report_invalid_type_checking_constant, report_invalid_or_unsupported_base, report_invalid_return_type,
report_invalid_type_checking_constant,
report_namedtuple_field_without_default_after_field_with_default, report_non_subscriptable, report_namedtuple_field_without_default_after_field_with_default, report_non_subscriptable,
report_possibly_missing_attribute, report_possibly_unresolved_reference, report_possibly_missing_attribute, report_possibly_unresolved_reference,
report_rebound_typevar, report_slice_step_size_zero, report_rebound_typevar, report_slice_step_size_zero,
@ -3025,7 +3026,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
// it will actually be the type of the generic parameters to `BaseExceptionGroup` or `ExceptionGroup`. // it will actually be the type of the generic parameters to `BaseExceptionGroup` or `ExceptionGroup`.
let symbol_ty = if let Some(tuple_spec) = node_ty.tuple_instance_spec(self.db()) { let symbol_ty = if let Some(tuple_spec) = node_ty.tuple_instance_spec(self.db()) {
let mut builder = UnionBuilder::new(self.db()); let mut builder = UnionBuilder::new(self.db());
for element in tuple_spec.all_elements().copied() { let mut invalid_elements = vec![];
for (index, element) in tuple_spec.all_elements().enumerate() {
builder = builder.add( builder = builder.add(
if element.is_assignable_to(self.db(), type_base_exception) { if element.is_assignable_to(self.db(), type_base_exception) {
element.to_instance(self.db()).expect( element.to_instance(self.db()).expect(
@ -3033,13 +3036,29 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
if called on a type assignable to `type[BaseException]`", if called on a type assignable to `type[BaseException]`",
) )
} else { } else {
if let Some(node) = node { invalid_elements.push((index, element));
report_invalid_exception_caught(&self.context, node, element);
}
Type::unknown() Type::unknown()
}, },
); );
} }
if !invalid_elements.is_empty()
&& let Some(node) = node
{
if let ast::Expr::Tuple(tuple) = node
&& !tuple.iter().any(ast::Expr::is_starred_expr)
&& Some(tuple.len()) == tuple_spec.len().into_fixed_length()
{
let invalid_elements = invalid_elements
.iter()
.map(|(index, ty)| (&tuple.elts[*index], **ty));
report_invalid_exception_tuple_caught(&self.context, tuple, invalid_elements);
} else {
report_invalid_exception_caught(&self.context, node, node_ty);
}
}
builder.build() builder.build()
} else if node_ty.is_assignable_to(self.db(), type_base_exception) { } else if node_ty.is_assignable_to(self.db(), type_base_exception) {
node_ty.to_instance(self.db()).expect( node_ty.to_instance(self.db()).expect(