Skip to content

Commit ed2bce6

Browse files
[red-knot] Report invalid exceptions (#15042)
Co-authored-by: Alex Waygood <[email protected]>
1 parent f0012df commit ed2bce6

File tree

4 files changed

+177
-10
lines changed

4 files changed

+177
-10
lines changed

crates/red_knot_python_semantic/resources/mdtest/exception/basic.md

+80
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,83 @@ def foo(
9090
# TODO: should emit a diagnostic here:
9191
reveal_type(g) # revealed: @Todo(full tuple[...] support)
9292
```
93+
94+
## Object raised is not an exception
95+
96+
```py
97+
try:
98+
raise AttributeError() # fine
99+
except:
100+
...
101+
102+
try:
103+
raise FloatingPointError # fine
104+
except:
105+
...
106+
107+
try:
108+
raise 1 # error: [invalid-raise]
109+
except:
110+
...
111+
112+
try:
113+
raise int # error: [invalid-raise]
114+
except:
115+
...
116+
117+
def _(e: Exception | type[Exception]):
118+
raise e # fine
119+
120+
def _(e: Exception | type[Exception] | None):
121+
raise e # error: [invalid-raise]
122+
```
123+
124+
## Exception cause is not an exception
125+
126+
```py
127+
try:
128+
raise EOFError() from GeneratorExit # fine
129+
except:
130+
...
131+
132+
try:
133+
raise StopIteration from MemoryError() # fine
134+
except:
135+
...
136+
137+
try:
138+
raise BufferError() from None # fine
139+
except:
140+
...
141+
142+
try:
143+
raise ZeroDivisionError from False # error: [invalid-raise]
144+
except:
145+
...
146+
147+
try:
148+
raise SystemExit from bool() # error: [invalid-raise]
149+
except:
150+
...
151+
152+
try:
153+
raise
154+
except KeyboardInterrupt as e: # fine
155+
reveal_type(e) # revealed: KeyboardInterrupt
156+
raise LookupError from e # fine
157+
158+
try:
159+
raise
160+
except int as e: # error: [invalid-exception-caught]
161+
reveal_type(e) # revealed: Unknown
162+
raise KeyError from e
163+
164+
def _(e: Exception | type[Exception]):
165+
raise ModuleNotFoundError from e # fine
166+
167+
def _(e: Exception | type[Exception] | None):
168+
raise IndexError from e # fine
169+
170+
def _(e: int | None):
171+
raise IndexError from e # error: [invalid-raise]
172+
```

crates/red_knot_python_semantic/src/types.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -2315,10 +2315,11 @@ impl<'db> KnownClass {
23152315
.unwrap_or(Type::Unknown)
23162316
}
23172317

2318-
pub fn to_subclass_of(self, db: &'db dyn Db) -> Option<Type<'db>> {
2318+
pub fn to_subclass_of(self, db: &'db dyn Db) -> Type<'db> {
23192319
self.to_class_literal(db)
23202320
.into_class_literal()
23212321
.map(|ClassLiteralType { class }| Type::subclass_of(class))
2322+
.unwrap_or(Type::subclass_of_base(ClassBase::Unknown))
23222323
}
23232324

23242325
/// Return the module in which we should look up the definition for this class

crates/red_knot_python_semantic/src/types/diagnostic.rs

+67
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
3434
registry.register_lint(&INVALID_DECLARATION);
3535
registry.register_lint(&INVALID_EXCEPTION_CAUGHT);
3636
registry.register_lint(&INVALID_PARAMETER_DEFAULT);
37+
registry.register_lint(&INVALID_RAISE);
3738
registry.register_lint(&INVALID_TYPE_FORM);
3839
registry.register_lint(&INVALID_TYPE_VARIABLE_CONSTRAINTS);
3940
registry.register_lint(&NON_SUBSCRIPTABLE);
@@ -248,6 +249,49 @@ declare_lint! {
248249
}
249250
}
250251

252+
declare_lint! {
253+
/// Checks for `raise` statements that raise non-exceptions or use invalid
254+
/// causes for their raised exceptions.
255+
///
256+
/// ## Why is this bad?
257+
/// Only subclasses or instances of `BaseException` can be raised.
258+
/// For an exception's cause, the same rules apply, except that `None` is also
259+
/// permitted. Violating these rules results in a `TypeError` at runtime.
260+
///
261+
/// ## Examples
262+
/// ```python
263+
/// def f():
264+
/// try:
265+
/// something()
266+
/// except NameError:
267+
/// raise "oops!" from f
268+
///
269+
/// def g():
270+
/// raise NotImplemented from 42
271+
/// ```
272+
///
273+
/// Use instead:
274+
/// ```python
275+
/// def f():
276+
/// try:
277+
/// something()
278+
/// except NameError as e:
279+
/// raise RuntimeError("oops!") from e
280+
///
281+
/// def g():
282+
/// raise NotImplementedError from None
283+
/// ```
284+
///
285+
/// ## References
286+
/// - [Python documentation: The `raise` statement](https://docs.python.org/3/reference/simple_stmts.html#raise)
287+
/// - [Python documentation: Built-in Exceptions](https://docs.python.org/3/library/exceptions.html#built-in-exceptions)
288+
pub(crate) static INVALID_RAISE = {
289+
summary: "detects `raise` statements that raise invalid exceptions or use invalid causes",
290+
status: LintStatus::preview("1.0.0"),
291+
default_level: Level::Error,
292+
}
293+
}
294+
251295
declare_lint! {
252296
/// ## What it does
253297
/// Checks for invalid type expressions.
@@ -721,3 +765,26 @@ pub(super) fn report_invalid_exception_caught(context: &InferContext, node: &ast
721765
),
722766
);
723767
}
768+
769+
pub(crate) fn report_invalid_exception_raised(context: &InferContext, node: &ast::Expr, ty: Type) {
770+
context.report_lint(
771+
&INVALID_RAISE,
772+
node.into(),
773+
format_args!(
774+
"Cannot raise object of type `{}` (must be a `BaseException` subclass or instance)",
775+
ty.display(context.db())
776+
),
777+
);
778+
}
779+
780+
pub(crate) fn report_invalid_exception_cause(context: &InferContext, node: &ast::Expr, ty: Type) {
781+
context.report_lint(
782+
&INVALID_RAISE,
783+
node.into(),
784+
format_args!(
785+
"Cannot use object of type `{}` as exception cause \
786+
(must be a `BaseException` subclass or instance or `None`)",
787+
ty.display(context.db())
788+
),
789+
);
790+
}

crates/red_knot_python_semantic/src/types/infer.rs

+28-9
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ use crate::Db;
7474

7575
use super::context::{InferContext, WithDiagnostics};
7676
use super::diagnostic::{
77-
report_index_out_of_bounds, report_invalid_exception_caught, report_non_subscriptable,
77+
report_index_out_of_bounds, report_invalid_exception_caught, report_invalid_exception_cause,
78+
report_invalid_exception_raised, report_non_subscriptable,
7879
report_possibly_unresolved_reference, report_slice_step_size_zero, report_unresolved_reference,
7980
};
8081
use super::string_annotation::{
@@ -1574,9 +1575,7 @@ impl<'db> TypeInferenceBuilder<'db> {
15741575
// If it's an `except*` handler, this won't actually be the type of the bound symbol;
15751576
// it will actually be the type of the generic parameters to `BaseExceptionGroup` or `ExceptionGroup`.
15761577
let symbol_ty = if let Type::Tuple(tuple) = node_ty {
1577-
let type_base_exception = KnownClass::BaseException
1578-
.to_subclass_of(self.db())
1579-
.unwrap_or(Type::Unknown);
1578+
let type_base_exception = KnownClass::BaseException.to_subclass_of(self.db());
15801579
let mut builder = UnionBuilder::new(self.db());
15811580
for element in tuple.elements(self.db()).iter().copied() {
15821581
builder = builder.add(
@@ -1594,9 +1593,7 @@ impl<'db> TypeInferenceBuilder<'db> {
15941593
} else if node_ty.is_subtype_of(self.db(), KnownClass::Tuple.to_instance(self.db())) {
15951594
todo_type!("Homogeneous tuple in exception handler")
15961595
} else {
1597-
let type_base_exception = KnownClass::BaseException
1598-
.to_subclass_of(self.db())
1599-
.unwrap_or(Type::Unknown);
1596+
let type_base_exception = KnownClass::BaseException.to_subclass_of(self.db());
16001597
if node_ty.is_assignable_to(self.db(), type_base_exception) {
16011598
node_ty.to_instance(self.db())
16021599
} else {
@@ -2198,8 +2195,30 @@ impl<'db> TypeInferenceBuilder<'db> {
21982195
exc,
21992196
cause,
22002197
} = raise;
2201-
self.infer_optional_expression(exc.as_deref());
2202-
self.infer_optional_expression(cause.as_deref());
2198+
2199+
let base_exception_type = KnownClass::BaseException.to_subclass_of(self.db());
2200+
let base_exception_instance = base_exception_type.to_instance(self.db());
2201+
2202+
let can_be_raised =
2203+
UnionType::from_elements(self.db(), [base_exception_type, base_exception_instance]);
2204+
let can_be_exception_cause =
2205+
UnionType::from_elements(self.db(), [can_be_raised, Type::none(self.db())]);
2206+
2207+
if let Some(raised) = exc {
2208+
let raised_type = self.infer_expression(raised);
2209+
2210+
if !raised_type.is_assignable_to(self.db(), can_be_raised) {
2211+
report_invalid_exception_raised(&self.context, raised, raised_type);
2212+
}
2213+
}
2214+
2215+
if let Some(cause) = cause {
2216+
let cause_type = self.infer_expression(cause);
2217+
2218+
if !cause_type.is_assignable_to(self.db(), can_be_exception_cause) {
2219+
report_invalid_exception_cause(&self.context, cause, cause_type);
2220+
}
2221+
}
22032222
}
22042223

22052224
/// Given a `from .foo import bar` relative import, resolve the relative module

0 commit comments

Comments
 (0)