Incorrect error span in proc_macro

[Ar4ys]

rust-analyzer version: 0.3.1807-standalone

rustc version: 1.75.0 (82e1608df 2023-12-21)
rustc version (nightly): 1.77.0-nightly (88189a71e 2024-01-19)

relevant settings: none
repo with reproducible example: https://github.com/Ar4ys/rust-analyzer-span-repro

I wanted to fix error reporting from view! macro in leptos, so that the whole view! macro is not red when one property in some component is not provided. But it seems like I encountered a bug in rust-analyzer, specifically how it uses spans to report errors (i.e. it reports error over the whole view! macro, instead of over Component name only):

I think this is a bug in rust-analyzer, because cargo build spans errors correctly (i.e. reports error over Component name only, instead of over the whole view! macro):

error[E0061]: this method takes 1 argument but 0 arguments were supplied
  --> src/main.rs:14:12
   |
14 |         <  Component />
   |            ^^^^^^^^^ an argument of type `bool` is missing
   |
note: method defined here
  --> src/main.rs:9:8
   |
9  |     fn build(&self, _a: bool) {}
   |        ^^^^^        --------
help: provide the argument
   |
14 |         <  Component(/* bool */) />
   |                     ++++++++++++

Code of the view! proc macro:

use quote::{quote, quote_spanned};
use rstml::{node::Node, parse};
use syn::spanned::Spanned;

#[proc_macro]
pub fn view(tokens: proc_macro::TokenStream) -> proc_macro::TokenStream {
    let nodes = match parse(tokens) {
        Ok(value) => value,
        Err(error) => return error.into_compile_error().into(),
    };

    let Some(Node::Element(node)) = nodes.first() else {
        return proc_macro::TokenStream::new();
    };

    let name = node.name();
    let build = quote_spanned! { node.name().span() => .build() };

    quote! {
        DOM(#name)#build
    }
    .into()
}

Code that I am using for testing:

#![allow(non_snake_case)]
use test_proc_macro::view;

fn Component(_a: bool) {}

struct DOM(fn(bool));

impl DOM {
    fn build(&self, _a: bool) {}
}

fn main() {
    view! {
        <  Component />
    };

    // Recursive expansion of view! macro
    // ===================================
    DOM(Component).build();
}

[Ar4ys]

I tried replacing quote! with quote_spanned! in view! proc macro and results are interesting...

Replaced code:

// Before:
quote! {
    DOM(#name)#build
}

// After:
quote_spanned! { node.span() =>
    DOM(#name)#build
}

As a result, the whole view! invocation is not being highlighted, but instead error highlighting starts from the < token (i.e. start of the node), rather then C (i.e. start of the node's name).

That's strange.... Why rust-analyzer ignores the start of node.name().span() and uses start of "outer" span (node.span()) for error reporting?

[Ar4ys]

Forgot to share, how the error reporting is supposed to look: only the Component should be highlighted.

[Ar4ys]

I think I found the root cause of the bug: E0107 diagnostic from rust-analyzer itself. When I disable "native rust-analyzer diagnostics" ("rust-analyzer.diagnostics.enable": false in vscode) - the error from rustc is correctly reported over Component name:

When I enable "native rust-analyzer diagnostics":

• the error from rustc is still correctly reported:
  
• but the E0107 diagnostics from rust-analyzer reports over the whole view! macro, for some reason:
  

I managed to fix E0107 diagnostic by replacing adjusted_display_range with adjusted_display_range_new in invalid_args_range in mismatched_arg_count:

// crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs:39
pub(crate) fn mismatched_arg_count(
    ctx: &DiagnosticsContext<'_>,
    d: &hir::MismatchedArgCount,
) -> Diagnostic {
    let s = if d.expected == 1 { "" } else { "s" };
    let message = format!("expected {} argument{s}, found {}", d.expected, d.found);
    Diagnostic::new(
        DiagnosticCode::RustcHardError("E0107"),
        message,
        invalid_args_range_new(ctx, d.call_expr.map(Into::into), d.expected, d.found),
    )
}

fn invalid_args_range_new(
    ctx: &DiagnosticsContext<'_>,
    source: InFile<AstPtr<ast::Expr>>,
    expected: usize,
    found: usize,
) -> FileRange {
    adjusted_display_range_new(ctx, source, &|expr| {
        let (text_range, r_paren_token, expected_arg) = match expr {
            ast::Expr::CallExpr(call) => {
                let arg_list = call.arg_list()?;
                (
                    arg_list.syntax().text_range(),
                    arg_list.r_paren_token(),
                    arg_list.args().nth(expected).map(|it| it.syntax().text_range()),
                )
            }
            ast::Expr::MethodCallExpr(call) => {
                let arg_list = call.arg_list()?;
                (
                    arg_list.syntax().text_range(),
                    arg_list.r_paren_token(),
                    arg_list.args().nth(expected).map(|it| it.syntax().text_range()),
                )
            }
            _ => {
                return None;
            }
        };
        if found < expected {
            if found == 0 {
                return Some(text_range);
            }
            if let Some(r_paren) = r_paren_token {
                return Some(r_paren.text_range());
            }
        }
        if expected < found {
            if expected == 0 {
                return Some(text_range);
            }
            let zip = expected_arg.zip(r_paren_token);
            if let Some((arg, r_paren)) = zip {
                return Some(arg.cover(r_paren.text_range()));
            }
        }

        None
    })
}

Unfortunately, the function signature of adjusted_display_range_new is not exactly the same as in adjusted_display_range, so I created a duplicate of invalid_args_range that uses adjusted_display_range_new under the hood and removes "incompatible functionality" (Either::Right(pat) in match expr). Because of that I am not sure I can make PR with this fix applied -
I have no idea how to integrate adjusted_display_range_new with old functionality (Either::Right(pat) in match expr).

I would be very grateful if any maintainer of rust-analyzer picks up this fix and properly implements it :D

[Veykril]

You should be able to just split the invalid_args_range function into two, one for the expr and one for the pattern case, and then extract the common part (the if checks) out into another function.

Nice to see that the new adjusted display range function does work as wanted though, that confirms that my changes were useful for diagnostics at least :)

[Ar4ys]

Seems like debugging code while being sleep deprived is not a good idea 😅

After going though the code with a fresh mind I noticed that ast::Pat is actually an enum and ast::TupleStructPat is one of it's variants. So I just replaced Either::Right(pat) with Either::Right(ast::Pat::TupleStructPat(pat)) in invalid_args_range and called it a day :D

Here is PR with applied changes: #16431