Skip to content
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.

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

[pydoclint] Teach rules to understand reraised exceptions as being explicitly raised #12639

Merged
merged 1 commit into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
68 changes: 50 additions & 18 deletions crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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>,
}
Expand All @@ -514,6 +516,7 @@ impl<'a> BodyVisitor<'a> {
Self {
returns: Vec::new(),
yields: Vec::new(),
currently_suspended_exceptions: None,
raised_exceptions: Vec::new(),
semantic,
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Comment on lines -574 to -584
Copy link
Member Author

Choose a reason for hiding this comment

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

I got rid of this helper by using map_callable() before passing the node to resolve_qualified_name in visit_stmt

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading