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

function "in_external_macro” judgment is incorrect #131993

Open
yu532758082 opened this issue Oct 21, 2024 · 4 comments
Open

function "in_external_macro” judgment is incorrect #131993

yu532758082 opened this issue Oct 21, 2024 · 4 comments
Labels
C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress.

Comments

@yu532758082
Copy link

I use the derive macro provided by the clap crate in my code, and then call the in_external_macro function in the context of a custom lint diagnostic code. The function determines that the code derived from the derive macro is local code.

I tried this code:

fn main() {
    let _a = 1;
}

use clap::Args;


#[derive(Args)]
pub struct ToFrom {
    #[arg(long = "workspace")]
    a: i32,
}

dependency is clap = { version = "=4.2.0", features = ["derive"] }

My lint code is:

    fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
           let res = in_external_macro(cx.sess(), block.span);
        info!("check block in_external_macro res {:?}", res);
        if res == false {
            info!("check block {:?}", block);
        }
}

I expected to see this happen: I think all code generated by derived macros should belong to external macros derived from external crates.

Instead, this happened: Some of the code generated by the derived macro is considered to be local code

The error macro code is expanded as follows.
The blocks inside the .arg function are considered native code.

                .arg({
                    #[allow(deprecated)]
                    let arg = clap::Arg::new("a")
                        .value_name("A")
                        .required(true && clap::ArgAction::Set.takes_values())
                        .value_parser({
                            use ::clap_builder::builder::via_prelude::*;
                            let auto = ::clap_builder::builder::_AutoValueParser::<
                                i32,
                            >::new();
                            (&&&&&&auto).value_parser()
                        })
                        .action(clap::ArgAction::Set);
                    let arg = arg.long("workspace");
                    let arg = arg.required(false);
                    arg
                });

rustc --version --verbose:

+nightly-2024-03-07
@yu532758082 yu532758082 added the C-bug Category: This is a bug. label Oct 21, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 21, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Oct 21, 2024

Can you please provide a complete reproducer, including how your lint is setup? It doesn't need to be minimal, I just need something that's more complete instead of having to figure out how to reproduce the issue you are observing.

Is this in the context of clippy lints, or rustc lints, or some other custom lint framework?

@jieyouxu jieyouxu added S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress. S-needs-info Status: The issue lacks details necessary to triage or act on it. labels Oct 21, 2024
@yu532758082
Copy link
Author

Can you please provide a complete reproducer, including how your lint is setup? It doesn't need to be minimal, I just need something that's more complete instead of having to figure out how to reproduce the issue you are observing.

Is this in the context of clippy lints, or rustc lints, or some other custom lint framework?

My lint context is custom, but the logic is similar to clippy, which should have no effect on the function return value.

@jieyouxu
Copy link
Member

jieyouxu commented Oct 21, 2024

My lint context is custom, but the logic is similar to clippy, which should have no effect on the function return value.

I can't test your changes because I don't have a complete example, but, if you point your code to use a local clap checkout clap = { path = "my/local/checkout", features = ["derive"] }, then modify that clap checkout's derive proc-macro https://github.com/clap-rs/clap/blob/61f5ee514f8f60ed8f04c6494bdf36c19e7a8126/clap_derive/src/derives/args.rs

such that all the code with the code pattern matching quote_spaned! { $SPAN => $PARTIAL }

quote_spanned! { ty.span()=>
                #arg_matches.#get_many(#id)
                    .map(|v| v.collect::<Vec<_>>())
                    .unwrap_or_else(Vec::new)
            }

becomes quote! { $PARTIAL }, i.e. stop forwarding the user's span without attaching a layer of syntax context

quote! { #arg_matches.#get_many(#id)
                    .map(|v| v.collect::<Vec<_>>())
                    .unwrap_or_else(Vec::new)
}

or maybe

quote_spanned! { Span::call_site().located_at(ty.span()) =>
                #arg_matches.#get_many(#id)
                    .map(|v| v.collect::<Vec<_>>())
                    .unwrap_or_else(Vec::new)
            }

Does making any of the two changes cause in_external_macro in your custom lint to no longer report false?

@jieyouxu jieyouxu removed the S-needs-info Status: The issue lacks details necessary to triage or act on it. label Oct 21, 2024
@yu532758082
Copy link
Author

My lint context is custom, but the logic is similar to clippy, which should have no effect on the function return value.

I can't test your changes because I don't have a complete example, but, if you point your code to use a local clap checkout clap = { path = "my/local/checkout", features = ["derive"] }, then modify that clap checkout's derive proc-macro https://github.com/clap-rs/clap/blob/61f5ee514f8f60ed8f04c6494bdf36c19e7a8126/clap_derive/src/derives/args.rs

such that all the code with the code pattern matching quote_spaned! { $SPAN => $PARTIAL }

quote_spanned! { ty.span()=>
#arg_matches.#get_many(#id)
.map(|v| v.collect::<Vec<_>>())
.unwrap_or_else(Vec::new)
}
becomes quote! { $PARTIAL }, i.e. stop forwarding the user's span without attaching a layer of syntax context

quote! { #arg_matches.#get_many(#id)
.map(|v| v.collect::<Vec<_>>())
.unwrap_or_else(Vec::new)
}
or maybe

quote_spanned! { Span::call_site().located_at(ty.span()) =>
#arg_matches.#get_many(#id)
.map(|v| v.collect::<Vec<_>>())
.unwrap_or_else(Vec::new)
}
Does making any of the two changes cause in_external_macro in your custom lint to no longer report false?

I change quote_spam! to quote but not change the result

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress.
Projects
None yet
Development

No branches or pull requests

3 participants