Open mcarton opened 8 years ago
because checking whether the mut is still needed after our suggestion is actually hard.
easy, leave it there, the next round of clippy or rustc lints will catch it. We have many lints that will trigger more (other) lints after being fixed
easy, leave it there, the next round of clippy or rustc lints will catch it. We have many lints that will trigger more (other) lints after being fixed
Meh.
I mean it, we can't possibly catch all resulting situations. If a lint causes a change in a match arm, two match arms might now be equal. But it's unrealistic to suggest merging the arms after the suggestion without going through rustc again.
Yes but if we know there is one out of two chances the suggestion might trigger a new warning, maybe we can show that.
I'd be okay with adding a "Play again?" option after rustfix saved the changes. Actually, it might be a good idea to suggest running cargo test
afterwards anyway. 😄
I'm not sure I like the idea of placeholders in replacement suggestions, tbh. Why not just write several spans to replace only certain parts of the input? If that conflicts with what the diagnostics format (either JSON or the new fancy human readable variant) supports, we should fix that. Clippy itself can of course write whatever it likes to its help messages (incl. concatenated suggestions with placeholders), as long as the JSON makes sense.@mcarton cleared that up in https://github.com/killercup/rustfix/issues/14#issuecomment-231754724
If there was a need for a placeholder I would suggest something like \x1d...\x1d
(where \x1d
is the ASCII group separator which will either not show up in terminal output or look funny, just try echo -e "\x1d...\x1d"
).
So, just a heads up: The <mut>
placeholder from USELESS_LET_IF_SEQ
lint is currently the only thing that makes running rustfix --clippy --yolo
on https://github.com/space-wizards/bsdiff-rs generate code that doesn't compile.
Clippy has some suggestions that will contain a placeholder. Most of the time they are
..
or_
¹, but fortunately sometimes it is something more useful. Eg. clippy might suggest to replacefor i in 0..vec.len()
byfor <item> in &vec
².useless_let_if_seq
can also suggestlet <mut> foo =
(because checking whether themut
is still needed after our suggestion is actually hard).A nice feature for a tool to replace suggestions automatically would be to ask the user what to write there. For that we might want to mark these placeholders somehow:
<…>
is nice but it has some problems:a < b && c > d
, but< b && c >
is obviously not a placeholder;<item>
above) or might want to remove (<mut>
above).IMO, we should use different characters for placeholder markers such that:
rustfix
to detect placeholders;<item>
and<mut>
.Unicode can help: eg. we could use
‹item›
for something the user should change and❪mut❫
for something the user should opt-in.Some considerations though:
‹›❪❫
work for me™);span_suggestion
today, 2) Clippy suggestions still need a lot of work to be automatically fixable and have a working POC.Cc @killercup.
1: we should kill that btw. 2: and in the future should be able to add a proper suggestion for all the
vec[i]
in the loop as well.