BUG: candid_method macro messes up intellisense and stops it from working

[saikatdas0790]

Applying the #[candid::candid_method] macro for autogeneration of Candid in a Rust project breaks intellisense on that function. Here's a minimal reproduction:
https://github.com/saikatdas0790/ic_cdk_macro_intellisense_test

Steps to reproduce:

• Clone repo and install dependencies
• Open the project in a VS Code instance with the rust-analyzer extension installed
• Try prompting intellisense in the increment_1 and increment_2 functions
• Intellisense doesn't work in the first one with both macros but works in the second one

Here are some screenshots:

Intellisense shows generic word results here

Intellisense shows relevant results received from the Rust language server

Additional environment details:

• Running on Windows 11 with WSL 2
• VS Code stable - 1.74.2
• rustc version - rustc 1.66.0 (69f9c33d7 2022-12-12)
• rust-analyzer version - 0.3.1361-standalone

Additional context:
I reported this bug to the author of the candid library here but they confirmed that it seems to be a problem with rust-analyzer. However, if you need additional details/context, I can follow up with them to provide more details

Thank you for building this awesome extension 😃

[lnicola]

We don't expand the macro correctly.

cargo-expand:

mod api {
    use crate::CANISTER_DATA;
    #[export_name = "canister_update increment_1"]
    fn increment_1_0_() {
        ic_cdk::setup();
        ic_cdk::spawn(async {
            let () = ic_cdk::api::call::arg_data();
            let result = increment_1();
            ic_cdk::api::call::reply((result,))
        });
    }
    fn increment_1() -> u64 {
        CANISTER_DATA
            .with(|data| {
                let mut data = data.borrow_mut();
                data.counter += 1;
                data.counter
            })
    }
}

rust-analyzer:

#[export_name = "canister_update increment_1"]
fn increment_1_40_() {
    ic_cdk::setup();
    ic_cdk::spawn(async {
        let () = ic_cdk::api::call::arg_data();
        let result = increment_1();
        ic_cdk::api::call::reply((result,))
    });
}
compile_error! {
  "duplicate method name increment_1"
}

I suspect we handle the macro's state differently from rustc, see https://github.com/dfinity/candid/blob/master/rust/candid_derive/src/func.rs#L15-L23. As an IDE we need to keep the macro loaded and ask it multiple times to expand macro calls, the compiler doesn't have to do this.

[Veykril]

This most likely occurs due to our proc-macro caching, so not really something we can do about I think.

[lnicola]

We expand the macro on every typed character, but the macro stays loaded, so on each expansion it sees a new definition of the function, which causes it to fail with that error. I wonder how well the other crates making use of the same trick work.

[flodiebold]

Also, this is very bad for incrementality (using a global counter to generate identifiers).

[flodiebold]

IMO this is a bug in the proc macro. I really think we need some official definition of what proc macros are allowed to do, and impure behavior based on global state should not be part of it.

[saikatdas0790]

Thank you for the diagnosis. I've updated the original library author about the discussion here and waiting for their inputs. At this stage, is there a resolution that you would suggest?

I understand that the library has impure behaviour that leads to intellisense breaking, but ideally, the extension should also handle such scenarios so as to gracefully fail and not turn off intellisense completely, right?

[lnicola]

We fail gracefully, i.e. don't crash. Do you mean we should ignore the attribute if it fails to expand? That won't work in general, because the macro could take anything as in input, and because our expansion won't be correct anyway, see #11014 (comment). But you can get that effect by setting rust-analyzer.procMacro.ignored.

In addition, there's some non-trivial caching involved, and we can't tell why the macro failed. Maybe it wasn't able to parse something? Maybe it's crashing due to some questionable use of local state? Maybe it crashed because a database server wasn't accessible? Will it crash the same way if executed again?

[flodiebold]

Also, the macro doesn't fail. It successfully expands to an expansion with a compile_error and which doesn't contain the original code. That means we just don't have any semantic information for the original code, because it doesn't exist in the macro-expanded code. If the macro expanded to something that still contains the original function body, we would be able to provide completions inside it.

[flodiebold]

Anyway, let's close this issue.