From 387d6700671f8c704107b6d3baa88acf04641b26 Mon Sep 17 00:00:00 2001 From: seroperson Date: Tue, 24 Mar 2026 23:36:56 +0300 Subject: [PATCH 1/2] Implement useless-finally (RUF-072) Closes #19158 --- .../resources/test/fixtures/ruff/RUF072.py | 128 +++++++++ .../test/fixtures/ruff/RUF072_RUF047.py | 20 ++ .../test/fixtures/ruff/RUF072_SIM105.py | 10 + .../src/checkers/ast/analyze/statement.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/ruff/mod.rs | 46 ++- .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + .../src/rules/ruff/rules/useless_finally.rs | 272 ++++++++++++++++++ ...uff__tests__preview__RUF072_RUF072.py.snap | 241 ++++++++++++++++ ...__useless_finally_and_needless_else-2.snap | 18 ++ ...ts__useless_finally_and_needless_else.snap | 60 ++++ ..._finally_and_suppressible_exception-2.snap | 10 + ...ss_finally_and_suppressible_exception.snap | 19 ++ ruff.schema.json | 1 + 14 files changed, 830 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF072_SIM105.py create mode 100644 crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else-2.snap create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_suppressible_exception-2.snap create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_suppressible_exception.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py new file mode 100644 index 00000000000000..e5b49e57bf9c7e --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py @@ -0,0 +1,128 @@ +# Errors + +# Case A: try/except/finally with pass +try: + foo() +except Exception: + bar() +finally: + pass + +# Case A: try/except/finally with ellipsis +try: + foo() +except Exception: + bar() +finally: + ... + +# Case A: try/except/else/finally with pass +try: + foo() +except Exception: + bar() +else: + baz() +finally: + pass + +# Case B: bare try/finally with pass +try: + foo() +finally: + pass + +# Case B: bare try/finally with ellipsis +try: + foo() +finally: + ... + +# Case B: bare try/finally with multi-line body +try: + foo() + bar() + baz() +finally: + pass + +# Nested try with useless finally +try: + try: + foo() + finally: + pass +except Exception: + bar() + +# OK + +# finally with real code +try: + foo() +finally: + cleanup() + +# finally with pass and other statements +try: + foo() +finally: + pass + cleanup() + +# No finally at all +try: + foo() +except Exception: + bar() + +# Comments — diagnostic but no fix + +# Comment on `finally:` line +try: + foo() +except Exception: + bar() +finally: # comment + pass + +# Comment on `pass` line +try: + foo() +except Exception: + bar() +finally: + pass # comment + +# Preceding own-line comment +try: + foo() +except Exception: + bar() +# comment +finally: + pass + +# Trailing own-line comment +try: + foo() +except Exception: + bar() +finally: + pass + # comment + +# Own-line comment before `pass` in the finally body +try: + foo() +except Exception: + bar() +finally: + # comment + pass + +# Comment on bare try/finally +try: + foo() +finally: # comment + pass diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py new file mode 100644 index 00000000000000..0733e20a09dfa4 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py @@ -0,0 +1,20 @@ +# RUF072 removes the empty `finally`, RUF047 removes the empty `else` +# Both fixes apply independently on the same try statement +try: + foo() +except Exception: + bar() +else: + pass +finally: + pass + +# Only the `finally` is empty; `else` has real code +try: + foo() +except Exception: + bar() +else: + baz() +finally: + pass diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_SIM105.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_SIM105.py new file mode 100644 index 00000000000000..fe9360a178e64d --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_SIM105.py @@ -0,0 +1,10 @@ +# SIM105 cannot fire while `finally` is non-empty +# RUF072 removes the empty `finally` first, then SIM105 rewrites +# the `try/except: pass` to `contextlib.suppress()` on the next pass +try: + foo() +except Exception: + pass +finally: + pass + diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 3c4a20437ed923..4031022200d2f7 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1384,6 +1384,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.is_rule_enabled(Rule::NeedlessElse) { ruff::rules::needless_else(checker, try_stmt.into()); } + if checker.is_rule_enabled(Rule::UselessFinally) { + ruff::rules::useless_finally(checker, try_stmt); + } } Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => { if checker.is_rule_enabled(Rule::SelfOrClsAssignment) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 159135dd417727..0559059f79e649 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1069,6 +1069,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "069") => rules::ruff::rules::FloatEqualityComparison, (Ruff, "070") => rules::ruff::rules::UnnecessaryAssignBeforeYield, (Ruff, "071") => rules::ruff::rules::OsPathCommonprefix, + (Ruff, "072") => rules::ruff::rules::UselessFinally, (Ruff, "100") => rules::ruff::rules::UnusedNOQA, (Ruff, "101") => rules::ruff::rules::RedirectedNOQA, diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index b5250c4a08cf11..32cd2f32fb897c 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -22,7 +22,8 @@ mod tests { use crate::rules::pydocstyle::settings::Settings as PydocstyleSettings; use crate::settings::LinterSettings; use crate::settings::types::{CompiledPerFileIgnoreList, PerFileIgnore, PreviewMode}; - use crate::test::{test_path, test_resource_path, test_snippet}; + use crate::source_kind::SourceKind; + use crate::test::{test_contents, test_path, test_resource_path, test_snippet}; use crate::{assert_diagnostics, assert_diagnostics_diff, settings}; #[test_case(Rule::CollectionLiteralConcatenation, Path::new("RUF005.py"))] @@ -134,6 +135,48 @@ mod tests { Ok(()) } + /// Test that RUF072 (useless-finally) and RUF047 (needless-else) converge + /// when both are enabled on the same `try` statement: RUF072 removes the + /// empty `finally` and RUF047 removes the empty `else` independently + #[test] + fn useless_finally_and_needless_else() -> Result<()> { + use ruff_python_ast::{PySourceType, SourceType}; + + let path = test_resource_path("fixtures").join("ruff/RUF072_RUF047.py"); + let source_type = SourceType::Python(PySourceType::from(&path)); + let source_kind = SourceKind::from_path(&path, source_type)?.expect("valid source"); + let settings = + settings::LinterSettings::for_rules(vec![Rule::UselessFinally, Rule::NeedlessElse]); + + let (diagnostics, transformed) = test_contents(&source_kind, &path, &settings); + assert_diagnostics!(diagnostics); + + insta::assert_snapshot!(transformed.source_code()); + Ok(()) + } + + /// Test that RUF072 (useless-finally) and SIM105 (suppressible-exception) + /// converge: RUF072 removes the empty `finally` first, unblocking SIM105 + /// to rewrite `try/except: pass` into `contextlib.suppress()` + #[test] + fn useless_finally_and_suppressible_exception() -> Result<()> { + use ruff_python_ast::{PySourceType, SourceType}; + + let path = test_resource_path("fixtures").join("ruff/RUF072_SIM105.py"); + let source_type = SourceType::Python(PySourceType::from(&path)); + let source_kind = SourceKind::from_path(&path, source_type)?.expect("valid source"); + let settings = settings::LinterSettings::for_rules(vec![ + Rule::UselessFinally, + Rule::SuppressibleException, + ]); + + let (diagnostics, transformed) = test_contents(&source_kind, &path, &settings); + assert_diagnostics!(diagnostics); + + insta::assert_snapshot!(transformed.source_code()); + Ok(()) + } + #[test] fn missing_fstring_syntax_backslash_py311() -> Result<()> { assert_diagnostics_diff!( @@ -668,6 +711,7 @@ mod tests { #[test_case(Rule::FloatEqualityComparison, Path::new("RUF069.py"))] #[test_case(Rule::UnnecessaryAssignBeforeYield, Path::new("RUF070.py"))] #[test_case(Rule::OsPathCommonprefix, Path::new("RUF071.py"))] + #[test_case(Rule::UselessFinally, Path::new("RUF072.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 92ae2cf7c6073c..dfa6d0900bb06d 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -66,6 +66,7 @@ pub(crate) use unused_async::*; pub(crate) use unused_noqa::*; pub(crate) use unused_unpacked_variable::*; pub(crate) use used_dummy_variable::*; +pub(crate) use useless_finally::*; pub(crate) use useless_if_else::*; pub(crate) use zip_instead_of_pairwise::*; @@ -140,6 +141,7 @@ mod unused_async; mod unused_noqa; mod unused_unpacked_variable; mod used_dummy_variable; +mod useless_finally; mod useless_if_else; mod zip_instead_of_pairwise; diff --git a/crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs b/crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs new file mode 100644 index 00000000000000..f69be18d1f4020 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs @@ -0,0 +1,272 @@ +use std::cmp::Ordering; + +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::helpers::comment_indentation_after; +use ruff_python_ast::token::TokenKind; +use ruff_python_ast::whitespace::indentation; +use ruff_python_ast::{Stmt, StmtExpr, StmtTry}; +use ruff_source_file::LineRanges; +use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; + +use crate::checkers::ast::Checker; +use crate::{Edit, Fix, FixAvailability, Violation}; + +/// ## What it does +/// Checks for `finally` clauses that only contain `pass` or `...` statements +/// +/// ## Why is this bad? +/// An empty `finally` clause is a no-op and adds unnecessary noise. +/// If the `try` statement has `except` or `else` clauses, the `finally` +/// clause can simply be removed. If it's a bare `try/finally`, the entire +/// `try` statement can be replaced with its body +/// +/// ## Example +/// ```python +/// try: +/// foo() +/// except Exception: +/// bar() +/// finally: +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// try: +/// foo() +/// except Exception: +/// bar() +/// ``` +/// +/// ## Example +/// ```python +/// try: +/// foo() +/// finally: +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// foo() +/// ``` +/// +/// ## See also +/// - [`needless-else`][RUF047]: Removes empty `else` clauses on `try` (and +/// other statements). Both rules can fire on the same `try` statement +/// - [`suppressible-exception`][SIM105]: Rewrites `try/except: pass` to +/// `contextlib.suppress()`. Won't apply while a `finally` clause is present, +/// so RUF072 must remove it first +/// - [`useless-try-except`][TRY203]: Flags `try/except` that only re-raises +#[derive(ViolationMetadata)] +#[violation_metadata(preview_since = "0.15.7")] +pub(crate) struct UselessFinally; + +impl Violation for UselessFinally { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + "Empty `finally` clause".to_string() + } + + fn fix_title(&self) -> Option { + Some("Remove the `finally` clause".to_string()) + } +} + +/// RUF072 +pub(crate) fn useless_finally(checker: &Checker, try_stmt: &StmtTry) { + let finalbody = &try_stmt.finalbody; + + if !body_is_no_op(finalbody) { + return; + } + + let source = checker.source(); + let tokens = checker.tokens(); + + // `body_is_no_op` guarantees exactly one statement in finalbody + let finalbody_stmt = &finalbody[0]; + let (preceding_end, preceding_stmt) = preceding_clause_info(try_stmt); + + let Some(finally_start) = tokens + .in_range(TextRange::new(preceding_end, finalbody_stmt.end())) + .iter() + .find(|token| token.kind() == TokenKind::Finally) + .map(Ranged::start) + else { + return; + }; + + let finally_range = TextRange::new(finally_start, finalbody_stmt.end()); + + let has_comments = finally_contains_comments( + preceding_stmt, + preceding_end, + finalbody_stmt, + finally_range, + checker, + ); + + let mut diagnostic = checker.report_diagnostic(UselessFinally, finally_range); + + if has_comments { + return; + } + + let is_bare_try_finally = try_stmt.handlers.is_empty() && try_stmt.orelse.is_empty(); + + if is_bare_try_finally { + // bare `try/finally: pass` — unwrap the try body + let (Some(first_body_stmt), Some(last_body_stmt)) = + (try_stmt.body.first(), try_stmt.body.last()) + else { + return; + }; + + let try_indentation = indentation(source, try_stmt).unwrap_or_default(); + + let Ok(adjusted) = crate::fix::edits::adjust_indentation( + TextRange::new( + source.line_start(first_body_stmt.start()), + source.full_line_end(last_body_stmt.end()), + ), + try_indentation, + checker.locator(), + checker.indexer(), + checker.stylist(), + ) else { + return; + }; + + let try_line_start = source.line_start(try_stmt.start()); + let finally_full_end = source.full_line_end(finalbody_stmt.end()); + let edit = + Edit::range_replacement(adjusted, TextRange::new(try_line_start, finally_full_end)); + diagnostic.set_fix(Fix::safe_edit(edit)); + } else { + // `try/except/finally: pass` — remove the finally clause + let finally_line_start = source.line_start(finally_start); + let finally_full_end = source.full_line_end(finalbody_stmt.end()); + let edit = Edit::range_deletion(TextRange::new(finally_line_start, finally_full_end)); + diagnostic.set_fix(Fix::safe_edit(edit)); + } +} + +/// Whether `body` contains only one `pass` or `...` statement +fn body_is_no_op(body: &[Stmt]) -> bool { + match body { + [Stmt::Pass(_)] => true, + [Stmt::Expr(StmtExpr { value, .. })] => value.is_ellipsis_literal_expr(), + _ => false, + } +} + +/// Returns the end offset of the clause preceding `finally` and the last +/// statement in that clause's body (used for comment indentation checks) +fn preceding_clause_info(try_stmt: &StmtTry) -> (TextSize, Option<&Stmt>) { + if let Some(last) = try_stmt.orelse.last() { + return (last.end(), Some(last)); + } + if let Some(handler) = try_stmt.handlers.last() { + let last_stmt = handler.as_except_handler().and_then(|h| h.body.last()); + return (handler.end(), last_stmt); + } + match try_stmt.body.last() { + Some(last) => (last.end(), Some(last)), + None => (try_stmt.start(), None), + } +} + +fn finally_contains_comments( + preceding_stmt: Option<&Stmt>, + preceding_end: TextSize, + finalbody_stmt: &Stmt, + finally_range: TextRange, + checker: &Checker, +) -> bool { + let source = checker.source(); + let finally_full_end = source.full_line_end(finally_range.end()); + let commentable_range = TextRange::new(finally_range.start(), finally_full_end); + + // A comment after the `finally` keyword or after the dummy statement + if checker.comment_ranges().intersects(commentable_range) { + return true; + } + + let Some(preceding_stmt) = preceding_stmt else { + return false; + }; + + finally_has_preceding_comment(preceding_stmt, preceding_end, finally_range, checker) + || finally_has_trailing_comment(finalbody_stmt, finally_full_end, checker) +} + +/// Returns `true` if the `finally` clause header has a leading own-line comment +fn finally_has_preceding_comment( + preceding_stmt: &Stmt, + preceding_end: TextSize, + finally_range: TextRange, + checker: &Checker, +) -> bool { + let (tokens, source) = (checker.tokens(), checker.source()); + let before_finally_full_end = source.full_line_end(preceding_end); + let preceding_indentation = indentation(source, preceding_stmt) + .unwrap_or_default() + .text_len(); + + for token in tokens.in_range(TextRange::new( + before_finally_full_end, + finally_range.start(), + )) { + if token.kind() != TokenKind::Comment { + continue; + } + + let comment_indentation = + comment_indentation_after(preceding_stmt.into(), token.range(), source); + + match comment_indentation.cmp(&preceding_indentation) { + Ordering::Greater | Ordering::Equal => continue, + Ordering::Less => return true, + } + } + + false +} + +/// Returns `true` if the `finally` branch has a trailing own-line comment +fn finally_has_trailing_comment( + last_finally_stmt: &Stmt, + finally_full_end: TextSize, + checker: &Checker, +) -> bool { + let (tokens, source) = (checker.tokens(), checker.source()); + let preceding_indentation = indentation(source, last_finally_stmt) + .unwrap_or_default() + .text_len(); + + for token in tokens.after(finally_full_end) { + match token.kind() { + TokenKind::Comment => { + let comment_indentation = + comment_indentation_after(last_finally_stmt.into(), token.range(), source); + + match comment_indentation.cmp(&preceding_indentation) { + Ordering::Greater | Ordering::Equal => return true, + Ordering::Less => break, + } + } + + TokenKind::NonLogicalNewline + | TokenKind::Newline + | TokenKind::Indent + | TokenKind::Dedent => {} + + _ => break, + } + } + + false +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap new file mode 100644 index 00000000000000..8b3493466132c8 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap @@ -0,0 +1,241 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF072 [*] Empty `finally` clause + --> RUF072.py:8:1 + | + 6 | except Exception: + 7 | bar() + 8 | / finally: + 9 | | pass + | |________^ +10 | +11 | # Case A: try/except/finally with ellipsis + | +help: Remove the `finally` clause +5 | foo() +6 | except Exception: +7 | bar() + - finally: + - pass +8 | +9 | # Case A: try/except/finally with ellipsis +10 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072.py:16:1 + | +14 | except Exception: +15 | bar() +16 | / finally: +17 | | ... + | |_______^ +18 | +19 | # Case A: try/except/else/finally with pass + | +help: Remove the `finally` clause +13 | foo() +14 | except Exception: +15 | bar() + - finally: + - ... +16 | +17 | # Case A: try/except/else/finally with pass +18 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072.py:26:1 + | +24 | else: +25 | baz() +26 | / finally: +27 | | pass + | |________^ +28 | +29 | # Case B: bare try/finally with pass + | +help: Remove the `finally` clause +23 | bar() +24 | else: +25 | baz() + - finally: + - pass +26 | +27 | # Case B: bare try/finally with pass +28 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072.py:32:1 + | +30 | try: +31 | foo() +32 | / finally: +33 | | pass + | |________^ +34 | +35 | # Case B: bare try/finally with ellipsis + | +help: Remove the `finally` clause +27 | pass +28 | +29 | # Case B: bare try/finally with pass + - try: + - foo() + - finally: + - pass +30 + foo() +31 | +32 | # Case B: bare try/finally with ellipsis +33 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072.py:38:1 + | +36 | try: +37 | foo() +38 | / finally: +39 | | ... + | |_______^ +40 | +41 | # Case B: bare try/finally with multi-line body + | +help: Remove the `finally` clause +33 | pass +34 | +35 | # Case B: bare try/finally with ellipsis + - try: + - foo() + - finally: + - ... +36 + foo() +37 | +38 | # Case B: bare try/finally with multi-line body +39 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072.py:46:1 + | +44 | bar() +45 | baz() +46 | / finally: +47 | | pass + | |________^ +48 | +49 | # Nested try with useless finally + | +help: Remove the `finally` clause +39 | ... +40 | +41 | # Case B: bare try/finally with multi-line body + - try: + - foo() + - bar() + - baz() + - finally: + - pass +42 + foo() +43 + bar() +44 + baz() +45 | +46 | # Nested try with useless finally +47 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072.py:53:5 + | +51 | try: +52 | foo() +53 | / finally: +54 | | pass + | |____________^ +55 | except Exception: +56 | bar() + | +help: Remove the `finally` clause +48 | +49 | # Nested try with useless finally +50 | try: + - try: + - foo() + - finally: + - pass +51 + foo() +52 | except Exception: +53 | bar() +54 | + +RUF072 Empty `finally` clause + --> RUF072.py:86:1 + | +84 | except Exception: +85 | bar() +86 | / finally: # comment +87 | | pass + | |________^ +88 | +89 | # Comment on `pass` line + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:94:1 + | +92 | except Exception: +93 | bar() +94 | / finally: +95 | | pass # comment + | |________^ +96 | +97 | # Preceding own-line comment + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:103:1 + | +101 | bar() +102 | # comment +103 | / finally: +104 | | pass + | |________^ +105 | +106 | # Trailing own-line comment + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:111:1 + | +109 | except Exception: +110 | bar() +111 | / finally: +112 | | pass + | |________^ +113 | # comment + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:120:1 + | +118 | except Exception: +119 | bar() +120 | / finally: +121 | | # comment +122 | | pass + | |________^ +123 | +124 | # Comment on bare try/finally + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:127:1 + | +125 | try: +126 | foo() +127 | / finally: # comment +128 | | pass + | |________^ + | +help: Remove the `finally` clause diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else-2.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else-2.snap new file mode 100644 index 00000000000000..e293d05c026c72 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else-2.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +expression: transformed.source_code() +--- +# RUF072 removes the empty `finally`, RUF047 removes the empty `else` +# Both fixes apply independently on the same try statement +try: + foo() +except Exception: + bar() + +# Only the `finally` is empty; `else` has real code +try: + foo() +except Exception: + bar() +else: + baz() diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap new file mode 100644 index 00000000000000..9ff11b7d52de0c --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap @@ -0,0 +1,60 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF047 [*] Empty `else` clause + --> RUF072_RUF047.py:7:1 + | + 5 | except Exception: + 6 | bar() + 7 | / else: + 8 | | pass + | |________^ + 9 | finally: +10 | pass + | +help: Remove the `else` clause +4 | foo() +5 | except Exception: +6 | bar() + - else: + - pass +7 | finally: +8 | pass +9 | + +RUF072 [*] Empty `finally` clause + --> RUF072_RUF047.py:9:1 + | + 7 | else: + 8 | pass + 9 | / finally: +10 | | pass + | |________^ +11 | +12 | # Only the `finally` is empty; `else` has real code + | +help: Remove the `finally` clause +6 | bar() +7 | else: +8 | pass + - finally: + - pass +9 | +10 | # Only the `finally` is empty; `else` has real code +11 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072_RUF047.py:19:1 + | +17 | else: +18 | baz() +19 | / finally: +20 | | pass + | |________^ + | +help: Remove the `finally` clause +16 | bar() +17 | else: +18 | baz() + - finally: + - pass diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_suppressible_exception-2.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_suppressible_exception-2.snap new file mode 100644 index 00000000000000..0185aad1560214 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_suppressible_exception-2.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +expression: transformed.source_code() +--- +# SIM105 cannot fire while `finally` is non-empty +# RUF072 removes the empty `finally` first, then SIM105 rewrites +# the `try/except: pass` to `contextlib.suppress()` on the next pass +import contextlib +with contextlib.suppress(Exception): + foo() diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_suppressible_exception.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_suppressible_exception.snap new file mode 100644 index 00000000000000..3138dd1953c807 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_suppressible_exception.snap @@ -0,0 +1,19 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF072 [*] Empty `finally` clause + --> RUF072_SIM105.py:8:1 + | +6 | except Exception: +7 | pass +8 | / finally: +9 | | pass + | |________^ + | +help: Remove the `finally` clause +5 | foo() +6 | except Exception: +7 | pass + - finally: + - pass +8 | diff --git a/ruff.schema.json b/ruff.schema.json index 967befb8bd346c..9dbae611cc90a6 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4218,6 +4218,7 @@ "RUF07", "RUF070", "RUF071", + "RUF072", "RUF1", "RUF10", "RUF100", From f10de7a0ebd1fb79eafd39d72210cc70a848b40a Mon Sep 17 00:00:00 2001 From: seroperson Date: Wed, 25 Mar 2026 14:06:21 +0300 Subject: [PATCH 2/2] Addressing review --- .../resources/test/fixtures/ruff/RUF072.py | 56 ++++- .../test/fixtures/ruff/RUF072_RUF047.py | 10 + .../fixtures/ruff/RUF072_RUF047_SIM105.py | 10 + crates/ruff_linter/src/rules/ruff/mod.rs | 22 ++ .../src/rules/ruff/rules/useless_finally.rs | 31 +-- ...uff__tests__preview__RUF072_RUF072.py.snap | 193 +++++++++++++----- ...__useless_finally_and_needless_else-2.snap | 6 + ...ts__useless_finally_and_needless_else.snap | 52 ++++- ...ess_else_and_suppressible_exception-2.snap | 9 + ...dless_else_and_suppressible_exception.snap | 38 ++++ crates/ruff_python_ast/src/helpers.rs | 12 ++ 11 files changed, 355 insertions(+), 84 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047_SIM105.py create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else_and_suppressible_exception-2.snap create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else_and_suppressible_exception.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py index e5b49e57bf9c7e..f2ad1a58799bcd 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py @@ -1,6 +1,6 @@ # Errors -# Case A: try/except/finally with pass +# try/except/finally with pass try: foo() except Exception: @@ -8,7 +8,7 @@ finally: pass -# Case A: try/except/finally with ellipsis +# try/except/finally with ellipsis try: foo() except Exception: @@ -16,7 +16,7 @@ finally: ... -# Case A: try/except/else/finally with pass +# try/except/else/finally with pass try: foo() except Exception: @@ -26,19 +26,19 @@ finally: pass -# Case B: bare try/finally with pass +# bare try/finally with pass try: foo() finally: pass -# Case B: bare try/finally with ellipsis +# bare try/finally with ellipsis try: foo() finally: ... -# Case B: bare try/finally with multi-line body +# bare try/finally with multi-line body try: foo() bar() @@ -55,6 +55,22 @@ except Exception: bar() +# finally with two pass statements +try: + foo() +except Exception: + bar() +finally: + pass + pass + +# bare try/finally with pass and ellipsis +try: + foo() +finally: + pass + ... + # OK # finally with real code @@ -121,6 +137,34 @@ # comment pass +# Own-line comment extra-indented before `pass` +try: + foo() +except Exception: + bar() +finally: + # comment + pass + +# Trailing comment indented one level (belongs to finally body) +try: + foo() +except Exception: + bar() +finally: + pass + # indented comment + +# Trailing comment dedented one level (not part of finally, but +# immediately adjacent — suppresses fix conservatively) +try: + foo() +except Exception: + bar() +finally: + pass +# dedented comment + # Comment on bare try/finally try: foo() diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py index 0733e20a09dfa4..3899d653f102cf 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py @@ -9,6 +9,16 @@ finally: pass +# All non-body clauses are no-ops +try: + foo() +except Exception: + pass +else: + pass +finally: + pass + # Only the `finally` is empty; `else` has real code try: foo() diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047_SIM105.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047_SIM105.py new file mode 100644 index 00000000000000..557de28662f468 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047_SIM105.py @@ -0,0 +1,10 @@ +# All three non-body clauses are no-ops — after all rules converge, +# only `contextlib.suppress(Exception): foo()` remains +try: + foo() +except Exception: + pass +else: + pass +finally: + pass diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 32cd2f32fb897c..78ae80e1026eda 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -177,6 +177,28 @@ mod tests { Ok(()) } + /// Test that RUF072 + RUF047 + SIM105 converge when all three non-body + /// clauses (except, else, finally) are no-ops + #[test] + fn useless_finally_and_needless_else_and_suppressible_exception() -> Result<()> { + use ruff_python_ast::{PySourceType, SourceType}; + + let path = test_resource_path("fixtures").join("ruff/RUF072_RUF047_SIM105.py"); + let source_type = SourceType::Python(PySourceType::from(&path)); + let source_kind = SourceKind::from_path(&path, source_type)?.expect("valid source"); + let settings = settings::LinterSettings::for_rules(vec![ + Rule::UselessFinally, + Rule::NeedlessElse, + Rule::SuppressibleException, + ]); + + let (diagnostics, transformed) = test_contents(&source_kind, &path, &settings); + assert_diagnostics!(diagnostics); + + insta::assert_snapshot!(transformed.source_code()); + Ok(()) + } + #[test] fn missing_fstring_syntax_backslash_py311() -> Result<()> { assert_diagnostics_diff!( diff --git a/crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs b/crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs index f69be18d1f4020..86028209ff9856 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs @@ -1,10 +1,10 @@ use std::cmp::Ordering; use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::helpers::comment_indentation_after; +use ruff_python_ast::helpers::{comment_indentation_after, is_stub_body}; use ruff_python_ast::token::TokenKind; use ruff_python_ast::whitespace::indentation; -use ruff_python_ast::{Stmt, StmtExpr, StmtTry}; +use ruff_python_ast::{Stmt, StmtTry}; use ruff_source_file::LineRanges; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -59,7 +59,7 @@ use crate::{Edit, Fix, FixAvailability, Violation}; /// so RUF072 must remove it first /// - [`useless-try-except`][TRY203]: Flags `try/except` that only re-raises #[derive(ViolationMetadata)] -#[violation_metadata(preview_since = "0.15.7")] +#[violation_metadata(preview_since = "NEXT_RUFF_VERSION")] pub(crate) struct UselessFinally; impl Violation for UselessFinally { @@ -79,19 +79,19 @@ impl Violation for UselessFinally { pub(crate) fn useless_finally(checker: &Checker, try_stmt: &StmtTry) { let finalbody = &try_stmt.finalbody; - if !body_is_no_op(finalbody) { + if !is_stub_body(finalbody) { return; } let source = checker.source(); let tokens = checker.tokens(); - // `body_is_no_op` guarantees exactly one statement in finalbody - let finalbody_stmt = &finalbody[0]; + // `is_stub_body` guarantees at least one statement in finalbody + let last_finalbody_stmt = finalbody.last().unwrap(); let (preceding_end, preceding_stmt) = preceding_clause_info(try_stmt); let Some(finally_start) = tokens - .in_range(TextRange::new(preceding_end, finalbody_stmt.end())) + .in_range(TextRange::new(preceding_end, last_finalbody_stmt.end())) .iter() .find(|token| token.kind() == TokenKind::Finally) .map(Ranged::start) @@ -99,12 +99,12 @@ pub(crate) fn useless_finally(checker: &Checker, try_stmt: &StmtTry) { return; }; - let finally_range = TextRange::new(finally_start, finalbody_stmt.end()); + let finally_range = TextRange::new(finally_start, last_finalbody_stmt.end()); let has_comments = finally_contains_comments( preceding_stmt, preceding_end, - finalbody_stmt, + last_finalbody_stmt, finally_range, checker, ); @@ -141,28 +141,19 @@ pub(crate) fn useless_finally(checker: &Checker, try_stmt: &StmtTry) { }; let try_line_start = source.line_start(try_stmt.start()); - let finally_full_end = source.full_line_end(finalbody_stmt.end()); + let finally_full_end = source.full_line_end(last_finalbody_stmt.end()); let edit = Edit::range_replacement(adjusted, TextRange::new(try_line_start, finally_full_end)); diagnostic.set_fix(Fix::safe_edit(edit)); } else { // `try/except/finally: pass` — remove the finally clause let finally_line_start = source.line_start(finally_start); - let finally_full_end = source.full_line_end(finalbody_stmt.end()); + let finally_full_end = source.full_line_end(last_finalbody_stmt.end()); let edit = Edit::range_deletion(TextRange::new(finally_line_start, finally_full_end)); diagnostic.set_fix(Fix::safe_edit(edit)); } } -/// Whether `body` contains only one `pass` or `...` statement -fn body_is_no_op(body: &[Stmt]) -> bool { - match body { - [Stmt::Pass(_)] => true, - [Stmt::Expr(StmtExpr { value, .. })] => value.is_ellipsis_literal_expr(), - _ => false, - } -} - /// Returns the end offset of the clause preceding `finally` and the last /// statement in that clause's body (used for comment indentation checks) fn preceding_clause_info(try_stmt: &StmtTry) -> (TextSize, Option<&Stmt>) { diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap index 8b3493466132c8..cef2321bc5c0bc 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap @@ -10,7 +10,7 @@ RUF072 [*] Empty `finally` clause 9 | | pass | |________^ 10 | -11 | # Case A: try/except/finally with ellipsis +11 | # try/except/finally with ellipsis | help: Remove the `finally` clause 5 | foo() @@ -19,7 +19,7 @@ help: Remove the `finally` clause - finally: - pass 8 | -9 | # Case A: try/except/finally with ellipsis +9 | # try/except/finally with ellipsis 10 | try: RUF072 [*] Empty `finally` clause @@ -31,7 +31,7 @@ RUF072 [*] Empty `finally` clause 17 | | ... | |_______^ 18 | -19 | # Case A: try/except/else/finally with pass +19 | # try/except/else/finally with pass | help: Remove the `finally` clause 13 | foo() @@ -40,7 +40,7 @@ help: Remove the `finally` clause - finally: - ... 16 | -17 | # Case A: try/except/else/finally with pass +17 | # try/except/else/finally with pass 18 | try: RUF072 [*] Empty `finally` clause @@ -52,7 +52,7 @@ RUF072 [*] Empty `finally` clause 27 | | pass | |________^ 28 | -29 | # Case B: bare try/finally with pass +29 | # bare try/finally with pass | help: Remove the `finally` clause 23 | bar() @@ -61,7 +61,7 @@ help: Remove the `finally` clause - finally: - pass 26 | -27 | # Case B: bare try/finally with pass +27 | # bare try/finally with pass 28 | try: RUF072 [*] Empty `finally` clause @@ -73,19 +73,19 @@ RUF072 [*] Empty `finally` clause 33 | | pass | |________^ 34 | -35 | # Case B: bare try/finally with ellipsis +35 | # bare try/finally with ellipsis | help: Remove the `finally` clause 27 | pass 28 | -29 | # Case B: bare try/finally with pass +29 | # bare try/finally with pass - try: - foo() - finally: - pass 30 + foo() 31 | -32 | # Case B: bare try/finally with ellipsis +32 | # bare try/finally with ellipsis 33 | try: RUF072 [*] Empty `finally` clause @@ -97,19 +97,19 @@ RUF072 [*] Empty `finally` clause 39 | | ... | |_______^ 40 | -41 | # Case B: bare try/finally with multi-line body +41 | # bare try/finally with multi-line body | help: Remove the `finally` clause 33 | pass 34 | -35 | # Case B: bare try/finally with ellipsis +35 | # bare try/finally with ellipsis - try: - foo() - finally: - ... 36 + foo() 37 | -38 | # Case B: bare try/finally with multi-line body +38 | # bare try/finally with multi-line body 39 | try: RUF072 [*] Empty `finally` clause @@ -126,7 +126,7 @@ RUF072 [*] Empty `finally` clause help: Remove the `finally` clause 39 | ... 40 | -41 | # Case B: bare try/finally with multi-line body +41 | # bare try/finally with multi-line body - try: - foo() - bar() @@ -164,78 +164,165 @@ help: Remove the `finally` clause 53 | bar() 54 | -RUF072 Empty `finally` clause - --> RUF072.py:86:1 +RUF072 [*] Empty `finally` clause + --> RUF072.py:63:1 | -84 | except Exception: -85 | bar() -86 | / finally: # comment -87 | | pass +61 | except Exception: +62 | bar() +63 | / finally: +64 | | pass +65 | | pass | |________^ -88 | -89 | # Comment on `pass` line +66 | +67 | # bare try/finally with pass and ellipsis | help: Remove the `finally` clause +60 | foo() +61 | except Exception: +62 | bar() + - finally: + - pass + - pass +63 | +64 | # bare try/finally with pass and ellipsis +65 | try: -RUF072 Empty `finally` clause - --> RUF072.py:94:1 +RUF072 [*] Empty `finally` clause + --> RUF072.py:70:1 | -92 | except Exception: -93 | bar() -94 | / finally: -95 | | pass # comment - | |________^ -96 | -97 | # Preceding own-line comment +68 | try: +69 | foo() +70 | / finally: +71 | | pass +72 | | ... + | |_______^ +73 | +74 | # OK | help: Remove the `finally` clause +65 | pass +66 | +67 | # bare try/finally with pass and ellipsis + - try: + - foo() + - finally: + - pass + - ... +68 + foo() +69 | +70 | # OK +71 | RUF072 Empty `finally` clause - --> RUF072.py:103:1 + --> RUF072.py:102:1 | +100 | except Exception: 101 | bar() -102 | # comment -103 | / finally: -104 | | pass +102 | / finally: # comment +103 | | pass | |________^ -105 | -106 | # Trailing own-line comment +104 | +105 | # Comment on `pass` line | help: Remove the `finally` clause RUF072 Empty `finally` clause - --> RUF072.py:111:1 + --> RUF072.py:110:1 | -109 | except Exception: -110 | bar() -111 | / finally: -112 | | pass +108 | except Exception: +109 | bar() +110 | / finally: +111 | | pass # comment | |________^ -113 | # comment +112 | +113 | # Preceding own-line comment | help: Remove the `finally` clause RUF072 Empty `finally` clause - --> RUF072.py:120:1 + --> RUF072.py:119:1 | -118 | except Exception: -119 | bar() -120 | / finally: -121 | | # comment -122 | | pass +117 | bar() +118 | # comment +119 | / finally: +120 | | pass | |________^ -123 | -124 | # Comment on bare try/finally +121 | +122 | # Trailing own-line comment | help: Remove the `finally` clause RUF072 Empty `finally` clause --> RUF072.py:127:1 | -125 | try: -126 | foo() -127 | / finally: # comment +125 | except Exception: +126 | bar() +127 | / finally: 128 | | pass | |________^ +129 | # comment + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:136:1 + | +134 | except Exception: +135 | bar() +136 | / finally: +137 | | # comment +138 | | pass + | |________^ +139 | +140 | # Own-line comment extra-indented before `pass` + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:145:1 + | +143 | except Exception: +144 | bar() +145 | / finally: +146 | | # comment +147 | | pass + | |________^ +148 | +149 | # Trailing comment indented one level (belongs to finally body) + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:154:1 + | +152 | except Exception: +153 | bar() +154 | / finally: +155 | | pass + | |________^ +156 | # indented comment + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:164:1 + | +162 | except Exception: +163 | bar() +164 | / finally: +165 | | pass + | |________^ +166 | # dedented comment + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:171:1 + | +169 | try: +170 | foo() +171 | / finally: # comment +172 | | pass + | |________^ | help: Remove the `finally` clause diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else-2.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else-2.snap index e293d05c026c72..ef2e026ef5b611 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else-2.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else-2.snap @@ -9,6 +9,12 @@ try: except Exception: bar() +# All non-body clauses are no-ops +try: + foo() +except Exception: + pass + # Only the `finally` is empty; `else` has real code try: foo() diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap index 9ff11b7d52de0c..fd8d88e4e50928 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap @@ -31,7 +31,7 @@ RUF072 [*] Empty `finally` clause 10 | | pass | |________^ 11 | -12 | # Only the `finally` is empty; `else` has real code +12 | # All non-body clauses are no-ops | help: Remove the `finally` clause 6 | bar() @@ -40,21 +40,63 @@ help: Remove the `finally` clause - finally: - pass 9 | -10 | # Only the `finally` is empty; `else` has real code +10 | # All non-body clauses are no-ops 11 | try: +RUF047 [*] Empty `else` clause + --> RUF072_RUF047.py:17:1 + | +15 | except Exception: +16 | pass +17 | / else: +18 | | pass + | |________^ +19 | finally: +20 | pass + | +help: Remove the `else` clause +14 | foo() +15 | except Exception: +16 | pass + - else: + - pass +17 | finally: +18 | pass +19 | + RUF072 [*] Empty `finally` clause --> RUF072_RUF047.py:19:1 | 17 | else: -18 | baz() +18 | pass 19 | / finally: 20 | | pass | |________^ +21 | +22 | # Only the `finally` is empty; `else` has real code | help: Remove the `finally` clause -16 | bar() +16 | pass 17 | else: -18 | baz() +18 | pass + - finally: + - pass +19 | +20 | # Only the `finally` is empty; `else` has real code +21 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072_RUF047.py:29:1 + | +27 | else: +28 | baz() +29 | / finally: +30 | | pass + | |________^ + | +help: Remove the `finally` clause +26 | bar() +27 | else: +28 | baz() - finally: - pass diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else_and_suppressible_exception-2.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else_and_suppressible_exception-2.snap new file mode 100644 index 00000000000000..f256003d446e67 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else_and_suppressible_exception-2.snap @@ -0,0 +1,9 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +expression: transformed.source_code() +--- +# All three non-body clauses are no-ops — after all rules converge, +# only `contextlib.suppress(Exception): foo()` remains +import contextlib +with contextlib.suppress(Exception): + foo() diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else_and_suppressible_exception.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else_and_suppressible_exception.snap new file mode 100644 index 00000000000000..89fbb461f8a1d2 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else_and_suppressible_exception.snap @@ -0,0 +1,38 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF047 [*] Empty `else` clause + --> RUF072_RUF047_SIM105.py:7:1 + | + 5 | except Exception: + 6 | pass + 7 | / else: + 8 | | pass + | |________^ + 9 | finally: +10 | pass + | +help: Remove the `else` clause +4 | foo() +5 | except Exception: +6 | pass + - else: + - pass +7 | finally: +8 | pass + +RUF072 [*] Empty `finally` clause + --> RUF072_RUF047_SIM105.py:9:1 + | + 7 | else: + 8 | pass + 9 | / finally: +10 | | pass + | |________^ + | +help: Remove the `finally` clause +6 | pass +7 | else: +8 | pass + - finally: + - pass diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 9680d03ec3f61a..37d8c5ac48b5ef 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1107,6 +1107,18 @@ pub fn is_docstring_stmt(stmt: &Stmt) -> bool { } } +/// Returns `true` if all statements in `body` are `pass` or `...` (ellipsis) +/// +/// An empty body (`[]`) returns `false` +pub fn is_stub_body(body: &[Stmt]) -> bool { + !body.is_empty() + && body.iter().all(|stmt| match stmt { + Stmt::Pass(_) => true, + Stmt::Expr(ast::StmtExpr { value, .. }) => value.is_ellipsis_literal_expr(), + _ => false, + }) +} + /// Check if a node is part of a conditional branch. pub fn on_conditional_branch<'a>(parents: &mut impl Iterator) -> bool { parents.any(|parent| {