Add autofix for PIE800 #8668
Add autofix for PIE800 #8668
Conversation
Whenever we see a **{...} inside another dictionary literal, just
delete the `**{` and `}` to inline the key-value pairs
crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_spread.rs
Outdated
Show resolved
Hide resolved
| 9 |- **{ | ||
| 9 |+ | ||
| 10 10 | "bar": 10 | ||
| 11 |- }, | ||
| 11 |+ , |
There was a problem hiding this comment.
Is there a requirement that the output be formatted nicely? The edit here is syntactically valid, but the formatting is going to be quite off after the fix if there are multiple indented keys.
There was a problem hiding this comment.
It's generally seen as "best effort" but not required.
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PIE800 | 6 | 0 | 0 | 6 | 0 |
| let mut diagnostic = Diagnostic::new(UnnecessarySpread, value.range()); | ||
| if checker.settings.preview.is_enabled() { | ||
| // Delete the `**{` | ||
| let tokenizer = BackwardsTokenizer::up_to( |
There was a problem hiding this comment.
Is there any way to restructure this to use SimpleTokenizer, i.e., forwards lexing? We've considered removing the backwards lexer in the past since it's quite complex, so I'd prefer to minimize usages if possible.
There was a problem hiding this comment.
Yeah, I think that should be possible:
- For deleting
**{, I can lex forward from the end of the previous value in the outer dictionary. - For deleting the last
}, I can lex forward from the last value of the inner dictionary.
I might have to change the function signature to passi n the whole dict (and not just the keys + values) to handle the case where {**{"a": "b"}} case where there's no previous value (in which case I'd lex forward from the dict start).
(might not get to this until tomorrow though).
There was a problem hiding this comment.
Ok -- I think this should work.
I ended up leaving the formatting fairly ugly -- the more I tried to make it look nice, the less confident I became that I handled all the edge-cases correctly (e.g. handling comments correctly, avoiding double commas, getting the indentation right, etc) so I ended up going for something simple and just letting an autoformatter figure it out afterwards.
I might try and take another crack at having the fix provide better formatted code later.
There was a problem hiding this comment.
Thanks for doing a second pass here!
charliermarsh
added
fixes
Summary
This adds an autofix for PIE800 (unnecessary spread) -- whenever we see a
**{...}inside another dictionary literal, just delete the**{and}to inline the key-value pairs. So{"a": "b", **{"c": "d"}}becomes just{"a": "b", "c": "d"}.I have enabled this just for preview mode.
Test Plan
Updated the preview snapshot test.