diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF019.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF019.py index 89e22e9d97..62f1445aa1 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF019.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF019.py @@ -22,3 +22,11 @@ v = "k" in d and d["k"] if f() in d and d[f()]: pass + + +if ( + "key" in d + and # text + d ["key"] +): + ... diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_key_check.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_key_check.rs index 502391dcf2..5418561aae 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_key_check.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_key_check.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::{self as ast, BoolOp, CmpOp, Expr}; @@ -28,6 +29,9 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; /// if dct.get("key"): /// ... /// ``` +/// +/// ## Fix safety +/// This rule's fix is marked as safe, unless the expression contains comments. #[derive(ViolationMetadata)] #[violation_metadata(stable_since = "v0.2.0")] pub(crate) struct UnnecessaryKeyCheck; @@ -104,18 +108,28 @@ pub(crate) fn unnecessary_key_check(checker: &Checker, expr: &Expr) { } let mut diagnostic = checker.report_diagnostic(UnnecessaryKeyCheck, expr.range()); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - format!( - "{}.get({})", - checker.locator().slice( - parenthesized_range(obj_right.into(), right.into(), checker.tokens(),) - .unwrap_or(obj_right.range()) - ), - checker.locator().slice( - parenthesized_range(key_right.into(), right.into(), checker.tokens(),) - .unwrap_or(key_right.range()) + + let applicability = if checker.comment_ranges().intersects(expr.range()) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement( + format!( + "{}.get({})", + checker.locator().slice( + parenthesized_range(obj_right.into(), right.into(), checker.tokens(),) + .unwrap_or(obj_right.range()) + ), + checker.locator().slice( + parenthesized_range(key_right.into(), right.into(), checker.tokens(),) + .unwrap_or(key_right.range()) + ), ), + expr.range(), ), - expr.range(), - ))); + applicability, + )); } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF019_RUF019.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF019_RUF019.py.snap index ba96087e04..d2250c2c76 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF019_RUF019.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF019_RUF019.py.snap @@ -114,3 +114,26 @@ help: Replace with `dict.get` 19 | 20 | # OK 21 | v = "k" in d and d["k"] + +RUF019 [*] Unnecessary key check before dictionary access + --> RUF019.py:28:9 + | +27 | if ( +28 | / "key" in d +29 | | and # text +30 | | d ["key"] + | |_________________^ +31 | ): +32 | ... + | +help: Replace with `dict.get` +25 | +26 | +27 | if ( + - "key" in d + - and # text + - d ["key"] +28 + d.get("key") +29 | ): +30 | ... +note: This is an unsafe fix and may change runtime behavior