Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: emit original token stream when there are syntax errors in #[component] or #[island] function signature (closes #2133) #2134

Merged
merged 3 commits into from
Dec 25, 2023

Conversation

gbj
Copy link
Collaborator

@gbj gbj commented Dec 25, 2023

There's a compile-time trade-off here, as this requires cloning the TokenStream another time so makes every #[component] macro slightly slower in exchange for correctly passing along errors when the function signature is broken.

…mponent]` or `#[island]` function signature (closes #2133)
@pheki
Copy link
Contributor

pheki commented Dec 25, 2023

I think that the extra clone can be removed by checking the dummy result earlier:

    let Ok(mut dummy) = syn::parse::<DummyModel>(s.clone()) else {
        return s.into();
    };
    let parse_result = syn::parse::<component::Model>(s);

    if let (ref mut unexpanded, Ok(model)) = (&mut dummy, parse_result) {
        let expanded = model.is_transparent(is_transparent).into_token_stream();
        unexpanded.sig.ident =
            unmodified_fn_name_from_fn_name(&unexpanded.sig.ident);
        quote! {
            #expanded
            #[doc(hidden)]
            #[allow(non_snake_case, dead_code, clippy::too_many_arguments)]
            #unexpanded
        }
    } else {
        dummy.sig.ident = unmodified_fn_name_from_fn_name(&dummy.sig.ident);
        quote! {
            #[doc(hidden)]
            #[allow(non_snake_case, dead_code, clippy::too_many_arguments)]
            #dummy
        }
    }
    .into()

@gbj
Copy link
Collaborator Author

gbj commented Dec 25, 2023

Oh GREAT point. Yeah this is perfect. Fixes both #2133 and (still fixes) #1781, no additional cost. Thanks!

@gbj gbj merged commit 15946c6 into main Dec 25, 2023
59 checks passed
@gbj gbj deleted the 2133 branch December 25, 2023 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants