What about placeholders?

[mcarton]

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 replace for i in 0..vec.len() by for <item> in &vec². useless_let_if_seq can also suggest let <mut> foo = (because checking whether the mut 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:

• it's valid rust to write a < b && c > d, but < b && c > is obviously not a placeholder;
• there is no difference between something the user should rename (<item> above) or might want to remove (<mut> above).

IMO, we should use different characters for placeholder markers such that:

• it's obvious for the user they need to change something and they can't just copy&paste a suggestion if so;
• it's trivial for rustfix to detect placeholders;
• there is a difference between <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:

• we should chose characters present in common monospaced fonts (‹›❪❫ work for me™);
• Unicode might not be available (Windows anyone?);
• this should be marked in the JSON errors rather than as text in the suggestion but I'm not felling like making an RFC for now since 1) rustc makes really few uses of 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.

[oli-obk]

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

[mcarton]

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.

[oli-obk]

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.

[mcarton]

Yes but if we know there is one out of two chances the suggestion might trigger a new warning, maybe we can show that.

[killercup]

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").

[killercup]

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.