[pydoclint] Teach rules to understand reraised exceptions as being explicitly raised (#12639)
## Summary Fixes #12630. DOC501 and DOC502 now understand functions with constructs like this to be explicitly raising `TypeError` (which should be documented in a function's docstring): ```py try: foo(): except TypeError: ... raise ``` I made an exception for `Exception` and `BaseException`, however. Constructs like this are reasonably common, and I don't think anybody would say that it's worth putting in the docstring that it raises "some kind of generic exception": ```py try: foo() except BaseException: do_some_logging() raise ``` ## Test Plan `cargo test -p ruff_linter --lib`
This commit is contained in:
@@ -190,3 +190,45 @@ def foo(bar: int):
|
||||
something.SomeError: Wow.
|
||||
"""
|
||||
raise something.SomeError
|
||||
|
||||
|
||||
# DOC501
|
||||
def calculate_speed(distance: float, time: float) -> float:
|
||||
"""Calculate speed as distance divided by time.
|
||||
|
||||
Args:
|
||||
distance: Distance traveled.
|
||||
time: Time spent traveling.
|
||||
|
||||
Returns:
|
||||
Speed as distance divided by time.
|
||||
|
||||
Raises:
|
||||
TypeError: if you didn't pass a number for both parameters
|
||||
"""
|
||||
try:
|
||||
return distance / time
|
||||
except ZeroDivisionError:
|
||||
print("Oh no, why would you divide something by zero?")
|
||||
raise
|
||||
except TypeError:
|
||||
print("Not a number? Shame on you!")
|
||||
raise
|
||||
|
||||
|
||||
# This is fine
|
||||
def calculate_speed(distance: float, time: float) -> float:
|
||||
"""Calculate speed as distance divided by time.
|
||||
|
||||
Args:
|
||||
distance: Distance traveled.
|
||||
time: Time spent traveling.
|
||||
|
||||
Returns:
|
||||
Speed as distance divided by time.
|
||||
"""
|
||||
try:
|
||||
return distance / time
|
||||
except Exception as e:
|
||||
print(f"Oh no, we encountered {e}")
|
||||
raise
|
||||
|
||||
@@ -76,3 +76,60 @@ def calculate_speed(distance: float, time: float) -> float:
|
||||
raise FasterThanLightError from exc
|
||||
except:
|
||||
raise ValueError
|
||||
|
||||
|
||||
# DOC501
|
||||
def calculate_speed(distance: float, time: float) -> float:
|
||||
"""Calculate speed as distance divided by time.
|
||||
|
||||
ACalculate speed as distance divided by time.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
distance : float
|
||||
Distance traveled.
|
||||
time : float
|
||||
Time spent traveling.
|
||||
|
||||
Returns
|
||||
-------
|
||||
float
|
||||
Speed as distance divided by time.
|
||||
|
||||
Raises
|
||||
------
|
||||
ZeroDivisionError
|
||||
If attempting to divide by zero.
|
||||
"""
|
||||
try:
|
||||
return distance / time
|
||||
except ZeroDivisionError:
|
||||
print("Oh no, why would you divide something by zero?")
|
||||
raise
|
||||
except TypeError:
|
||||
print("Not a number? Shame on you!")
|
||||
raise
|
||||
|
||||
|
||||
# This is fine
|
||||
def calculate_speed(distance: float, time: float) -> float:
|
||||
"""
|
||||
Calculate speed as distance divided by time.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
distance : float
|
||||
Distance traveled.
|
||||
time : float
|
||||
Time spent traveling.
|
||||
|
||||
Returns
|
||||
-------
|
||||
float
|
||||
Speed as distance divided by time.
|
||||
"""
|
||||
try:
|
||||
return distance / time
|
||||
except Exception as e:
|
||||
print(f"Oh no, we encountered {e}")
|
||||
raise
|
||||
|
||||
@@ -56,3 +56,28 @@ def calculate_speed(distance: float, time: float) -> float:
|
||||
return distance / time
|
||||
except ZeroDivisionError as exc:
|
||||
raise FasterThanLightError from exc
|
||||
|
||||
|
||||
# This is fine
|
||||
def calculate_speed(distance: float, time: float) -> float:
|
||||
"""Calculate speed as distance divided by time.
|
||||
|
||||
Args:
|
||||
distance: Distance traveled.
|
||||
time: Time spent traveling.
|
||||
|
||||
Returns:
|
||||
Speed as distance divided by time.
|
||||
|
||||
Raises:
|
||||
ZeroDivisionError: If you pass `0` for the time
|
||||
TypeError: if you didn't pass a number for both parameters
|
||||
"""
|
||||
try:
|
||||
return distance / time
|
||||
except ZeroDivisionError:
|
||||
print("Oh no, why would you divide something by zero?")
|
||||
raise
|
||||
except TypeError:
|
||||
print("Not a number? Shame on you!")
|
||||
raise
|
||||
|
||||
@@ -82,3 +82,38 @@ def calculate_speed(distance: float, time: float) -> float:
|
||||
return distance / time
|
||||
except ZeroDivisionError as exc:
|
||||
raise FasterThanLightError from exc
|
||||
|
||||
|
||||
# This is fine
|
||||
def calculate_speed(distance: float, time: float) -> float:
|
||||
"""Calculate speed as distance divided by time.
|
||||
|
||||
ACalculate speed as distance divided by time.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
distance : float
|
||||
Distance traveled.
|
||||
time : float
|
||||
Time spent traveling.
|
||||
|
||||
Returns
|
||||
-------
|
||||
float
|
||||
Speed as distance divided by time.
|
||||
|
||||
Raises
|
||||
------
|
||||
TypeError
|
||||
If one or both of the parameters is not a number.
|
||||
ZeroDivisionError
|
||||
If attempting to divide by zero.
|
||||
"""
|
||||
try:
|
||||
return distance / time
|
||||
except ZeroDivisionError:
|
||||
print("Oh no, why would you divide something by zero?")
|
||||
raise
|
||||
except TypeError:
|
||||
print("Not a number? Shame on you!")
|
||||
raise
|
||||
|
||||
@@ -2,6 +2,7 @@ use itertools::Itertools;
|
||||
use ruff_diagnostics::Diagnostic;
|
||||
use ruff_diagnostics::Violation;
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::helpers::map_callable;
|
||||
use ruff_python_ast::name::QualifiedName;
|
||||
use ruff_python_ast::visitor::Visitor;
|
||||
use ruff_python_ast::{self as ast, visitor, Expr, Stmt};
|
||||
@@ -505,6 +506,7 @@ struct BodyEntries<'a> {
|
||||
struct BodyVisitor<'a> {
|
||||
returns: Vec<Entry>,
|
||||
yields: Vec<Entry>,
|
||||
currently_suspended_exceptions: Option<&'a ast::Expr>,
|
||||
raised_exceptions: Vec<ExceptionEntry<'a>>,
|
||||
semantic: &'a SemanticModel<'a>,
|
||||
}
|
||||
@@ -514,6 +516,7 @@ impl<'a> BodyVisitor<'a> {
|
||||
Self {
|
||||
returns: Vec::new(),
|
||||
yields: Vec::new(),
|
||||
currently_suspended_exceptions: None,
|
||||
raised_exceptions: Vec::new(),
|
||||
semantic,
|
||||
}
|
||||
@@ -529,15 +532,47 @@ impl<'a> BodyVisitor<'a> {
|
||||
}
|
||||
|
||||
impl<'a> Visitor<'a> for BodyVisitor<'a> {
|
||||
fn visit_except_handler(&mut self, handler: &'a ast::ExceptHandler) {
|
||||
let ast::ExceptHandler::ExceptHandler(handler_inner) = handler;
|
||||
self.currently_suspended_exceptions = handler_inner.type_.as_deref();
|
||||
visitor::walk_except_handler(self, handler);
|
||||
self.currently_suspended_exceptions = None;
|
||||
}
|
||||
|
||||
fn visit_stmt(&mut self, stmt: &'a Stmt) {
|
||||
match stmt {
|
||||
Stmt::Raise(ast::StmtRaise { exc: Some(exc), .. }) => {
|
||||
if let Some(qualified_name) = extract_raised_exception(self.semantic, exc.as_ref())
|
||||
{
|
||||
self.raised_exceptions.push(ExceptionEntry {
|
||||
qualified_name,
|
||||
range: exc.as_ref().range(),
|
||||
});
|
||||
Stmt::Raise(ast::StmtRaise { exc, .. }) => {
|
||||
if let Some(exc) = exc.as_ref() {
|
||||
if let Some(qualified_name) =
|
||||
self.semantic.resolve_qualified_name(map_callable(exc))
|
||||
{
|
||||
self.raised_exceptions.push(ExceptionEntry {
|
||||
qualified_name,
|
||||
range: exc.range(),
|
||||
});
|
||||
}
|
||||
} else if let Some(exceptions) = self.currently_suspended_exceptions {
|
||||
let mut maybe_store_exception = |exception| {
|
||||
let Some(qualified_name) = self.semantic.resolve_qualified_name(exception)
|
||||
else {
|
||||
return;
|
||||
};
|
||||
if is_exception_or_base_exception(&qualified_name) {
|
||||
return;
|
||||
}
|
||||
self.raised_exceptions.push(ExceptionEntry {
|
||||
qualified_name,
|
||||
range: stmt.range(),
|
||||
});
|
||||
};
|
||||
|
||||
if let ast::Expr::Tuple(tuple) = exceptions {
|
||||
for exception in &tuple.elts {
|
||||
maybe_store_exception(exception);
|
||||
}
|
||||
} else {
|
||||
maybe_store_exception(exceptions);
|
||||
}
|
||||
}
|
||||
}
|
||||
Stmt::Return(ast::StmtReturn {
|
||||
@@ -571,17 +606,14 @@ impl<'a> Visitor<'a> for BodyVisitor<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
fn extract_raised_exception<'a>(
|
||||
semantic: &SemanticModel<'a>,
|
||||
exc: &'a Expr,
|
||||
) -> Option<QualifiedName<'a>> {
|
||||
if let Some(qualified_name) = semantic.resolve_qualified_name(exc) {
|
||||
return Some(qualified_name);
|
||||
}
|
||||
if let Expr::Call(ast::ExprCall { func, .. }) = exc {
|
||||
return extract_raised_exception(semantic, func.as_ref());
|
||||
}
|
||||
None
|
||||
fn is_exception_or_base_exception(qualified_name: &QualifiedName) -> bool {
|
||||
matches!(
|
||||
qualified_name.segments(),
|
||||
[
|
||||
"" | "builtins",
|
||||
"BaseException" | "Exception" | "BaseExceptionGroup" | "ExceptionGroup"
|
||||
]
|
||||
)
|
||||
}
|
||||
|
||||
/// DOC201, DOC202, DOC402, DOC403, DOC501, DOC502
|
||||
|
||||
@@ -56,3 +56,14 @@ DOC501_google.py:139:11: DOC501 Raised exception `SomeError` missing from docstr
|
||||
| ^^^^^^^^^^^^^^^^^^^ DOC501
|
||||
|
|
||||
= help: Add `SomeError` to the docstring
|
||||
|
||||
DOC501_google.py:213:9: DOC501 Raised exception `ZeroDivisionError` missing from docstring
|
||||
|
|
||||
211 | except ZeroDivisionError:
|
||||
212 | print("Oh no, why would you divide something by zero?")
|
||||
213 | raise
|
||||
| ^^^^^ DOC501
|
||||
214 | except TypeError:
|
||||
215 | print("Not a number? Shame on you!")
|
||||
|
|
||||
= help: Add `ZeroDivisionError` to the docstring
|
||||
|
||||
@@ -29,3 +29,12 @@ DOC501_numpy.py:78:15: DOC501 Raised exception `ValueError` missing from docstri
|
||||
| ^^^^^^^^^^ DOC501
|
||||
|
|
||||
= help: Add `ValueError` to the docstring
|
||||
|
||||
DOC501_numpy.py:111:9: DOC501 Raised exception `TypeError` missing from docstring
|
||||
|
|
||||
109 | except TypeError:
|
||||
110 | print("Not a number? Shame on you!")
|
||||
111 | raise
|
||||
| ^^^^^ DOC501
|
||||
|
|
||||
= help: Add `TypeError` to the docstring
|
||||
|
||||
Reference in New Issue
Block a user