From f2eb7bcacf22a02b70792fe3ad1876b5b55611f9 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Fri, 25 Aug 2023 11:02:31 -0500 Subject: [PATCH] Update ERA100 to apply to commented dictionary items with trailing comments (#6822) Closes https://github.com/astral-sh/ruff/issues/6821 ERA100 was not raising on commented parts of dictionaries if it included another comment (such as a noqa clause). In cases where this comment was a noqa clause, RUF100 to be emitted since the noqa would have no effect. Here, we update ERA100 to raise even when there are trailing comments. This resolves the linked issue _and_ increases the scope of ERA100. We could narrow the regular expression to only apply to noqa comments if we do not want to expand ERA100 however I think this change is within the spirit of the rule. --- .../test/fixtures/eradicate/ERA001.py | 9 ++++ .../resources/test/fixtures/ruff/RUF100_5.py | 11 ++++ crates/ruff/src/rules/eradicate/detection.rs | 2 +- ...s__eradicate__tests__ERA001_ERA001.py.snap | 42 ++++++++++++++++ crates/ruff/src/rules/ruff/mod.rs | 15 ++++++ .../ruff__rules__ruff__tests__ruf100_5.snap | 50 +++++++++++++++++++ 6 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 crates/ruff/resources/test/fixtures/ruff/RUF100_5.py create mode 100644 crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruf100_5.snap diff --git a/crates/ruff/resources/test/fixtures/eradicate/ERA001.py b/crates/ruff/resources/test/fixtures/eradicate/ERA001.py index a21d51c4ca94d..56e433f395b13 100644 --- a/crates/ruff/resources/test/fixtures/eradicate/ERA001.py +++ b/crates/ruff/resources/test/fixtures/eradicate/ERA001.py @@ -19,3 +19,12 @@ def foo(x, y, z): class A(): pass # b = c + + +dictionary = { + # "key1": 123, # noqa: ERA001 + # "key2": 456, + # "key3": 789, # test +} + +#import os # noqa diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF100_5.py b/crates/ruff/resources/test/fixtures/ruff/RUF100_5.py new file mode 100644 index 0000000000000..d5d7f47b02cff --- /dev/null +++ b/crates/ruff/resources/test/fixtures/ruff/RUF100_5.py @@ -0,0 +1,11 @@ +#import os # noqa +#import os # noqa: ERA001 + +dictionary = { + # "key1": 123, # noqa: ERA001 + # "key2": 456, # noqa + # "key3": 789, +} + + +#import os # noqa: E501 diff --git a/crates/ruff/src/rules/eradicate/detection.rs b/crates/ruff/src/rules/eradicate/detection.rs index a424d164ffda8..77be87c048f1b 100644 --- a/crates/ruff/src/rules/eradicate/detection.rs +++ b/crates/ruff/src/rules/eradicate/detection.rs @@ -28,7 +28,7 @@ static HASH_NUMBER: Lazy = Lazy::new(|| Regex::new(r"#\d").unwrap()); static MULTILINE_ASSIGNMENT_REGEX: Lazy = Lazy::new(|| Regex::new(r"^\s*([(\[]\s*)?(\w+\s*,\s*)*\w+\s*([)\]]\s*)?=.*[(\[{]$").unwrap()); static PARTIAL_DICTIONARY_REGEX: Lazy = - Lazy::new(|| Regex::new(r#"^\s*['"]\w+['"]\s*:.+[,{]\s*$"#).unwrap()); + Lazy::new(|| Regex::new(r#"^\s*['"]\w+['"]\s*:.+[,{]\s*(#.*)?$"#).unwrap()); static PRINT_RETURN_REGEX: Lazy = Lazy::new(|| Regex::new(r"^(print|return)\b\s*").unwrap()); /// Returns `true` if a comment contains Python code. diff --git a/crates/ruff/src/rules/eradicate/snapshots/ruff__rules__eradicate__tests__ERA001_ERA001.py.snap b/crates/ruff/src/rules/eradicate/snapshots/ruff__rules__eradicate__tests__ERA001_ERA001.py.snap index 17ccde3ca7e5e..18aa504d2b5bc 100644 --- a/crates/ruff/src/rules/eradicate/snapshots/ruff__rules__eradicate__tests__ERA001_ERA001.py.snap +++ b/crates/ruff/src/rules/eradicate/snapshots/ruff__rules__eradicate__tests__ERA001_ERA001.py.snap @@ -105,5 +105,47 @@ ERA001.py:21:5: ERA001 [*] Found commented-out code 19 19 | class A(): 20 20 | pass 21 |- # b = c +22 21 | +23 22 | +24 23 | dictionary = { + +ERA001.py:26:5: ERA001 [*] Found commented-out code + | +24 | dictionary = { +25 | # "key1": 123, # noqa: ERA001 +26 | # "key2": 456, + | ^^^^^^^^^^^^^^ ERA001 +27 | # "key3": 789, # test +28 | } + | + = help: Remove commented-out code + +ℹ Possible fix +23 23 | +24 24 | dictionary = { +25 25 | # "key1": 123, # noqa: ERA001 +26 |- # "key2": 456, +27 26 | # "key3": 789, # test +28 27 | } +29 28 | + +ERA001.py:27:5: ERA001 [*] Found commented-out code + | +25 | # "key1": 123, # noqa: ERA001 +26 | # "key2": 456, +27 | # "key3": 789, # test + | ^^^^^^^^^^^^^^^^^^^^^^ ERA001 +28 | } + | + = help: Remove commented-out code + +ℹ Possible fix +24 24 | dictionary = { +25 25 | # "key1": 123, # noqa: ERA001 +26 26 | # "key2": 456, +27 |- # "key3": 789, # test +28 27 | } +29 28 | +30 29 | #import os # noqa diff --git a/crates/ruff/src/rules/ruff/mod.rs b/crates/ruff/src/rules/ruff/mod.rs index be54ab37f2f7c..79028c9b5b235 100644 --- a/crates/ruff/src/rules/ruff/mod.rs +++ b/crates/ruff/src/rules/ruff/mod.rs @@ -165,6 +165,21 @@ mod tests { Ok(()) } + #[test] + fn ruf100_5() -> Result<()> { + let diagnostics = test_path( + Path::new("ruff/RUF100_5.py"), + &settings::Settings { + ..settings::Settings::for_rules(vec![ + Rule::UnusedNOQA, + Rule::LineTooLong, + Rule::CommentedOutCode, + ]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } #[test] fn flake8_noqa() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruf100_5.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruf100_5.snap new file mode 100644 index 0000000000000..9f212869a4dee --- /dev/null +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruf100_5.snap @@ -0,0 +1,50 @@ +--- +source: crates/ruff/src/rules/ruff/mod.rs +--- +RUF100_5.py:7:5: ERA001 [*] Found commented-out code + | +5 | # "key1": 123, # noqa: ERA001 +6 | # "key2": 456, # noqa +7 | # "key3": 789, + | ^^^^^^^^^^^^^^ ERA001 +8 | } + | + = help: Remove commented-out code + +ℹ Possible fix +4 4 | dictionary = { +5 5 | # "key1": 123, # noqa: ERA001 +6 6 | # "key2": 456, # noqa +7 |- # "key3": 789, +8 7 | } +9 8 | +10 9 | + +RUF100_5.py:11:1: ERA001 [*] Found commented-out code + | +11 | #import os # noqa: E501 + | ^^^^^^^^^^^^^^^^^^^^^^^^ ERA001 + | + = help: Remove commented-out code + +ℹ Possible fix +8 8 | } +9 9 | +10 10 | +11 |-#import os # noqa: E501 + +RUF100_5.py:11:13: RUF100 [*] Unused `noqa` directive (unused: `E501`) + | +11 | #import os # noqa: E501 + | ^^^^^^^^^^^^ RUF100 + | + = help: Remove unused `noqa` directive + +ℹ Suggested fix +8 8 | } +9 9 | +10 10 | +11 |-#import os # noqa: E501 + 11 |+#import os + +