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..f2ad1a58799bcd --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py @@ -0,0 +1,172 @@ +# Errors + +# try/except/finally with pass +try: + foo() +except Exception: + bar() +finally: + pass + +# try/except/finally with ellipsis +try: + foo() +except Exception: + bar() +finally: + ... + +# try/except/else/finally with pass +try: + foo() +except Exception: + bar() +else: + baz() +finally: + pass + +# bare try/finally with pass +try: + foo() +finally: + pass + +# bare try/finally with ellipsis +try: + foo() +finally: + ... + +# 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() + +# 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 +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 + +# 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() +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..3899d653f102cf --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py @@ -0,0 +1,30 @@ +# 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 + +# 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() +except Exception: + bar() +else: + baz() +finally: + pass 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/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..78ae80e1026eda 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,70 @@ 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 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!( @@ -668,6 +733,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..86028209ff9856 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/useless_finally.rs @@ -0,0 +1,263 @@ +use std::cmp::Ordering; + +use ruff_macros::{ViolationMetadata, derive_message_formats}; +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, 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 = "NEXT_RUFF_VERSION")] +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 !is_stub_body(finalbody) { + return; + } + + let source = checker.source(); + let tokens = checker.tokens(); + + // `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, last_finalbody_stmt.end())) + .iter() + .find(|token| token.kind() == TokenKind::Finally) + .map(Ranged::start) + else { + return; + }; + + let finally_range = TextRange::new(finally_start, last_finalbody_stmt.end()); + + let has_comments = finally_contains_comments( + preceding_stmt, + preceding_end, + last_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(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(last_finalbody_stmt.end()); + let edit = Edit::range_deletion(TextRange::new(finally_line_start, finally_full_end)); + diagnostic.set_fix(Fix::safe_edit(edit)); + } +} + +/// 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..cef2321bc5c0bc --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF072_RUF072.py.snap @@ -0,0 +1,328 @@ +--- +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 | # try/except/finally with ellipsis + | +help: Remove the `finally` clause +5 | foo() +6 | except Exception: +7 | bar() + - finally: + - pass +8 | +9 | # 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 | # try/except/else/finally with pass + | +help: Remove the `finally` clause +13 | foo() +14 | except Exception: +15 | bar() + - finally: + - ... +16 | +17 | # 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 | # bare try/finally with pass + | +help: Remove the `finally` clause +23 | bar() +24 | else: +25 | baz() + - finally: + - pass +26 | +27 | # 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 | # bare try/finally with ellipsis + | +help: Remove the `finally` clause +27 | pass +28 | +29 | # bare try/finally with pass + - try: + - foo() + - finally: + - pass +30 + foo() +31 | +32 | # bare try/finally with ellipsis +33 | try: + +RUF072 [*] Empty `finally` clause + --> RUF072.py:38:1 + | +36 | try: +37 | foo() +38 | / finally: +39 | | ... + | |_______^ +40 | +41 | # bare try/finally with multi-line body + | +help: Remove the `finally` clause +33 | pass +34 | +35 | # bare try/finally with ellipsis + - try: + - foo() + - finally: + - ... +36 + foo() +37 | +38 | # 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 | # 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:63:1 + | +61 | except Exception: +62 | bar() +63 | / finally: +64 | | pass +65 | | pass + | |________^ +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:70:1 + | +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:102:1 + | +100 | except Exception: +101 | bar() +102 | / finally: # comment +103 | | pass + | |________^ +104 | +105 | # Comment on `pass` line + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:110:1 + | +108 | except Exception: +109 | bar() +110 | / finally: +111 | | pass # comment + | |________^ +112 | +113 | # Preceding own-line comment + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:119:1 + | +117 | bar() +118 | # comment +119 | / finally: +120 | | pass + | |________^ +121 | +122 | # Trailing own-line comment + | +help: Remove the `finally` clause + +RUF072 Empty `finally` clause + --> RUF072.py:127:1 + | +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 new file mode 100644 index 00000000000000..ef2e026ef5b611 --- /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,24 @@ +--- +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() + +# All non-body clauses are no-ops +try: + foo() +except Exception: + pass + +# 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..fd8d88e4e50928 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__useless_finally_and_needless_else.snap @@ -0,0 +1,102 @@ +--- +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 | # All non-body clauses are no-ops + | +help: Remove the `finally` clause +6 | bar() +7 | else: +8 | pass + - finally: + - pass +9 | +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 | pass +19 | / finally: +20 | | pass + | |________^ +21 | +22 | # Only the `finally` is empty; `else` has real code + | +help: Remove the `finally` clause +16 | pass +17 | else: +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_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/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| { 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",