rust-clippy
rust-clippy copied to clipboard
rust-lang
WIP Suggestion creation macro
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
r? @llogiq
(rust-highfive has picked a reviewer for you, use r? to override)
:umbrella: The latest upstream changes (presumably #8001) made this pull request unmergeable. Please resolve the merge conflicts.
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.
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.
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)
Just playing around with syntax options:
-
$expr(expr1) + $expr(expr2) -
$expr + $expr, expr1, expr2 -
{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:
-
$(expr1) + &$mut(mut1) foo -
$_ + &$mut foo, expr1, mut1 -
{} + &{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.
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.
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.
:umbrella: The latest upstream changes (presumably #8266) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #8403) made this pull request unmergeable. Please resolve the merge conflicts.
Sorry for letting you wait, I had a very busy week. This should be good to merge after yet another rebase.
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)?
As far as I can tell this shouldn't be any more work than what we would currently need to do.
:umbrella: The latest upstream changes (presumably #9421) made this pull request unmergeable. Please resolve the merge conflicts.
@Jarcho are you still interested in working on this? Or should it be closed? ヘ(^･ω･^=)~
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
