Fix error reporting in view macro

[Ar4ys]

rust-analyzer: 2024-01-29 (v0.3.1823)

This PR is aimed to improve DX of using leptos by fixing error reporting in view! macro, so that errors inside it are reported over the token, that caused an error, instead of the whole view! macro call site.

Note

This PR is tied to bugfix in rust-analyzer (that is already merged), so "minimum expected rust-analyzer version" is v0.3.1823. Also, in order to get better rust-analyzer feedback - you need to add "rust-analyzer.cargo.allFeatures": true to your VSCode config (or similar flag for other IDEs). Should I add these two points to leptos-rs/book/getting_started/leptos_dx.md?

Here is a simple example:

Before
After

List of all errors that are covered by this PR

unreachable_code/missing_field

Disable "unreachable_code" lint when there is an error view! macro

Report "missing field" and other .build() errors on component's name

Regression: "Go to Definition" triggered on Component name in view! macro now suggests build method definition in addition to Component itself. Previously only Component was reported

Before
After

component_with_props_without_macro

Component with props but without #[component] macro is used in view! macro

Before
After

generics

When incorrect generics are passed or inferred in component the error is reported over the whole view! macro.

Regression: "Go to Definition" triggered on Component name in view! macro now suggests leptos::component_props_builder definition in addition to Component itself.

Before
After

prop_into

Error reporting with #[prop(into)]

Before
After

events

Error for "on:" events should be reported on it's name

Before
After

directive

Missing or incorrect type of the param when using directive

Before
After

block_in_fragment

If block inside fragment returns something, that is not "renderable"

Before
After

slots_prop_into

Error reporting with #[prop(into)] in slots

Before
After

wrong_slot_name

Wrong slot name

Before
After

undefined_slot

Using slot that is not defined in a component

Before
After

slot_expected_one_got_more

Expected one slot, got more

Before
After

slot_unreachable_code/slot_missing_field

Disable "unreachable_code" warn when there is an error in slot in view! macro

Report "missing field" and other .build() errors on slot's name

Regression: "Go to Definition" triggered on Then in view! macro now suggests build method definition in addition to Then itself. Previously only Then was reported

Before
After

children_clone

Using clone: notation on type that does not implement clone

Before
After

children_let_expected_one_got_none

Expected let: binding, got Children.

Before
After

children_projection_error

Before
After

children_projection_error_slot_version

Before
After

expected_children_got_let_bind

Before
After

expected_children_got_let_bind_slot_version

Before
After

expected_one_let_bind_got_more

Before
After

expected_one_let_bind_got_more_slot_version

Before
After

element_incorrect_attribute_type

Before
After

element_incorrect_spread_type

Before
After

element_incorrect_child_type

Before
After

Note

There are also some regressions regarding "Go to Definition" LSP functionality - in some cases it now suggests implementation details alongside original definition. See missing_field, generics and slot_missing_field. Although it makes DX of using "Go to Definition" worse than before - I believe it's a fair trade-off for correct error reporting.

Almost all examples listed above have a corresponding test case in leptos_macro/tests/ui/view. Some tests have subtly different output between ssr and client modes.

Example

EXPECTED:
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
error[E0277]: expected a `Fn()` closure, found `A`
 --> tests/ui/view/element/incorrect_child_type.rs:7:14
  |
6 | /     view! {
7 | |         <div>{A}</div>
  | |              ^-^
  | |              ||
  | |              |this tail expression is of type `A`
  | |              expected an `Fn()` closure, found `A`
8 | |     };
  | |_____- required by a bound introduced by this call
  |
  = help: the trait `std::ops::Fn<()>` is not implemented for `A`
  = note: wrap the `A` in a closure with no arguments: `|| { /* code */ }`
  = help: the following other types implement trait `IntoView`:
            bool
            char
            isize
            i8
            i16
            i32
            i64
            i128
          and $N others
  = note: required for `A` to implement `IntoView`
help: you might have meant to create the closure instead of a block
  |
7 |         <div>|_| {A}</div>
  |              +++
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈

ACTUAL OUTPUT:
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
error[E0277]: expected a `Fn()` closure, found `A`
 --> tests/ui/view/element/incorrect_child_type.rs:7:14
  |
6 | /     view! {
7 | |         <div>{A}</div>
  | |              ^-^
  | |              ||
  | |              |this tail expression is of type `A`
  | |              expected an `Fn()` closure, found `A`
8 | |     };
  | |_____- required by a bound introduced by this call
  |
  = help: the trait `std::ops::Fn<()>` is not implemented for `A`
  = note: wrap the `A` in a closure with no arguments: `|| { /* code */ }`
  = help: the following other types implement trait `IntoView`:
            bool
            char
            isize
            i8
            i16
            i32
            i64
            i128
          and $N others
  = note: required for `A` to implement `IntoView`
note: required by a bound in `HtmlElement::<El>::child`
 --> $WORKSPACE/leptos_dom/src/html.rs
  |
  |     pub fn child(self, child: impl IntoView) -> Self {
  |                                    ^^^^^^^^ required by this bound in `HtmlElement::<El>::child`
help: you might have meant to create the closure instead of a block
  |
7 |         <div>|_| {A}</div>
  |              +++
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈

List of all problematic tests

use leptos::*;

struct A;

fn incorrect_attribute_type() {
    view! {
        <div id=A />
    };
}

fn incorrect_child_type() {
    view! {
        <div>{A}</div>
    };
}

fn incorrect_class_type() {
    view! {
        <div class:id=A />
    };
}

fn incorrect_fancy_class_type() {
    view! {
        <div class=("id", A) />
    };
}

fn incorrect_global_class_type() {
    view! { class=A,
        <div />
    };
}

fn incorrect_style_type() {
    view! {
        <div style:id=A />
    };
}

fn incorrect_fancy_style_type() {
    view! {
        <div style=("id", A) />
    };
}


fn incorrect_event_type() {
    view! {
        <div on:click=A />
    };
}

// `prop:` are not currently checked in ssr at all
fn incorrect_prop_type() {
    view! {
        <div prop:id=A />
    };
}

fn incorrect_spread_type() {
    view! {
        <div {..A} />
    };
}

Unfortunately, there is no easy way to implement these tests, because maintainer of trybuild crate refuses to allow specifying custom suffix on TestCases: dtolnay/trybuild#245. Currently, I see 4 possible ways about this:

• Duplicate these tests in separate folders (view_ssr and view_client) and load them conditionally (based on current feature flags)
• Fork trybuild crate and add suffix functionality
• "Add code in the test to rename the right set of files to .stderr before calling into trybuild" - suggestion from maintainer of trybuild
• Just ignore these tests and hope DX will not degrade in the future

[Ar4ys]

Looks like after adding a bunch of spans clippy now more aggressively checks output of view! macro. I fixed ones, that broke the tests/examples, but I think this does not cover all possible clippy lints...

[gbj]

Simply going on the basis of the screenshots: Wow. I have not looked through it in more depth yet, but just wanted to express my gratitude for your work on this, which will be a huge help to the whole community.

[Ar4ys]

@gbj Thank you for such kind words :3. That's actually just a starting point - I plan on making at least 2 more PRs: better support for LSP features inside view! macro (like "Go to Definition" for attributes, better support for punctuated attributes, etc.) and better syntax highlighting (I have an idea how to highlight closing tag for components without duplicating the whole node tree :D).

On the side node: I am a bit of a dumby and forgot to run cargo make test in leptos_macro - already pushed a commit with fixes

[gbj]

Thanks! I've just looked through and this looks great over all. However, it looks like there are still some failing tests in leptos_macro, including at least some of the tests you're adding in this PR (I think). Once those are passing, I think this should be good to go.

[Ar4ys]

As per this discussion on discord I decided to remove snapshot tests for view! macro. Quick rundown for anyone without access to discord:

Errors reported by rustc seem to fluctuate A LOT between versions (specifically nightly ones). For example:

• I had locally version rustc 1.77.0-nightly (88189a71e 2024-01-19) - cd leptos_macro && cargo make test reports no failing snapshot tests
• On github ci there is rustc 1.77.0-nightly (635124704 2024-01-27) - it reports snapshot mismatches in missing_field.rs, slot_missing_field.rs and slot_unreachable_code.rs
• I run rustup update and it installed rustc 1.78.0-nightly (0ecbd0605 2024-02-25) - it now reports snapshot mismatches in block_in_fragment.rs, directives.rs, generics.rs, prop_into.rs, slots_prop_into.rs and wrong_slot_name.rs, BUT NOT in the tests, that ci reports failures in.

If this is a common trend in rustc (errors reported change each new nightly release) - I am not sure that snapshot tests for view! macro is worth the effort, as it seems to me that every week or two we will need to update all snapshots because there is a minor update to errors. Looks like a lot of unnecessary burden on leptos maintainers.

[Ar4ys]

What is the state/progress of this PR? Is there anything that should be done from my side?

[gbj]

My apologies! The status of the PR is that I could've sworn it was merged long ago because it was excellent, when in reality I lost track of it during a busy month and have done nothing.

I just merged the one outstanding conflict and will let the CI run now and merge. Thanks so much for the contribution and sorry about the delay.