fix(ERA001): detect commented out case statements, add more one-line support
#10055
Conversation
| @@ -47,7 +47,8 @@ fn is_standalone_comment(line: &str) -> bool { | |||
| for char in line.chars() { | |||
| if char == '#' { | |||
| return true; | |||
| } else if !char.is_whitespace() { | |||
| } | |||
| if !char.is_whitespace() { | |||
There was a problem hiding this comment.
Was this change intentional? Just checking.
There was a problem hiding this comment.
Yes, but I can revert! Just was a style lint in VSCode
| @@ -26,7 +26,7 @@ static HASH_NUMBER: Lazy<Regex> = Lazy::new(|| Regex::new(r"#\d").unwrap()); | |||
| static POSITIVE_CASES: Lazy<RegexSet> = Lazy::new(|| { | |||
| RegexSet::new([ | |||
| // Keywords | |||
| r"^(?:elif\s+.*\s*:|else\s*:|try\s*:|finally\s*:|except\s+.*\s*:)$", | |||
| r"^(?:elif\s+.*\s*:|else\s*:|try\s*:|finally\s*:|except\s+.*\s*:|case\s+.*\s*:.*)$", | |||
There was a problem hiding this comment.
note: this group ends with .* to detect the one-line case:
case "hi": print()There was a problem hiding this comment.
I feel like the other cases don't catch that -- can you try to add a test case with try followed by a one-line body?
There was a problem hiding this comment.
Yep - I think this will catch more non-code statements though like "to try: use iterators" or something. I'll write some tests to see
There was a problem hiding this comment.
Ok seems to work well, should I add .* for all of the cases, and add tests?
There was a problem hiding this comment.
Let's remove the trailing .* from this PR, and we can open a separate PR to add .* to all of the cases. It'll make it easier to approve and merge these as separate changes.
There was a problem hiding this comment.
Cool will do
There was a problem hiding this comment.
Ah sorry, I meant the opposite: can we remove the .* from here, so that we don't support one-line for any of the patterns (like before this PR), and then open a separate PR that adds the .* to all of them at once?
There was a problem hiding this comment.
Yep, that's what I understood you to mean! I'll do that and open an issue to add .* in another PR
There was a problem hiding this comment.
Done! Ticket opened, I'll open that PR next.
There was a problem hiding this comment.
Yep, that's what I understood you to mean! I'll do that and open an issue to add .* in another PR
(Oops, sorry, I think I was looking at the code, and thought you'd already pushed!)
ottaviohartman
changed the title
case statements, add more one-line support
|
Thank you! |
…e support (astral-sh#10055) ## Summary Closes astral-sh#10031 - Detect commented out `case` statements. Playground repro: https://play.ruff.rs/5a305aa9-6e5c-4fa4-999a-8fc427ab9a23 - Add more support for one-line commented out code. ## Test Plan Unit tested and tested with ```sh cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/eradicate/ERA001.py --no-cache --preview --select ERA001 ``` TODO: - [x] `cargo insta test`
Summary
Closes #10031
casestatements. Playground repro: https://play.ruff.rs/5a305aa9-6e5c-4fa4-999a-8fc427ab9a23Test Plan
Unit tested and tested with
TODO:
cargo insta test