diff --git a/crates/ruff_linter/src/rules/refurb/rules/single_item_membership_test.rs b/crates/ruff_linter/src/rules/refurb/rules/single_item_membership_test.rs index 2d5cefacc4639..774fc4632146f 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/single_item_membership_test.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/single_item_membership_test.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::helpers::generate_comparison; use ruff_python_ast::{self as ast, CmpOp, Expr, ExprStringLiteral}; @@ -25,6 +25,12 @@ use crate::fix::edits::pad; /// 1 == 1 /// ``` /// +/// ## Fix safety +/// +/// When the right-hand side is a string, the fix is marked as unsafe. +/// This is because `c in "a"` is true both when `c` is `"a"` and when `c` is the empty string, +/// so the fix can change the behavior of your program in these cases. +/// /// ## References /// - [Python documentation: Comparisons](https://docs.python.org/3/reference/expressions.html#comparisons) /// - [Python documentation: Membership test operations](https://docs.python.org/3/reference/expressions.html#membership-test-operations) @@ -74,9 +80,9 @@ pub(crate) fn single_item_membership_test( return; }; - let mut diagnostic = - Diagnostic::new(SingleItemMembershipTest { membership_test }, expr.range()); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + let diagnostic = Diagnostic::new(SingleItemMembershipTest { membership_test }, expr.range()); + + let edit = Edit::range_replacement( pad( generate_comparison( left, @@ -90,8 +96,17 @@ pub(crate) fn single_item_membership_test( checker.locator(), ), expr.range(), - ))); - checker.diagnostics.push(diagnostic); + ); + + let applicability = if right.is_string_literal_expr() { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + let fix = Fix::applicable_edit(edit, applicability); + + checker.diagnostics.push(diagnostic.with_fix(fix)); } /// Return the single item wrapped in `Some` if the expression contains a single diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171.py.snap index 7547b029589ab..7124e3e5c7f8d 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171.py.snap @@ -71,7 +71,7 @@ FURB171.py:12:4: FURB171 [*] Membership test against single-item container | = help: Convert to equality test -ℹ Safe fix +ℹ Unsafe fix 9 9 | if 1 in {1}: 10 10 | print("Single-element set") 11 11 |