Skip to content

Commit

Permalink
[refurb] Mark fix as unsafe when the right-hand side is a string (`…
Browse files Browse the repository at this point in the history
…FURB171`) (astral-sh#15273)
  • Loading branch information
InSyncWithFoo authored Jan 5, 2025
1 parent eb82089 commit 00aa387
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down

0 comments on commit 00aa387

Please sign in to comment.