dtolnay / syn

Parser for Rust source code
Apache License 2.0
2.88k stars 311 forks source link

`Generics` does not tokenize its `WhereClause` #782

Open Aaron1011 opened 4 years ago

Aaron1011 commented 4 years ago

The ToTokens impl for Generics does not tokenize the contained WhereClause - instead, any items which contain a Generics (e.g. ItemStruct) explicitly tokenize the WhereClause in the correct position.

This is a major footgun - if a Generics is extracted from a parsed item and then quote!ed somewhere else, the WhereClause will silently be lost. This can lead to confusing errors, since the lack of a where clause can cause type-checking to fail.

Making Generics include its WhereClause during tokenization would be a breaking change (and there may be drawbacks that I haven't thought of). However, the documentation should clearly state that tokenizing a Generics is lossy, to help prevent confusing errors in downstream crates.

TonalidadeHidrica commented 2 years ago

I definitely think this should be documented. I have just shot my foot, wondering why the constraint is not satisfied, and it was not until I read the code of impl ToToken for Generics that I realized that where clause is actually not turned into tokens, attempting to file a new issue to reach here.

LikeLakers2 commented 2 years ago

I just came across this issue, because my procedural macro was panicking for some unknown reason.

Here's the code I was using:

/// Applies the specified bounds to all generic parameters
pub fn apply_bounds(
    generics: Generics,
    _bound_to_add: TypeParamBound,
) -> Generics {
    let (params, where_clause) = (
        generics.params,
        generics.where_clause,
    );

    // (shortened -- this isn't what panics)
    let edited_params = 
        params.into_iter() // ...
        ;

    // The invocation below panics due to `#where_clause` being an "unexpected token".
    parse_quote! {
        <#(#edited_params),*> #where_clause
    }
}

I was able to fix it by changing that parse_quote! invocation to this:

let mut new_generics: Generics = parse_quote! {
    < #(#edited_params),* >
};
new_generics.where_clause = where_clause;

If I might suggest something, it would be to move the WhereClause out of Generics, rather than document this anomaly. I understand this would be a breaking change, however, but having it in structs where it would actually be tokenized would be quite a lot less confusing (especially for those trying to make a Generics via parse_quote!).

If nothing else, the parse_quote! macro could at least have a special case for this, where its panic message will specify to set generics.where_clause separately.

Imberflur commented 1 year ago

I just encountered a bug in my macro code where I assumed the WhereClause would be included and got errors that were difficult to track back to this. It's a bit surprising that Generics implements ToTokens but everything isn't included in the output.