proc_macro: Use ToTokens trait in quote macro

[SpriteOvO]

Tracking issues: #130977, #54722

This PR changed proc_macro::quote! to use ToTokens trait instead of TokenStream::from, and migrated test cases from quote crate.

r? @dtolnay
CC @tgross35

[tgross35]

Thanks for working on this!

Could you update the new test file with applicable tests from https://github.com/dtolnay/quote/blob/aafba72e10919ad47c05169271cb78e614fb2b9d/tests/test.rs? Because of token stream limitations we can't actually mark them #[test] or add the assert! (eventually we will, this should be a FIXME), but keeping the quote! invocations at least allows us to verify that the expected things do quote.

We can probably reuse the UI tests from https://github.com/dtolnay/quote/tree/aafba72e10919ad47c05169271cb78e614fb2b9d/tests/ui too. I think you can just make a new tests/ui/proc-macro/quote directory.

[SpriteOvO]

I have updated the PR for the macro implementation, and I will migrate the test cases from quote crate later.

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[SpriteOvO]

I'm working on migrating the applicable tests from https://github.com/dtolnay/quote/blob/0245506323a3616daa2ee41c6ad0b871e4d78ae4/tests/test.rs. It seems like the current proc_macro::quote! is still incomplete? Some features (quote_spanned!, format_ident!, repetition) are still not implemented, because of that many test cases cannot be migrated.

// - quote_spanned:
//   - fn test_quote_spanned_impl
//   - fn test_type_inference_for_span
// - format_ident:
//   - fn test_format_ident
//   - fn test_format_ident_strip_raw
// - repetition:
//   - fn test_iter
//   - fn test_array
//   - fn test_fancy_repetition
//   - fn test_nested_fancy_repetition
//   - fn test_duplicate_name_repetition
//   - fn test_duplicate_name_repetition_no_copy
//   - fn test_btreeset_repetition
//   - fn test_variable_name_conflict
//   - fn test_nonrep_in_repetition
//   - fn test_closure
//   - fn test_star_after_repetition

And many tests cases failed with proc_macro::quote!, e.g.

/*
message: assertion `left == right` failed
             left: "impl < 'a , T : ToTokens > ToTokens for & 'a T { fn to_tokens (& self , tokens : & mut TokenStream) { (* * self) . to_tokens (tokens) } }"
            right: "impl < 'a, T : ToTokens > ToTokens for & 'a T\n{\n    fn to_tokens(& self, tokens : & mut TokenStream)\n    { (** self).to_tokens(tokens) }\n}"
*/
fn test_quote_impl() {
    let tokens = quote! {
        impl<'a, T: ToTokens> ToTokens for &'a T {
            fn to_tokens(&self, tokens: &mut TokenStream) {
                (**self).to_tokens(tokens)
            }
        }
    };

    let expected = concat!(
        "impl < 'a , T : ToTokens > ToTokens for & 'a T { ",
        "fn to_tokens (& self , tokens : & mut TokenStream) { ",
        "(* * self) . to_tokens (tokens) ",
        "} ",
        "}"
    );

    assert_eq!(expected, tokens.to_string());
}

/*
message: assertion `left == right` failed
             left: "X < X > (X) [X] { X }"
            right: "X <X > (X) [X] { X }"
*/
fn test_substitution() {
    let x = X;
    let tokens = quote!($x <$x> ($x) [$x] {$x});

    let expected = "X < X > (X) [X] { X }";

    assert_eq!(expected, tokens.to_string());
}

/*
message: assertion `left == right` failed
             left: "struct SerializeWith < 'a , T > where T : Serialize { value : & 'a String , phantom : :: std :: marker :: PhantomData < Cow < 'a , str > > , } impl < 'a , T > :: serde :: Serialize for SerializeWith < 'a , T > where T : Serialize { fn serialize < S > (& self , s : & mut S) -> Result < () , S :: Error > where S : :: serde :: Serializer { SomeTrait :: serialize_with (self . value , s) } } SerializeWith { value : self . x , phantom : :: std :: marker :: PhantomData :: < Cow < 'a , str > > , }"
            right: "struct SerializeWith < 'a, T > where T : Serialize\n{\n    value : & 'a String, phantom : :: std :: marker :: PhantomData <Cow < 'a,\n    str > >,\n} impl < 'a, T > :: serde :: Serialize for SerializeWith < 'a, T > where T :\nSerialize\n{\n    fn serialize < S > (& self, s : & mut S) -> Result < (), S :: Error >\n    where S : :: serde :: Serializer\n    { SomeTrait :: serialize_with(self.value, s) }\n} SerializeWith\n{\n    value : self.x, phantom : :: std :: marker :: PhantomData ::<Cow < 'a, str\n    > >,\n}"
*/
fn test_advanced() { /* ... */ }

and more.

I propose to open a new PR to add these test cases with fixes, and leave a FIXME for these unimplemented features, to keep this PR clean. What do you think? @tgross35

[tgross35]

I'm working on migrating the applicable tests from https://github.com/dtolnay/quote/blob/0245506323a3616daa2ee41c6ad0b871e4d78ae4/tests/test.rs. It seems like the current proc_macro::quote! is still incomplete? Some features (quote_spanned!, format_ident!, repetition) are still not implemented, because of that many test cases cannot be migrated.

At least at this time, there is no plan for quote_spanned or format_ident so just ignore any tests out that rely on these. We definitely want repetition at some point, so keeping a list of missing tests for repetition in a FIXME comment sounds reasonable.

And many tests cases failed with proc_macro::quote!, e.g.

The examples you listed look like they have the same meaning but some different spacing. Updating the whitespace to match should be fine, as long as there aren't other inconsistencies.

[SpriteOvO]

Something is wrong with raw string.

/*
message: assertion `left == right` failed
             left: "# [doc = r\" doc\"]"
            right: "#[doc = \" doc\"]"
*/
fn test_outer_line_comment() {
    let tokens = quote! {
        /// doc
    };
    let expected = "# [doc = r\" doc\"]";
    assert_eq!(expected, tokens.to_string());
}

/*
message: `"r#raw_id"` is not a valid identifier
*/
fn test_quote_raw_id() {
    let id = quote!(r#raw_id);
    assert_eq!(id.to_string(), "r#raw_id");
}

[tgross35]

We probably need to call to Ident::new_raw rather than Ident::new in some cases, like quote has https://github.com/dtolnay/quote/blob/0245506323a3616daa2ee41c6ad0b871e4d78ae4/src/runtime.rs#L486.

For doc comments, maybe proc_macro2 and proc_macro unquote strings differently? @dtolnay would know better. You could run a test like this with our quote and see if it unquotes the same https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=19cdf0381b312f9ad5fac14af49e55f7.

[tgross35]

We probably need to call to Ident::new_raw rather than Ident::new in some cases, like quote has https://github.com/dtolnay/quote/blob/0245506323a3616daa2ee41c6ad0b871e4d78ae4/src/runtime.rs#L486.

Also if you prefer not to fix this here, it is okay to just open an issue and leave the test as a FIXME. I'd be happy to merge this nearly as-is since it's a big improvement over what we have, as long as tests pass and dtolnay has no objection.

Same goes for the r# string discrepancy as long as we understand where it is coming from.

[SpriteOvO]

Also if you prefer not to fix this here, it is okay to just open an issue and leave the test as a FIXME. I'd be happy to merge this nearly as-is since it's a big improvement over what we have, as long as tests pass and dtolnay has no objection.

I'm fine to fix it in this PR as long as it's not a huge diff :)

Also, compilefail tests are still being migrated.

[SpriteOvO]

For doc comments, maybe proc_macro2 and proc_macro unquote strings differently? @dtolnay would know better. You could run a test like this with our quote and see if it unquotes the same play.rust-lang.org?version=stable&mode=debug&edition=2021&gist=19cdf0381b312f9ad5fac14af49e55f7.

This is the output of our quote!. It looks like they have the same semantics - just escaped or not. I think it's fine to update the test cases with the new output.

&t = TokenStream [
    Punct {
        ch: '#',
        spacing: Alone,
        span: #5 bytes(76..94),
    },
    Group {
        delimiter: Bracket,
        stream: TokenStream [
            Ident {
                ident: "doc",
                span: #4 bytes(324157..324202),
            },
            Punct {
                ch: '=',
                spacing: Alone,
                span: #5 bytes(76..94),
            },
            Literal {
                kind: Str,
                symbol: " doc with \\\"all\\\" r\\\"kinds\\\" of r##\\\"strings\\\"##",
                suffix: None,
                span: #4 bytes(324157..324202),
            },
        ],
        span: #5 bytes(76..94),
    },
]
#[doc = " doc with \"all\" r\"kinds\" of r##\"strings\"##"]

[tgross35]

Couple small suggestions but otherwise lgtm.

Fyi all CI is having some problems currently.

[rust-log-analyzer]

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

curl: (22) The requested URL returned error: 404
curl: (22) The requested URL returned error: 404
ERROR: failed to download llvm from ci

    HELP: There could be two reasons behind this:
        1) The host triple is not supported for `download-ci-llvm`.
        2) Old builds get deleted after a certain time.
    HELP: In either case, disable `download-ci-llvm` in your config.toml:
    [llvm]
    download-ci-llvm = false
    
Build completed unsuccessfully in 0:00:23

[tgross35]

This is a significant improvement, thanks for the changes! Tree is closed for the CI fixes so this will have to wait a bit to merge.

[tgross35]

@bors r+

[bors]

📌 Commit ef69c30 has been approved by tgross35

It is now in the queue for this repository.