Fix incorrect diagnostics for failing built in macros

[brandondong]

Reproduction:

1. Use a built in macro in such a way that rust-analyzer fails to expand it. For example:

lib.rs

include!("<valid file but without a .rs extension so it is not indexed by rust-analyzer>");

2. rust-analyzer highlights the macro call and says the macro itself cannot be resolved even though include! is in the standard library (unresolved-macro-call diagnostic).
3. No macro-error diagnostic is raised.

Root cause for incorrect unresolved-macro-call diagnostic:

1. collector:collect_macro_call is able to resolve include! in legacy scope but the expansion fails. Therefore, it's pushed into unexpanded_macros to be retried with module scope.
2. include! fails at the resolution step in collector:resolve_macros now that it's using module scope. Therefore, it's retained in unexpanded_macros.
3. Finally, collector:finish tries resolving the remaining unexpanded macros but only with module scope. include! again fails at the resolution step so a diagnostic is created.

Root cause for missing macro-error diagnostic:

1. In collector:resolve_macros, directive.legacy is None since eager expansion failed in collector:collect_macro_call. The macro_call_as_call_id fails to resolve since we're retrying in module scope. Therefore, collect_macro_expansion is not called for the macro and no macro-error diagnostic is generated.

Fix:

• In collector:collect_macro_call, do not add failing built-in macros to the unexpanded_macros list and immediately raise the macro-error diagnostic. This is in contrast to lazy macros which are resolved in collector::resolve_macros and later expanded in collect_macro_expansion where a macro-error diagnostic may be raised.

[jonas-schievink]

Thanks, this looks good! Can you add a test to crates/hir_def/src/nameres/tests/diagnostics.rs? Something like this should work:

    check_diagnostics(
        r#"
        #[rustc_builtin_macro]
        macro_rules! include { () => {} }

        include!("doesntexist");
      //^^^^^^^^^^^^^^^^^^^^^^^ <text of diagnostic>
        "#,
    );

[brandondong]

Just to clarify, the behavior before my changes is that an incorrect unresolved-macro-call diagnostic would be raised in this scenario. No macro-error diagnostic on the other hand would be raised.

This is because in collector:resolve_macros, directive.legacy is None since eager expansion failed in collector:collect_macro_call. The macro_call_as_call_id fails to resolve since we're retrying in module scope. Therefore, collect_macro_expansion is not called for the macro and no macro-error diagnostic is generated. I believe that even if the code did make it there successfully, it would stop in collector:collect_macro_expansion because of the FIXME to handle eager macros.

My change only removes the incorrect unresolved-macro-call diagnostic. I can create a test to assert no diagnostics if that is desired.

I am assuming the optimal behavior of the system would be something like:

• macro-error is true if any of legacy or module resolutions (or both) succeeded but no expansions succeeded.
• unresolved-macro-call is true if both legacy and module resolutions failed (what this change implements).

I could try implementing the first bullet point as well but it appears to be a much more involved change.

[jonas-schievink]

Hmm, I see. I'm not entirely sure what the best fix here is – it's a bit complicated with eager expansion happening effectively during macro resolution.

[brandondong]

I took a look at the missing macro-error diagnostic issue. I believe I have a better understanding of the code now and redid the fix. If you could take another look, that would be greatly appreciated.

[jonas-schievink]

Looks good, thanks! Left one minor nit.

[jonas-schievink]

bors r+

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript (ubuntu-latest)
• TypeScript (windows-latest)

[lnicola]

changelog fix