-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Description
Edit: I've written my current thoughts on the matter down in a blog post: https://veykril.github.io/posts/ide-proc-macros/#conclusion
This issue is meant as a place to collect information and ideas about the current completion(IDE support rather) dilemma we have with proc-macros.
Current State of Things
When typing inside an attributed item(or proc-macro) the user will inevitably create some form of syntax error at some point which currently causes those proc-macros to just bail out, emitting a compile_error! invocation that r-a will happily replace the entire item with. This has the downside that when typing in such an item, all ide features will momentarily stop working resulting in incorrect or straight up missing completions and syntax highlighting flickers(when semantic highlighting is being used).
First Attempt at Solving This
Our first idea to solve this was to nudge proc-macro authors towards doing more fallible expansions, meaning instead of only emitting a compile_error! on unexpected input, they should instead try to produce output as close as possible to what the macro would expand to in the happy case together with the compile_error!.
This actually seems to be not as good of an idea as we have first imagined.
It goes against syn's ways as the crate is designed to return early on errors, and in fact, as it turns out(I wasn't aware of this fact prior) attribute and derive macros have the invariant/contract1 that they should always receive TokenStreams that parse to valid rust syntax.
Quoting @dtolnay's comment on the matter1 which raises a lot of good points on this matter:
Passing syntactically invalid input to attribute macros, whether in the context of rustc or rust-analyzer, is not a good plan because doing the recovery, emitting diagnostics about the recovery, and then running macros on the original unrecovered input means that in general you're forcing macros to reimplement their own recovery independent and inconsistent with rustc/rust-analyzer — so rustc/rust-analyzer will diagnose what it thinks you meant, and the macro will diagnose what it thinks, and probably they won't align, resulting in dissonant user-facing messages. The correct behavior in my opinion is for rustc/rust-analyzer to perform its normal high-effort syntax recovery the same as for nodes that are not macro input, report diagnostics on that recovery as normal, then pass the recovered input for the attribute to proceed on.
Recovery in this context would involve rust-analyzer snipping out the nearest syntactically invalid syntax tree node and swapping in whatever syntactically valid placeholder/sentinel it wants in its place. It can then run the macro which will expand successfully, then provide autocompletion and other functionality to the programmer based on the position where it finds its sentinel in the expanded output, and the original snipped out syntax.
While the user is typing inside of attribute macro input this approach will give high quality results for the IDE in vastly more cases than rust-analyzer's current behavior, without trying to force changes for invalid macro input into all the attribute macros in the ecosystem.
Possible Solutions
So with this, we have a few options at hand currently:
- Nudge proc-macro authors to "fix" their macros.
For attributes and derives(both of which expect rust syntax) this does seem like a bad idea after all after considering the points dtolnay has raised.
For function-like proc macros on the other hand, as there is no rust syntax being passed as well as for macros that error out on semantic problems like expecting certain identifiers, in these cases we should still nudge proc-macro authors to make their macros recoverable.
As there is nothing r-a can do in those cases.
This also has problem that we would define an implicit interface2 for r-a relying on exactly this behaviour for proc-macros, basically running the danger that if we would ever want to change our mind on this we would cause a eco system churn. - Analyze the macro expansion output, and if it merely expands to a
compile_error!keep the original item. Uncertain whether this is feasable in the first place, but it sounds like a bad trade off in general as a lot of macros that change their input item will not reflect their change in this case properly due to them still failing expansion.
Still violates the proc-macro contract. - Fix up the input nodes in a heuristic manner with what we expect they should be for completions/snip them out. This would ideally give the best results if properly implemented, but is also the most difficult to get right.
- Do nothing. Obviously a choice, and obviously a not so wise one.
Still violates the proc-macro contract.
Fixing up the input nodes seems like the best approach to me personally, to reiterate that would mean:
- For attribute and derive macros, we will fix up the input items syntax errors as best as we can
- We ask proc-macro authors to still implement recovery strategies for when their macros receive unexpected but syntactic valid input, as IDEs fundamentally need help here from macro.
Now we aren't the only IDE for rust, IntelliJ seems to currently hardcode a bunch of popular attributes to special case(ignore) them3. So this obviously also weighs into the decision and we should agree on what to do here so that people do not have to write specific adjustments for each IDE(that would be a truly nightmare-ish scenario). cc @vlad20012
Assuming we go with the approach of fixing up/snipping out syntax invalid syntax nodes. The question remains how to do this reliably in a way that enables completion to work as good, if not better than with proc-macros trying to recover from everything as our initial plan was. This is the main question we would have to resolve here next.
On a side-note not relevant to fixing up syntax nodes, regarding completions with proc-macros, there is potentially interesting trick we could try to make use of to get proc-macros to help out IDEs with guiding completions outlined here4. This is not relevant to fixing up syntax nodes though.
Footnotes
-
https://github.com/rust-analyzer/rust-analyzer/issues/10468#issuecomment-975480478 ↩ ↩2
-
https://github.com/rust-analyzer/rust-analyzer/issues/10468#issuecomment-975315065 ↩
-
https://github.com/rust-analyzer/rust-analyzer/issues/10468#issuecomment-975436728 ↩
-
https://github.com/rust-analyzer/rust-analyzer/issues/7402#issuecomment-770196608 ↩