From e428099e4cba94ff7dfecbceef75f5491213548c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 5 Sep 2023 13:51:34 +0200 Subject: [PATCH] Add required space when fixing SIM118 (#7150) --- .../test/fixtures/flake8_simplify/SIM118.py | 6 +++ .../flake8_simplify/rules/key_in_dict.rs | 18 ++++++- ...ke8_simplify__tests__SIM118_SIM118.py.snap | 53 +++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM118.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM118.py index 182412e545..2d4df8507d 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM118.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM118.py @@ -42,3 +42,9 @@ class Foo: def __contains__(self, key: object) -> bool: return key in self.keys() # OK + + +# Regression test for: https://github.com/astral-sh/ruff/issues/7124 +key in obj.keys()and foo +(key in obj.keys())and foo +key in (obj.keys())and foo diff --git a/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs b/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs index f6bf6cf940..8d6124074f 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/key_in_dict.rs @@ -116,8 +116,24 @@ fn key_in_dict( TextRange::new(left_range.start(), right_range.end()), ); if checker.patch(diagnostic.kind.rule()) { + // If the `.keys()` is followed by (e.g.) a keyword, we need to insert a space, + // since we're removing parentheses, which could lead to invalid syntax, as in: + // ```python + // if key in foo.keys()and bar: + // ``` + let requires_space = checker + .locator() + .after(right_range.end()) + .chars() + .next() + .is_some_and(|char| char.is_ascii_alphabetic()); + diagnostic.set_fix(Fix::suggested(Edit::range_replacement( - value_content.to_string(), + if requires_space { + format!("{value_content} ") + } else { + value_content.to_string() + }, right_range, ))); } diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM118_SIM118.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM118_SIM118.py.snap index 09dac25801..09d3763e74 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM118_SIM118.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM118_SIM118.py.snap @@ -309,4 +309,57 @@ SIM118.py:34:1: SIM118 [*] Use `(key) in (obj or {})` instead of `(key) in (obj 36 36 | from typing import KeysView 37 37 | +SIM118.py:48:1: SIM118 [*] Use `key in obj` instead of `key in obj.keys()` + | +47 | # Regression test for: https://github.com/astral-sh/ruff/issues/7124 +48 | key in obj.keys()and foo + | ^^^^^^^^^^^^^^^^^ SIM118 +49 | (key in obj.keys())and foo +50 | key in (obj.keys())and foo + | + = help: Convert to `key in obj` + +ℹ Suggested fix +45 45 | +46 46 | +47 47 | # Regression test for: https://github.com/astral-sh/ruff/issues/7124 +48 |-key in obj.keys()and foo + 48 |+key in obj and foo +49 49 | (key in obj.keys())and foo +50 50 | key in (obj.keys())and foo + +SIM118.py:49:2: SIM118 [*] Use `key in obj` instead of `key in obj.keys()` + | +47 | # Regression test for: https://github.com/astral-sh/ruff/issues/7124 +48 | key in obj.keys()and foo +49 | (key in obj.keys())and foo + | ^^^^^^^^^^^^^^^^^ SIM118 +50 | key in (obj.keys())and foo + | + = help: Convert to `key in obj` + +ℹ Suggested fix +46 46 | +47 47 | # Regression test for: https://github.com/astral-sh/ruff/issues/7124 +48 48 | key in obj.keys()and foo +49 |-(key in obj.keys())and foo + 49 |+(key in obj)and foo +50 50 | key in (obj.keys())and foo + +SIM118.py:50:1: SIM118 [*] Use `key in obj` instead of `key in obj.keys()` + | +48 | key in obj.keys()and foo +49 | (key in obj.keys())and foo +50 | key in (obj.keys())and foo + | ^^^^^^^^^^^^^^^^^^^ SIM118 + | + = help: Convert to `key in obj` + +ℹ Suggested fix +47 47 | # Regression test for: https://github.com/astral-sh/ruff/issues/7124 +48 48 | key in obj.keys()and foo +49 49 | (key in obj.keys())and foo +50 |-key in (obj.keys())and foo + 50 |+key in obj and foo +