diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py index f9e7f7a89f64a4..9709d9ff53637a 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py @@ -81,3 +81,55 @@ def calculate_speed(distance: float, time: float) -> float: except TypeError: print("Not a number? Shame on you!") raise + + +# This should NOT trigger DOC502 because OSError is explicitly re-raised +def f(): + """Do nothing. + + Raises: + OSError: If the OS errors. + """ + try: + pass + except OSError as e: + raise e + + +# This should NOT trigger DOC502 because OSError is explicitly re-raised with from None +def g(): + """Do nothing. + + Raises: + OSError: If the OS errors. + """ + try: + pass + except OSError as e: + raise e from None + + +# This should NOT trigger DOC502 because ValueError is explicitly re-raised from tuple exception +def h(): + """Do nothing. + + Raises: + ValueError: If something goes wrong. + """ + try: + pass + except (ValueError, TypeError) as e: + raise e + + +# This should NOT trigger DOC502 because TypeError is explicitly re-raised from tuple exception +def i(): + """Do nothing. + + Raises: + TypeError: If something goes wrong. + """ + try: + pass + except (ValueError, TypeError) as e: + raise e diff --git a/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs index dd4f8ee8a7dddc..a404c719c09318 100644 --- a/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs +++ b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs @@ -9,6 +9,7 @@ use ruff_python_semantic::{Definition, SemanticModel}; use ruff_python_stdlib::identifiers::is_identifier; use ruff_source_file::{LineRanges, NewlineWithTrailingNewline}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; +use rustc_hash::FxHashMap; use crate::Violation; use crate::checkers::ast::Checker; @@ -816,6 +817,8 @@ struct BodyVisitor<'a> { currently_suspended_exceptions: Option<&'a ast::Expr>, raised_exceptions: Vec>, semantic: &'a SemanticModel<'a>, + /// Maps exception variable names to their exception expressions in the current except clause + exception_variables: FxHashMap<&'a str, &'a ast::Expr>, } impl<'a> BodyVisitor<'a> { @@ -826,6 +829,7 @@ impl<'a> BodyVisitor<'a> { currently_suspended_exceptions: None, raised_exceptions: Vec::new(), semantic, + exception_variables: FxHashMap::default(), } } @@ -857,20 +861,47 @@ impl<'a> BodyVisitor<'a> { raised_exceptions, } } + + /// Store `exception` if its qualified name does not correspond to one of the exempt types. + fn maybe_store_exception(&mut self, exception: &'a Expr, range: TextRange) { + 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, + }); + } } 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(); + + // Track exception variable bindings + if let Some(name) = handler_inner.name.as_ref() { + if let Some(exceptions) = self.currently_suspended_exceptions { + // Store the exception expression(s) for later resolution + self.exception_variables + .insert(name.id.as_str(), exceptions); + } + } + visitor::walk_except_handler(self, handler); self.currently_suspended_exceptions = None; + // Clear exception variables when leaving the except handler + self.exception_variables.clear(); } fn visit_stmt(&mut self, stmt: &'a Stmt) { match stmt { Stmt::Raise(ast::StmtRaise { exc, .. }) => { if let Some(exc) = exc.as_ref() { + // First try to resolve the exception directly if let Some(qualified_name) = self.semantic.resolve_qualified_name(map_callable(exc)) { @@ -878,28 +909,27 @@ impl<'a> Visitor<'a> for BodyVisitor<'a> { qualified_name, range: exc.range(), }); + } else if let ast::Expr::Name(name) = exc.as_ref() { + // If it's a variable name, check if it's bound to an exception in the + // current except clause + if let Some(exception_expr) = self.exception_variables.get(name.id.as_str()) + { + if let ast::Expr::Tuple(tuple) = exception_expr { + for exception in tuple { + self.maybe_store_exception(exception, stmt.range()); + } + } else { + self.maybe_store_exception(exception_expr, stmt.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 { - maybe_store_exception(exception); + self.maybe_store_exception(exception, stmt.range()); } } else { - maybe_store_exception(exceptions); + self.maybe_store_exception(exceptions, stmt.range()); } } } diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py.snap index ab1377017453ad..c0b82991ae9616 100644 --- a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py.snap +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py.snap @@ -61,6 +61,66 @@ DOC501 Raised exception `FasterThanLightError` missing from docstring | help: Add `FasterThanLightError` to the docstring +DOC501 Raised exception `ZeroDivisionError` missing from docstring + --> DOC501_google.py:70:5 + | +68 | # DOC501 +69 | def calculate_speed(distance: float, time: float) -> float: +70 | / """Calculate speed as distance divided by time. +71 | | +72 | | Args: +73 | | distance: Distance traveled. +74 | | time: Time spent traveling. +75 | | +76 | | Returns: +77 | | Speed as distance divided by time. +78 | | """ + | |_______^ +79 | try: +80 | return distance / time + | +help: Add `ZeroDivisionError` to the docstring + +DOC501 Raised exception `ValueError` missing from docstring + --> DOC501_google.py:88:5 + | +86 | # DOC501 +87 | def calculate_speed(distance: float, time: float) -> float: +88 | / """Calculate speed as distance divided by time. +89 | | +90 | | Args: +91 | | distance: Distance traveled. +92 | | time: Time spent traveling. +93 | | +94 | | Returns: +95 | | Speed as distance divided by time. +96 | | """ + | |_______^ +97 | try: +98 | return distance / time + | +help: Add `ValueError` to the docstring + +DOC501 Raised exception `ZeroDivisionError` missing from docstring + --> DOC501_google.py:88:5 + | +86 | # DOC501 +87 | def calculate_speed(distance: float, time: float) -> float: +88 | / """Calculate speed as distance divided by time. +89 | | +90 | | Args: +91 | | distance: Distance traveled. +92 | | time: Time spent traveling. +93 | | +94 | | Returns: +95 | | Speed as distance divided by time. +96 | | """ + | |_______^ +97 | try: +98 | return distance / time + | +help: Add `ZeroDivisionError` to the docstring + DOC501 Raised exception `AnotherError` missing from docstring --> DOC501_google.py:106:5 | diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py_ignore_one_line.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py_ignore_one_line.snap index ab1377017453ad..c0b82991ae9616 100644 --- a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py_ignore_one_line.snap +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py_ignore_one_line.snap @@ -61,6 +61,66 @@ DOC501 Raised exception `FasterThanLightError` missing from docstring | help: Add `FasterThanLightError` to the docstring +DOC501 Raised exception `ZeroDivisionError` missing from docstring + --> DOC501_google.py:70:5 + | +68 | # DOC501 +69 | def calculate_speed(distance: float, time: float) -> float: +70 | / """Calculate speed as distance divided by time. +71 | | +72 | | Args: +73 | | distance: Distance traveled. +74 | | time: Time spent traveling. +75 | | +76 | | Returns: +77 | | Speed as distance divided by time. +78 | | """ + | |_______^ +79 | try: +80 | return distance / time + | +help: Add `ZeroDivisionError` to the docstring + +DOC501 Raised exception `ValueError` missing from docstring + --> DOC501_google.py:88:5 + | +86 | # DOC501 +87 | def calculate_speed(distance: float, time: float) -> float: +88 | / """Calculate speed as distance divided by time. +89 | | +90 | | Args: +91 | | distance: Distance traveled. +92 | | time: Time spent traveling. +93 | | +94 | | Returns: +95 | | Speed as distance divided by time. +96 | | """ + | |_______^ +97 | try: +98 | return distance / time + | +help: Add `ValueError` to the docstring + +DOC501 Raised exception `ZeroDivisionError` missing from docstring + --> DOC501_google.py:88:5 + | +86 | # DOC501 +87 | def calculate_speed(distance: float, time: float) -> float: +88 | / """Calculate speed as distance divided by time. +89 | | +90 | | Args: +91 | | distance: Distance traveled. +92 | | time: Time spent traveling. +93 | | +94 | | Returns: +95 | | Speed as distance divided by time. +96 | | """ + | |_______^ +97 | try: +98 | return distance / time + | +help: Add `ZeroDivisionError` to the docstring + DOC501 Raised exception `AnotherError` missing from docstring --> DOC501_google.py:106:5 |