async-trait is not IDE friendly with the latest rust-analyzer version

[asdrubalinea]

A month ago, rust-analyzer enabled by default a feature called experimental.procAttrMacros. This aims to provide better support for procedural macros, however it may cause issues with some crates that are not error resilient in this sense, making entire impl blocks unable to have auto-completion unless experimental.procAttrMacros is explicitly disabled globally. At the moment two major crates (tokio and actix-web) had this problem fixed in master.

Here is an example of what it looks like in practice: https://youtu.be/vhrOrfrNYlw

The correct fix, as suggested by rust-analyzer's folks (see rust-lang/rust-analyzer#10498, rust-lang/rust-analyzer#10611 and rust-lang/rust-analyzer#10468 for more details) would be for the crate to also report the original input as well, so that IDE can correctly recover.

Here are the patches that fixed the issue:

• make runtime macros more IDE friendly actix/actix-net#391
• attributes: speculatively expand when parsing fails tokio-rs/tracing#1634
• Make tokio-macros attributes more IDE friendly tokio-rs/tokio#4162

I tried to fix the issue in my own branch (https://github.com/asdrubalini/async-trait/tree/be-friendly-to-ides) without success, as it seems that both args and input parsing always return Ok. I am going to debug again tomorrow, but any suggestion is appreciated.

[dtolnay]

I think this needs to be fixed in rust-analyzer. I don't think attribute macros are expected to accommodate syntactically invalid input — rustc enforces valid syntax (or performs adjustments to the input syntax to make it valid when it knows what you mean) prior to expansion.

[asdrubalinea]

I think this needs to be fixed in rust-analyzer. I don't think attribute macros are expected to accommodate syntactically invalid input — rustc enforces valid syntax (or performs adjustments to the input syntax to make it valid when it knows what you mean) prior to expansion.

Well... this is an awkward situation, but rust-analyzer doesn't recognize this as a bug in their system and instead expects crates authors to better report syntax errors, so here's that. The issue itself is very cryptic to identify, it took me a few days to find the origin... maybe we can discuss it with rust-analyzer's devs? As far as I know there was an issue on RA's repository to fallback in case of a compile error but was rejected...

[dtolnay]

I feel it's fundamentally inappropriate to push making sense of invalid syntax into the macros rather than the IDE. Macros operate on at most an instantaneous snapshot of the invalid code, whereas the IDE has way more situational awareness of where the cursor is, what part of the input was typed or pasted most recently, etc.

My expectation would be that rust-analyzer snips out the nearest syntactically invalid syntax tree node and swaps in whatever syntactically valid placeholder/sentinel it wants in its place, then runs the macro which will expand successfully, then provides 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.

I don't have bandwidth at the moment to drive this discussion in rust-analyzer but hopefully the folks interested in using async-trait with rust-analyzer can follow up with the devs.

[flodiebold]

Hi @dtolnay,

I know you wrote you don't have the bandwidth to drive this discussion, and I don't want to pull you into a discussion (though we could really use your support), but I did want to address some of your points.

rustc enforces valid syntax (or performs adjustments to the input syntax to make it valid when it knows what you mean) prior to expansion

I don't think that's true, as you can actually see when you try to compile an attribute macro invocation with invalid syntax:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d0847ef468599fff6fe3857bcc4f5dbc
There are two syntax errors there, the first one coming from rustc, the second one from the macro. If you remove the macro invocation, only the first error remains.

Macros operate on at most an instantaneous snapshot of the invalid code, whereas the IDE has way more situational awareness of where the cursor is, what part of the input was typed or pasted most recently, etc

None of those things are used, or even (IMO) particularly useful for handling invalid input. In fact, all analysis in rust-analyzer also operates on a snapshot of the invalid code. On the other hand, only the macro knows what it should expand to. Handling invalid syntax in many cases amounts to just not unnecessarily parsing function bodies and instead leaving them as token streams, i.e. doing less. That would also make those macros more resilient to future changes in the Rust syntax, by the way, and maybe improve build times by not having to include a full parser for Rust expression syntax in each proc macro. (Also it would fix the duplicate syntax error you see in the playground above.)

That said, I think your proposal is interesting and could potentially work in almost all cases. The problem is that it still won't be fully correct; imagine for example a macro that replaces all variable names in the function body by something else -- grafting expressions from the input directly into the output would result in a wrong analysis in that case. Or, maybe more realistically, a macro that examines the tail expression of the function and expects it to have some specific form. It would be impossible for rust-analyzer to guess what placeholder it needs to use to make such a macro expand correctly, and implementing such a workaround would make it impossible for the macro to handle incomplete input itself.

[dtolnay]

I don't think that's true, as you can actually see when you try to compile an attribute macro invocation with invalid syntax

It's formerly true, at least. Looks like in 1.50.0 and older it passes syntactically valid macro input with the bad syntax snipped out, as trait Advertisement { fn run(); }. I would need to check with the compiler team whether the newfangled behavior in rustc is intended or a regression from recent Span-related work.

None of those things are used, or even (IMO) particularly useful for handling invalid input. In fact, all analysis in rust-analyzer also operates on a snapshot of the invalid code.

An example of where I would have expected information beyond an instantaneous snapshot to be useful is:

struct S {
    a: u8,
}

fn f() -> impl Sync {
    let union = 0; let u8 = 0; let x = union S { a: u8 }
}

If the user has just typed let x =, it's worth type checking this function as returning (). If the user has just typed let x = union, it's worth type checking this function as returning S { a: 0 }. If there were an attribute macro on this function, your own example of "maybe more realistically, a macro that examines the tail expression of the function" is going to be unable to make sense of this instantaneous snapshot of invalid input unless rust-analyzer helps out in the way that I described.

[flodiebold]

If the user has just typed let x =, it's worth type checking this function as returning (). If the user has just typed let x = union, it's worth type checking this function as returning S { a: 0 }.

That would result in the analysis changing when the user closes and re-opens the file, or maybe even when they move the cursor, so something like that isn't really an option anyway.

[dtolnay]

I would need to check with the compiler team whether the newfangled behavior in rustc is intended or a regression from recent Span-related work.

The rustc 1.51+ behavior change of passing illegal syntax to macros was not intended; we're following up in rust-lang/rust#90256 + rust-lang/rust#76360 (comment) to pass recovered valid syntax to attribute macros.