Skip to content

Commit

Permalink
[red-knot] Report invalid exceptions (astral-sh#15042)
Browse files Browse the repository at this point in the history
Co-authored-by: Alex Waygood <[email protected]>
  • Loading branch information
InSyncWithFoo and AlexWaygood authored Dec 18, 2024
1 parent f0012df commit ed2bce6
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,83 @@ def foo(
# TODO: should emit a diagnostic here:
reveal_type(g) # revealed: @Todo(full tuple[...] support)
```

## Object raised is not an exception

```py
try:
raise AttributeError() # fine
except:
...

try:
raise FloatingPointError # fine
except:
...

try:
raise 1 # error: [invalid-raise]
except:
...

try:
raise int # error: [invalid-raise]
except:
...

def _(e: Exception | type[Exception]):
raise e # fine

def _(e: Exception | type[Exception] | None):
raise e # error: [invalid-raise]
```

## Exception cause is not an exception

```py
try:
raise EOFError() from GeneratorExit # fine
except:
...

try:
raise StopIteration from MemoryError() # fine
except:
...

try:
raise BufferError() from None # fine
except:
...

try:
raise ZeroDivisionError from False # error: [invalid-raise]
except:
...

try:
raise SystemExit from bool() # error: [invalid-raise]
except:
...

try:
raise
except KeyboardInterrupt as e: # fine
reveal_type(e) # revealed: KeyboardInterrupt
raise LookupError from e # fine

try:
raise
except int as e: # error: [invalid-exception-caught]
reveal_type(e) # revealed: Unknown
raise KeyError from e

def _(e: Exception | type[Exception]):
raise ModuleNotFoundError from e # fine

def _(e: Exception | type[Exception] | None):
raise IndexError from e # fine

def _(e: int | None):
raise IndexError from e # error: [invalid-raise]
```
3 changes: 2 additions & 1 deletion crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2315,10 +2315,11 @@ impl<'db> KnownClass {
.unwrap_or(Type::Unknown)
}

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

/// Return the module in which we should look up the definition for this class
Expand Down
67 changes: 67 additions & 0 deletions crates/red_knot_python_semantic/src/types/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
registry.register_lint(&INVALID_DECLARATION);
registry.register_lint(&INVALID_EXCEPTION_CAUGHT);
registry.register_lint(&INVALID_PARAMETER_DEFAULT);
registry.register_lint(&INVALID_RAISE);
registry.register_lint(&INVALID_TYPE_FORM);
registry.register_lint(&INVALID_TYPE_VARIABLE_CONSTRAINTS);
registry.register_lint(&NON_SUBSCRIPTABLE);
Expand Down Expand Up @@ -248,6 +249,49 @@ declare_lint! {
}
}

declare_lint! {
/// Checks for `raise` statements that raise non-exceptions or use invalid
/// causes for their raised exceptions.
///
/// ## Why is this bad?
/// Only subclasses or instances of `BaseException` can be raised.
/// For an exception's cause, the same rules apply, except that `None` is also
/// permitted. Violating these rules results in a `TypeError` at runtime.
///
/// ## Examples
/// ```python
/// def f():
/// try:
/// something()
/// except NameError:
/// raise "oops!" from f
///
/// def g():
/// raise NotImplemented from 42
/// ```
///
/// Use instead:
/// ```python
/// def f():
/// try:
/// something()
/// except NameError as e:
/// raise RuntimeError("oops!") from e
///
/// def g():
/// raise NotImplementedError from None
/// ```
///
/// ## References
/// - [Python documentation: The `raise` statement](https://docs.python.org/3/reference/simple_stmts.html#raise)
/// - [Python documentation: Built-in Exceptions](https://docs.python.org/3/library/exceptions.html#built-in-exceptions)
pub(crate) static INVALID_RAISE = {
summary: "detects `raise` statements that raise invalid exceptions or use invalid causes",
status: LintStatus::preview("1.0.0"),
default_level: Level::Error,
}
}

declare_lint! {
/// ## What it does
/// Checks for invalid type expressions.
Expand Down Expand Up @@ -721,3 +765,26 @@ pub(super) fn report_invalid_exception_caught(context: &InferContext, node: &ast
),
);
}

pub(crate) fn report_invalid_exception_raised(context: &InferContext, node: &ast::Expr, ty: Type) {
context.report_lint(
&INVALID_RAISE,
node.into(),
format_args!(
"Cannot raise object of type `{}` (must be a `BaseException` subclass or instance)",
ty.display(context.db())
),
);
}

pub(crate) fn report_invalid_exception_cause(context: &InferContext, node: &ast::Expr, ty: Type) {
context.report_lint(
&INVALID_RAISE,
node.into(),
format_args!(
"Cannot use object of type `{}` as exception cause \
(must be a `BaseException` subclass or instance or `None`)",
ty.display(context.db())
),
);
}
37 changes: 28 additions & 9 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ use crate::Db;

use super::context::{InferContext, WithDiagnostics};
use super::diagnostic::{
report_index_out_of_bounds, report_invalid_exception_caught, report_non_subscriptable,
report_index_out_of_bounds, report_invalid_exception_caught, report_invalid_exception_cause,
report_invalid_exception_raised, report_non_subscriptable,
report_possibly_unresolved_reference, report_slice_step_size_zero, report_unresolved_reference,
};
use super::string_annotation::{
Expand Down Expand Up @@ -1574,9 +1575,7 @@ impl<'db> TypeInferenceBuilder<'db> {
// If it's an `except*` handler, this won't actually be the type of the bound symbol;
// it will actually be the type of the generic parameters to `BaseExceptionGroup` or `ExceptionGroup`.
let symbol_ty = if let Type::Tuple(tuple) = node_ty {
let type_base_exception = KnownClass::BaseException
.to_subclass_of(self.db())
.unwrap_or(Type::Unknown);
let type_base_exception = KnownClass::BaseException.to_subclass_of(self.db());
let mut builder = UnionBuilder::new(self.db());
for element in tuple.elements(self.db()).iter().copied() {
builder = builder.add(
Expand All @@ -1594,9 +1593,7 @@ impl<'db> TypeInferenceBuilder<'db> {
} else if node_ty.is_subtype_of(self.db(), KnownClass::Tuple.to_instance(self.db())) {
todo_type!("Homogeneous tuple in exception handler")
} else {
let type_base_exception = KnownClass::BaseException
.to_subclass_of(self.db())
.unwrap_or(Type::Unknown);
let type_base_exception = KnownClass::BaseException.to_subclass_of(self.db());
if node_ty.is_assignable_to(self.db(), type_base_exception) {
node_ty.to_instance(self.db())
} else {
Expand Down Expand Up @@ -2198,8 +2195,30 @@ impl<'db> TypeInferenceBuilder<'db> {
exc,
cause,
} = raise;
self.infer_optional_expression(exc.as_deref());
self.infer_optional_expression(cause.as_deref());

let base_exception_type = KnownClass::BaseException.to_subclass_of(self.db());
let base_exception_instance = base_exception_type.to_instance(self.db());

let can_be_raised =
UnionType::from_elements(self.db(), [base_exception_type, base_exception_instance]);
let can_be_exception_cause =
UnionType::from_elements(self.db(), [can_be_raised, Type::none(self.db())]);

if let Some(raised) = exc {
let raised_type = self.infer_expression(raised);

if !raised_type.is_assignable_to(self.db(), can_be_raised) {
report_invalid_exception_raised(&self.context, raised, raised_type);
}
}

if let Some(cause) = cause {
let cause_type = self.infer_expression(cause);

if !cause_type.is_assignable_to(self.db(), can_be_exception_cause) {
report_invalid_exception_cause(&self.context, cause, cause_type);
}
}
}

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

0 comments on commit ed2bce6

Please sign in to comment.