Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rocket should handle attribute macro errors more gracefully. #1967

Closed
twitchax opened this issue Oct 27, 2021 · 7 comments
Closed

Rocket should handle attribute macro errors more gracefully. #1967

twitchax opened this issue Oct 27, 2021 · 7 comments
Labels
triage A bug report being investigated

Comments

@twitchax
Copy link

Description

The Rocket attribute macros may not handle errors gracefully, which results in rust-analyzer failing to work inside of, e.g., http handler methods marked with attributes (e.g., get or post). See rust-lang/rust-analyzer#10652 for screenshots of rust-analyzer code completion suggestions in http handlers.

FWIW, other crates have similar issues. See rust-lang/rust-analyzer#10498.

To Reproduce

With rust-analyzer enabled, attempt a code completion in a Rocket http handler (e.g., get). For example, the following code would fail to provide suggestions (pipe used as cursor).

#[get("/topics?<key>")]
async fn get_topics(key: String, span: RequestSpan<'_>, state: &State<RocketState>) -> JsonResult<Vec<GetTopicResponse>> {
    info!("Processing rocket GET request for `topics` ...");

    key.|
}

Generally, key. would provide String method completion suggestions. When inside of this method, there are no suggestions due to the macro expansion. It seems like this happens because the macro re-emits the method, and doesn't handle the syntax errors well.

Expected Behavior

The macro expansion would handle errors gracefully and not break rust-analyzer code completion.

Environment:

  • OS Distribution and Kernel: Windows 11, MacOS 12
  • Rocket Version: 0.5.0-rc.1
@twitchax twitchax added the triage A bug report being investigated label Oct 27, 2021
@Follpvosten
Copy link
Contributor

I would like to add that in my opinion, this is kind of a necessity for a second RC or a full release of 0.5. Releasing with a broken IDE experience would certainly not be good (though I have to agree with the point async-trait made that this should probably not be the responsibility of the proc macro crates, but that's where we are right now).

@SergioBenitez
Copy link
Member

SergioBenitez commented Mar 8, 2022

To be clear:

  1. This should only be an issue on stable. We recommend using nightly during development.
  2. What's needed to "fix" this is to re-emit the entire broken token stream instead of emitting only a compile_error!().

I agree with @dtolnay that this is an issue that should be handled by rust-analyzer: when a tool requires every library to change itself to work with the tool, it might be a hint that the tool is doing the wrong thing. In any case, if it's an easy fix in Rocket, which it likely should be since all errors should go through one path, we should do it.

@twitchax
Copy link
Author

twitchax commented Mar 9, 2022

Hello, @SergioBenitez!

Great to hear from you, and many thanks for maintaining Rocket. :)

As for (1), I pretty much exclusively use nightly, and it was still a problem, but I have been using the "don't expand proc macros" feature of r-a for a while, which effectively unblocks this issue in 99% of cases.

I think most of the community has tended to agree with @dtolnay here, which is why r-a added a workaround. Not sure a lot of effort should be spent here, as r-a is actively looking for more reasonable resolution paths, as far as I can tell.

@SergioBenitez
Copy link
Member

which is why r-a added a workaround

Is this the "don't expand proc-macros" setting?

@SergioBenitez
Copy link
Member

As for (1), I pretty much exclusively use nightly, and it was still a problem

Note that you need to ask rust-analyzer to use nightly, too. I say that this shouldn't be an issue on nightly because we don't emit a compile_error!() invocation on nightly but instead a blank token stream and emit the diagnostic directly using the nightly-only APIs. Perhaps this also raises an issue for rust-analyzer.

@twitchax
Copy link
Author

Is this the "don't expand proc-macros" setting?

Yep! Or rather, you can specify a list of macros for r-a to ignore. For example, mine looks like this right now.

"rust-analyzer.procMacro.ignored": {
    "rocket_okapi_codegen": [
        "openapi"
    ],
    "async-trait": [
        "async_trait"
    ],
    "rocket_codegen": [
        "get", 
        "post",
        "put",
        "delete",
        "patch",
        "head",
        "options",
    ],
},

Note that you need to ask rust-analyzer to use nightly, too. I say that this shouldn't be an issue on nightly because we don't emit a compile_error!() invocation on nightly but instead a blank token stream and emit the diagnostic directly using the nightly-only APIs. Perhaps this also raises an issue for rust-analyzer.

Got it, this might be why I didn't ever see failures, but I saw just a "lack" of any local code completion capabilities. Since there was no compile_error!(), no failures were reported, but since there was no token stream, there were also no locals.

Personally, I am happy with the procMacro.ignored workaround, and I wouldn't really consider this high priority at this point.

@SergioBenitez
Copy link
Member

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage A bug report being investigated
Projects
None yet
Development

No branches or pull requests

3 participants