WIP Suggestion creation macro

[Jarcho]

This is a minimal and hacky implementation right now, but it should get the idea across.

A large number of suggestions in clippy are basically extracting a snippet of an expression (or multiple), adding something around it, and then replacing an expression. This is a proc-macro made to simplify that as much as possible. The syntax is a regular rust expression with meta-variables to insert snippets. It should handle keeping track of precedence and inserting parenthesis around snippets as needed, as well as keeping track of the final precedence

Some examples:

expr_sugg!(&*$snip(e).foo);
expr_sugg!(&$mut(m) *foo);
expr_sugg!(&$mut(m) foo[$snip(e)]);

changelog: None

[rust-highfive]

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

[llogiq]

Cool idea! I like how you did this without using syn. I do wonder how much this actually simplifies things though. Perhaps add another commit that uses it in a few lints once the tests pass so we can see the wins to be had?

[bors]

☔ The latest upstream changes (presumably #8001) made this pull request unmergeable. Please resolve the merge conflicts.

[llogiq]

Anything needed or should I review this now?

[Jarcho]

Still working away at it, slowly. Hopefully will be done this weekend. A good chunk of has been rewritten so there isn't really much of a point reviewing what's up there other than to see what the updated lints look like.

[camsteffen]

I think a good goal here would be for the macro to expand into code that could be written without the macro. This would take a lot of complexity out of the macro implementation.

For example, I would like to be able to write:

let sugg = Suggestion::borrow( // adds '&' and parenthesis according to arg precedence
    Suggestion::from_expr(cx, expr) // gets snippet
)
.position(cx, expr.hir_id) // adds outer parenthesis if needed when replacing `expr`
.to_string();

This is basically Sugg, but with more awareness of precedence.

[Jarcho]

That would strictly be more work. Currently it's being handled like a format string with the addition of keeping track of what precedence level snippets are being inserted at, and what the lowest precedence level in the expression is.

let sugg = Suggestion::borrow( // adds '&' and parenthesis according to arg precedence
    Suggestion::from_expr(cx, expr) // gets snippet
)
.position(cx, expr.hir_id) // adds outer parenthesis if needed when replacing `expr`
.to_string();

This kind of output requires building an ast as something like x * $snip * y and x + $snip * y would have to build the suggestion in different orders (the first being mul(mul(x, $snip), y) the second being add(x, mul($snip, y))).

That being said keeping Sugg around for more complicated things is still useful. (e.g. conditionally negating a suggestion)

[Jarcho]

Just playing around with syntax options:

1. $expr(expr1) + $expr(expr2)
2. $expr + $expr, expr1, expr2
3. {expr} + {expr}, expr1, expr2

There's also another option where the type of the expansion has a default value, and only needs to be specified sometimes:

1. $(expr1) + &$mut(mut1) foo
2. $_ + &$mut foo, expr1, mut1
3. {} + &{mut} foo, expr1, mut1

I like the look of the format string versions, but it does mean any block expressions would need double curly braces. (e.g. |x| { x } would need to be |x| {{ x }})

The types could also be specified after out of band (e.g. {} = &{} foo, expr1, mut: mut1), but that would require a second pass over the input as the trailing arguments would have to be parsed first.

[llogiq]

I still wonder whether the macro is going to pull its weight in terms of complexity. While we have a number of rather complex suggestions, I doubt they add up to 1.2k lines of code.

[Jarcho]

I have a few plans to shrink the amount of code, so there's still hope there.

Ultimately I'd like an interface along the lines of

lint_expr(cx, LINT_NAME, msg, expr, Applicability::MachineApplicable, sugg!($expr(foo) + 1));

This would cover a good chunk of lints which just replace one expression with another. Cover everything else with span_lint_and_then with some helper function to take the result of the sugg! macro.

[bors]

☔ The latest upstream changes (presumably #8266) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #8403) made this pull request unmergeable. Please resolve the merge conflicts.

[llogiq]

Sorry for letting you wait, I had a very busy week. This should be good to merge after yet another rebase.

[llogiq]

Sorry, I had a busy week. But another question arised: How does this mesh with the new diagnostics translation effort (which may include clippy in the future)?

[Jarcho]

As far as I can tell this shouldn't be any more work than what we would currently need to do.

[bors]

☔ The latest upstream changes (presumably #9421) made this pull request unmergeable. Please resolve the merge conflicts.

[blyxyas]

@Jarcho are you still interested in working on this? Or should it be closed? ヘ(^･ω･^=)~

[xFrednet]

Since there hasn't been any activity since the last comment asking about the status, I'm going to close this PR. @Jarcho you're welcome to reopen it or create a new one, if you're interested in continuing this work :D