Skip to content
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
172 changes: 172 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py
Original file line number Diff line number Diff line change
@@ -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
30 changes: 30 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF072_RUF047.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF072_SIM105.py
Original file line number Diff line number Diff line change
@@ -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

3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
68 changes: 67 additions & 1 deletion crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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__{}_{}",
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;

Expand Down Expand Up @@ -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;

Expand Down
Loading
Loading