yew icon indicating copy to clipboard operation
yew copied to clipboard

rust-analyzer cannot jump to the definition of components or attributes used inside of html!() macros

Open udoprog opened this issue 4 months ago • 9 comments

Problem When hovering and ctrl-clicking a component inside of an html!() macro it doesn't pull up the definition of the component, instead it gives a laundry list of uses which seemingly includes many of the items used inside of the macro (see among other things references to core and std):

Image

Steps To Reproduce Steps to reproduce the behavior:

  1. Take any yew project.
  2. ctrl-click on any component inside of an html!() macro.

Expected behavior You should jump to the component.

Environment:

  • Yew version: master
  • Rust version: 1.89.0

Cause

The root cause of this is the spans being produced by the macro. Yew makes heavy used of respanning methods such as quote_spanned! and parse_quote_spanned!. Judging by the test cases (and running some of them) this is to improve ergonomics in error reporting.

Unfortunately this has the effect of the relevant identifier being hovered over having the span of presumably a much wider expression than the scope of the identifier, which confuses rust-analyzer.

I have a branch where I removed all respanning methods, and there I can jump to the definition of components, feel free to try it out: https://github.com/udoprog/yew/tree/remove-spans.

This is what I get with my branch (it cleanly jumps to the definition when clicked):

Image

In my branch, jumping to attributes almost works as well. It currently uses two definitions which seems to confuse rust-analyzer:

Image

In my mind, this is an important use case than improving error diagnostics, jumping to definition is something I attempt all the time, which is what caused me to investigate the issue and write this bug report.

Thank you!

udoprog avatar Aug 19 '25 21:08 udoprog

thanks, this is worth investigating indeed.

I use neovim and get a bunch of garbage suggestions when I try to jump to definition with the LSP too. At this point, I have developed muscle memory to always jump to definition only after going to the previous occurences first (usually the use statement or the actual definition) .

I'm not immediatly sure how helpful the respanning methods are in improving diagnostics.

@udoprog do you wanna work on this and submit a PR?

Madoshakalaka avatar Aug 20 '25 12:08 Madoshakalaka

I can take a look, but it will take a while for me to find the spare cycles so if anyone else is interested I'd encourage them to do it.

There's immediately one instance which pops out that should be addressed when respanning is removed. When there are missing properties or generally bounds are not met, diagnostics points out the entire macro call rather than the component which is the cause:

+  --> tests/function_component_attr/generic-props-fail.rs:27:5
    |
 27 |     html! { <Comp<MissingTypeBounds> /> };
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `yew::Properties` is not implemented for `MissingTypeBounds`

You can see the full diff here: https://gist.github.com/udoprog/7caaae66cbd0f22f12bf4997efa87fb8

This causes a bit of an issue, because in order to report the bounds on the component it has to use the span of the identifier which we also want rust-analyzer to use when looking up definitions. This span will then tell rust-analyzer that it references the block of code which causes the bound to fail rather than something which just uses the identifier of the component, probably confusing it.

One way to improve this might be to respan the code which causes the bounds error to the empty span just prefixing the identifier:

-  --> tests/function_component_attr/generic-props-fail.rs:27:14
+  --> tests/function_component_attr/generic-props-fail.rs:27:5
    |
 27 |     html! { <Comp<MissingTypeBounds> /> };
+   |                   ^ the trait `yew::Properties` is not implemented for `MissingTypeBounds`

Without testing it though, I'm not entirely sure that's how rustc will end up formatting it. But at least it should avoid the look up definition confusion.

udoprog avatar Aug 20 '25 13:08 udoprog

Thanks for the much helpful report.

I'll work on it when I'm free.

Madoshakalaka avatar Aug 21 '25 17:08 Madoshakalaka

Having used my branch for a bit now, another benefit is that type and variable refactoring (incl. property names) with rust-analyzer works as expected.

udoprog avatar Aug 25 '25 14:08 udoprog

@its-the-shrimp I wonder what you think, since you have work on html macros extensively. Looking at udoprog's diagnostics diff, there is a tradeoff I think. We do get a tad more verbose/imprecise diagnostics without the respanning.

Madoshakalaka avatar Aug 26 '25 04:08 Madoshakalaka

I'm confused by how more precise spans make for less precise error spans

its-the-shrimp avatar Aug 26 '25 11:08 its-the-shrimp

These, for example, seem unforgivable.

These should be diagnostics for specific components and the span shouldn't be the whole html! macro

 error[E0277]: not all required properties have been provided
-  --> tests/html_macro/component-fail.rs:99:14
+  --> tests/html_macro/component-fail.rs:99:5
    |
 99 |     html! { <Child string="abc" /> };
-   |              ^^^^^ missing required properties for this component
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing required properties for this component
    |
    = help: the trait `HasProp<int, _>` is not implemented for `AssertAllProps`, which is required by `AssertAllProps: AllPropsFor<ChildPropertiesBuilder, _>`
 note: required for `CheckChildPropertiesAll<AssertAllProps>` to implement `HasAllProps<ChildProperties, (_,)>`
@@ -595,37 +596,41 @@ note: required by a bound in `yew::html::component::properties::__macro::PreBuil
    = note: this error originates in the macro `html` which comes from the expansion of the derive macro `Properties` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error[E0609]: no field `children` on type `ChildProperties`
-   --> tests/html_macro/component-fail.rs:103:14
+   --> tests/html_macro/component-fail.rs:103:5
     |
 103 |     html! { <Child>{ "Not allowed" }</Child> };
-    |              ^^^^^ unknown field
+    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unknown field
     |
     = note: this error originates in the macro `html` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error[E0599]: no method named `children` found for struct `ChildPropertiesBuilder` in the current scope
-   --> tests/html_macro/component-fail.rs:103:14
+   --> tests/html_macro/component-fail.rs:103:5
     |
 4   | #[derive(Clone, Properties, PartialEq)]
     |                 ---------- method `children` not found for this struct
 ...
 103 |     html! { <Child>{ "Not allowed" }</Child> };
-    |              ^^^^^ method not found in `ChildPropertiesBuilder`
+    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `ChildPropertiesBuilder`

Madoshakalaka avatar Aug 26 '25 12:08 Madoshakalaka

Hi guys, was just about to open this issue because I found a fix.

I don't know what @udoprog did on the branch. But my proposed fix has only 2 changes in yew-macro/src/hook/body.rs:

line 54:

                        // Use Span::call_site() instead of i.span() to fix rust-analyzer nav
                        *i = parse_quote_spanned! { proc_macro2::Span::call_site() => ::yew::functional::Hook::run(#i, #ctx_ident) };

line ~80:

                            // Use Span::call_site() instead of i.span() to fix rust-analyzer nav
                            *i = parse_quote_spanned! { proc_macro2::Span::call_site() => ::yew::functional::Hook::run(#i, #ctx_ident) };

That's it. Will submit PR for easy diff'ing. Would like help verifying. Fixes it for me.

P.S. This seems to be a terrible bug in rust-analyzer/rust internals, because the bug goes away if you copy the crates.io or github cache of this repo and use a local path patch override. So that tells me that the code i.span() should work, but because of some internal analyzer manging, it breaks. Weird.

tgrushka avatar Aug 31 '25 01:08 tgrushka

P.S. This seems to be a terrible bug in rust-analyzer/rust internals, because the bug goes away if you copy the crates.io or github cache of this repo and use a local path patch override. So that tells me that the code i.span() should work, but because of some internal analyzer manging, it breaks. Weird.

When messing around with rust-analyzer there's a command to rebuild macros you probably need to use when testing things. In vscode you do Ctrl+Shift+P and rust-analyzer: Rebuild proc-macros and build scripts. I think Rebuild build dependencies in the context menu (buttom of vscode) is the same thing.

udoprog avatar Aug 31 '25 02:08 udoprog