ssr: Allow replacing expressions with statements

[MarijnS95]

Depends on #6587

Until that is merged, the diff is https://github.com/MarijnS95/rust-analyzer/compare/stmt..replace-expr-with-stmt

---

Now that statements can be matched and replaced (#6587) some usecases require expressions to be replaced with statements as well. This happens when something that can ambiguously be an expression or statement like if and loop blocks appear in the last position of a block, as trailing expression. In this case a replacement pattern of the form if foo(){$a();}==>>$a(); will only substitute if blocks in the list of statements but not if they (implicitly) end up in the trailing expression, where they are not wrapped by an EXPR_STMT (but the pattern and template are, as parsing only succeeds for the stmt ==>> stmt case).

Instead of adding two rules that match an expression - and emit duplicate matching errors - allow the template for expressions to be a statement if it fails to parse as an expression.

---

Another gross change that does not seem to break any tests currently, but perhaps a safeguard should be added to only allow this kind of replacement in blocks by "pushing" the replacement template to the statement list and clearing the trailing expression?

CC @davidlattimore

[matklad]

bors d=davidlattimore

[bors]

✌️ davidlattimore can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

[davidlattimore]

Interesting. What happens if the search pattern matches somewhere a statement isn't valid? In this case the search pattern evaluates to the () type, which I guess is unlikely to be used in many such places. But still, I worry slightly that the rule can result in code that doesn't parse. It's already possible to write rules that produce code that doesn't type check, but so far, although I haven't thought too hard about it, I wasn't aware of being able to write rules that would make code not parse. That said, it might not be worthwhile the added complexity of trying to prevent this from happening.

[MarijnS95]

@davidlattimore The pattern in the tests if $a() {$b;} ==>> $b; already "assumes" the LHS is a "statement" without result because of the trailing semicolon after $b. Updating it to if $a() {$b} ==>> $b makes the test work on master already as $b now includes the full EXPR_STMT. For "true" expressions (not evaluating to ()) something like if $a() {$b} else {$c} ==>> $b works in much the same way, optionally allowing semicolons to be consumed by $b and $c. However, this doesn't work for the actual use-case this PR is made for: if $c.at($a) {$d.eat($b);} ==>> $c.eat($a);. Here the trailing semicolon can't be consumed, (not even by if $c.at($a) {$d.eat($b)$optional_semi} ==>> $c.eat($a)$optional_semi).

To answer your question there's nothing in place preventing users from adding or removing semicolons to make resulting code invalid, or replace expressions with something nonsensical entirely like functions or struct definitions. Don't know if we need/want to impose such limits though, in the end it is the user specifying exactly what they want to replace. Other than that it's hard to come up with plausible test cases where this would perform an undesired change - they must exist and it'd be great to cover them.

I haven't invested the time to check a potential implementation that limits such replacement to trailing block expressions only, but it already sounds complicated and error prone :)

[davidlattimore]

bors r+

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript (ubuntu-latest)
• TypeScript (windows-latest)