Autocomplete degradation

[edmand46]

Rust-Analyzer: 0.2.760 (9 days ago release)

All fine, completion works

Rust-Analyzer: 0.2.768 (2 days ago release, latest release)

All configration same, but autocompletion working more worse

Versions:

VSCode : 1.60.2 (user setup)
Commit: 7f6ab5485bbc008386c4386d08766667e155244e
Date: 2021-09-22T12:00:31.514Z
Electron: 13.1.8
Chrome: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Windows_NT x64 10.0.19043

rustup 1.24.3 (ce5817a94 2021-05-31)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active rustc version is rustc 1.54.0 (a178d0322 2021-07-26)

[Veykril]

This is due to the newest version enabling attribute macro expansion by default, as such completion doesn't work properly in actix_web::main attributed functions anymore because the proc-macro emits an error instead of recovering on a parse error(it fails to parse the function as you now have an invalid statement while typing in the middle of it).

So you can either lift your code out of the main function into a new one so that you don't need to type in the attributed function or set "rust-analyzer.experimental.procAttrMacros" :false in your settings.

[flodiebold]

Also it might start to make sense to open issues at the respective proc macro's repositories, since this will need changes in the proc macros to be properly fixed.

[Veykril]

Ye, though I doubt there will be much movement without a syn replacement/addition offering parsing with recovery.

[bjorn3]

Would it make sense to pretend that the macro attribute is an identity function if it failed to expand? At least for autocompletion?

[flodiebold]

Proc macros can't really fail to expand per se, they'll just expand to something else (a compiler error macro call). So that would require some heuristics, apart from causing other weirdness.

[flodiebold]

It's worth noting that macros like actix_web::main don't even really need error recovery in the parser -- the only thing the macro does with the function body is paste it somewhere else as a block, so it would work perfectly if it just didn't try to parse the body.

[Veykril]

This would be a simple solution for a lot of these attributes though. That is if it fails, just re-emit the attributed itemt together with the compile_error invocation. That should work out just fine for most attributes I imagine.

[Veykril]

Ye that seems to be a decent solution for most of these attributes, just tested this with tokios function attributes.

[Veykril]

Will open an issue for actix_web and tokio respectively as this is something the crates can fix themselves rather easily.
tokio-rs/tokio#4154
actix/actix-web#2394

[flodiebold]

Just emitting the original item kind of works as a workaround, but isn't really the solution we should recommend I think, since presumably the attribute macro is supposed to do something, which might have an effect on completions, which would then still only work when the item parses correctly. (E.g. the rocket macros from #10470 adding macros to the scope.)

[Veykril]

Ye good point, I'll clarify that this is a simple solution to avoid the main problem and that a better solution would be to add better recovery 👍

[Veykril]

I'll close this as this is technically not a RA issue and a corresponding library issue has been raised

[edmand46]

This is due to the newest version enabling attribute macro expansion by default, as such completion doesn't work properly in actix_web::main attributed functions anymore because the proc-macro emits an error instead of recovering on a parse error(it fails to parse the function as you now have an invalid statement while typing in the middle of it).

So you can either lift your code out of the main function into a new one so that you don't need to type in the attributed function or set "rust-analyzer.experimental.procAttrMacros" :false in your settings.

it's worked

[matklad]

cc https://old.reddit.com/r/rust/comments/qyxmsd/blog_post_ides_and_macros/hlkei16/

There’s a valid concern that maybe we shouldn’t ask crate authors to adjust their code to make it play nicely with rust-analyzer, and instead fix ra instead.

Without investigating to deeply:

• it seems that we never should asks to adjust “valid” cases, except for what be great for rustc as well (eg, using proper spans)
• for invalid and “incomplete” cases, I feel that we fundamentally need help from proc macros, and here the strategy works OK
• it, however, should be documented. @Veykril could you add a section to the manual which addresses proc macro authors? Might make sense to rope dtolnay and vlad20012 into discussions as well
• l worry a lot that this implicit interface for proc macros becomes new stability boundary of rust-analyzer. Ie, if we change our mind about how it should work later, that’ll be painful for the ecosystem.

[flodiebold]

I think everyone agrees that we shouldn't ask to adjust valid cases, and this:

for invalid and “incomplete” cases, I feel that we fundamentally need help from proc macros, and here the strategy works OK

is what dtolnay disagrees with. I also think that it is not possible to fully handle erroneous syntax in macro inputs if the macro doesn't cooperate, but it is true that we could be making more of an effort to make it work in common cases. I just worry this would get us to a 'mostly works, but janky' state and get us stuck there.

[flodiebold]

@matklad do you know how IntelliJ-Rust deals with this? They're able to expand attribute macros now as well, aren't they?

[matklad]

l worry a lot that this implicit interface for proc macros becomes new stability boundary of rust-analyzer

This I feel is a rather important point, send #10828 about that.

Do you know how IntelliJ-Rust deals with this?

Not really, though @vlad20012 pings me relatively frequently about designing some kind of API for proc macros, with a tag-line "every proc macro is its own LSP server".

I guess we can start with a shared hackmd between IJ and RA teams, and then rope proc macro folks in?

[Veykril]

I guess we can start with a shared hackmd between IJ and RA teams, and then rope proc macro folks in?

This sounds like the best step to me for now, noting down what we know about the current state of things might give us some better insight on how to address this.

[vlad20012]

I guess we can start with a shared hackmd between IJ and RA teams, and then rope proc macro folks in?

👍

Do you know how IntelliJ-Rust deals with this?

I think the main difference is that IntelliJ Rust has a list of hardcoded proc macros that IntelliJ Rust treats as built-in attributes (i.e. ignores them). We have actix_web::main there, so this particular issue is not an issue in intellij-rust :)

[dtolnay]

it seems that we never should asks to adjust “valid” cases, except for what be great for rustc as well (eg, using proper spans)

for invalid and “incomplete” cases, I feel that we fundamentally need help from proc macros, and here the strategy works OK

I do not agree that rust-analyzer fundamentally can't do better on syntactically invalid attribute macro input.

Rust-analyzer currently passes unmodified syntactically invalid input as macro input into attribute macros. In my opinion this violates the contract of attribute macros, which are required to be written on a valid AST node, regardless of what the macro does.

[Rustc also currently passes invalid syntax, but it's a recent regression which we plan to fix, with the approach described in the comments under rust-lang/rust#76360 (comment).]

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.