use_self: Fix false positive in macro expansion

[Y-Nak]

fixes #6902

changelog: use_self: Fix false positive related to macro expansion.

[rust-highfive]

r? @phansch

(rust-highfive has picked a reviewer for you, use r? to override)

[camsteffen]

This looks expensive and doesn't seem like the right solution to me. We probably need to check in_macro somewhere while descending into the HIR rather than the other way around.

[flip1995]

I agree, we shouldn't have to traverse the HIR to determine if something comes from a macro expansion. Is this maybe a bug in rustc, not tracking the origin of this span correctly?

[flip1995]

After doing some debugging I'm pretty sure this is a bug in the span lowering in rustc. Playground (You can expand the code under "Tools").

Expanded code – relevant snippet:

if let _serde::__private::Ok(__ok) =
       match _serde::Deserializer::deserialize_any(_serde::__private::de::ContentRefDeserializer::<__D::Error>::new(&__content),
                                                   _serde::__private::de::UntaggedUnitVisitor::new("Direction",
                                                                                                   "North"))
           {
           _serde::__private::Ok(()) =>
           _serde::__private::Ok(Direction::North),
           _serde::__private::Err(__err) =>
           _serde::__private::Err(__err),
       } {
    return _serde::__private::Ok(__ok);
}

The error is emitted for Direction::North. The weird thing is that when removing #[serde(untagged)] the exact same

_serde::__private::Ok(Direction::North)

Ctor is used in the code, but doesn't trigger the lint.

dbg!(span.ctxt().outer_expn_data() for the QPath of Direction::Nord gives:

[clippy_lints/src/use_self.rs:398] span.ctxt().outer_expn_data() = ExpnData {
    kind: Root,
    parent: ExpnId(
        0,
    ),
    call_site: tests/ui/issue-6902.rs:1:1: 1:1 (#0),
    def_site: tests/ui/issue-6902.rs:1:1: 1:1 (#0),
    allow_internal_unstable: None,
    allow_internal_unsafe: false,
    local_inner_macros: false,
    edition: Edition2015,
    macro_def_id: Some(
        DefId(0:0 ~ issue_6902[317d]),
    ),
    krate: crate0,
    orig_id: Some(
        0,
    ),
    disambiguator: 0,
}

TL;DR: It doesn't come from an expansion at all. Since this code is obviously generated and clearly not written by the user the compiler should track that it is expanded.

As a really wild guess, I would say, that this is because in the first case the expression is in the condition of an if let expression and in the second case it is just in a return position.

[flip1995]

So after trying to reproduce this for a few hours, I give up. I have this proc_macro that produces really similar code to the serde expansion:

#[proc_macro_derive(EnumUsedInIfLet)]
pub fn use_self(input: TokenStream) -> proc_macro::TokenStream {
    let item: syn::ItemEnum = parse_macro_input!(input);
    let _ident = item.ident;
    TokenStream::from(quote! {
        const _: () =
            {
                #[automatically_derived]
                impl Foo for #_ident {
                    fn foo() -> Result<Self, &'static str> {
                        if let Ok(ok) = match bar() {
                            Ok(()) => Ok(#_ident::A),
                            Err(err) => Err(err),
                        } {
                            return Ok(ok);
                        }
                        Err("Ohoh")
                    }
                }
            };
    })
}

Expanded:

trait Foo {
    fn foo() -> Result<S, &'static str>;
}

fn bar() -> Result<(), &'static str> { Ok(()) }

#[derive(EnumUsedInIfLet)]
enum S { A }

enum S { A, }
const _: () =
    {
        #[automatically_derived]
        impl Foo for S {
            fn foo() -> Result<Self, &'static str> {
                if let Ok(ok) =
                       match bar() {
                           Ok(()) => Ok(S::A),
                           Err(err) => Err(err),
                       } {
                    return Ok(ok);
                }
                Err("Ohoh")
            }
        }
    };

But it doesn't trigger the lint. So my guess is, that serde does som string manipulation in the proc macro and therefore rustc is unable to tell that the expanded span comes from a macro.

---

So what I get from this: This fix doesn't work generally. It just randomly works here, because rustc is able to figure out the spans of the parent expressions.

[camsteffen]

Same root issue as #6514?

[flip1995]

Unfortunately, neither Clippy nor rustc currently have a way to tell apart a quote_spanned span from a non-macro span, so this is currently impossible to fix on our side (Also see #6249 (comment)).

Seems like it

[Y-Nak]

Sorry for the late response, I took a vacation.
Yeah, I admit this is a really hacky solution and I'll close the PR.

In https://github.com/Y-Nak/clippy-6902-repro, I succeeded to find a minimal reproducer for #6902 and it seems not relevant to quote_spanned, so we may need further investigation on it.

The weird thing is that when removing #[serde(untagged)] the exact same
_serde::__private::Ok(Direction::North)
Ctor is used in the code, but doesn't trigger the lint.

No, this is not weird because the Direction::North appears in impl <'de> _serde::de::Visitor<'de> for __Visitor<'de> in the expansion.

[flip1995]

Ah thanks for doing some more investigation! Looking at your reproducer, I think the only thing I missed was that also the variant in the qpath had to be generated inside the macro with syn.

Do you want to open an issue in rust-lang/rust about this with your reproducer?

No, this is not weird because the Direction::North appears in impl <'de> _serde::de::Visitor<'de> for __Visitor<'de> in the expansion.

Ah right, makes sense.

[Y-Nak]

Do you want to open an issue in rust-lang/rust about this with your reproducer?

Yes, I'll open an issue.

[Y-Nak]

Opened rust-lang/rust#84122.