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

#[deny(unknown_lints)] doesn't apply to code from macros from external crates #109881

Open
peter-kehl opened this issue Apr 3, 2023 · 7 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) L-unknown_lints Lint: unknown_lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@peter-kehl
Copy link
Contributor

peter-kehl commented Apr 3, 2023

Code generated from proc_macro (or parts coming from proc_macro and incorporated (invoked by) a macro by example) doesn't get checked
by #[deny(unknown_lints)] (and friends). (Update: And the same from macro_rules in external crates - see @clubby789's explanation below.)

Can/should Rust report that? If not, could we document this?

That is even more confusing, because code coming from pure macro_rules!-based macros IS checked by #[deny(unknown_lints)]. (Update: Only if macro_rules is in not in an external crate.)

For an OLDER example, see https://github.com/peter-kehl/nonexisting_lint_from_proc_macro/tree/4f564d464d218bc2f0b211bd6d0e53075a5761b2/proc_mac/src/lib.rs for the proc macro, and https://github.com/peter-kehl/nonexisting_lint_from_proc_macro/tree/4f564d464d218bc2f0b211bd6d0e53075a5761b2/use_cases/src/lib.rs for its "consumer".

(Same on 1.68.2 and 1.70.0-nightly.)
(Updated; BUT I will provide a smaller example in a separate comment.)

@clubby789
Copy link
Contributor

clubby789 commented Apr 3, 2023

This isn't proc-macro specific, the lint is just configured not to fire on code inside macros in external crates (missing report_in_external_macro) - if the macro_rules macro is defined upstream, there is also no warning. This is because if the offending code comes from an upstream crate, it's no fault of the user so there's no point emitting a lint which they can't fix

If, however, your macro looks like this

macro_rules! bad {
    ($pat:path) => {
        #[allow($pat)] {}
    };
}

That will raise a warning on a bad lint, because the primary span comes from user code.
@rustbot label +A-lint +A-macros

@rustbot rustbot added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) labels Apr 3, 2023
@peter-kehl
Copy link
Contributor Author

Thank you @clubby789. I'm updating the issue name respectively.

@peter-kehl peter-kehl changed the title #[deny(unknown_lints)] doesn't apply to code from proc_macro #[deny(unknown_lints)] doesn't apply to code from macros from external crates Apr 3, 2023
@Nilstrieb Nilstrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@peter-kehl
Copy link
Contributor Author

peter-kehl commented Apr 11, 2023

Is this determined only by span, and if so, how can one check or print/visualize spans from proc macros, please? When I format! the result TokenStream with Debug output, it's:
Output: TokenStream [Punct { ch: '#', spacing: Joint, span: #5 bytes(227..282) }, Group { delimiter: Bracket, stream: TokenStream [Ident { ident: "allow", span: #5 bytes(227..282) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "non_existing_std_lint_from_proc_macro", span: #5 bytes(227..282) }], span: #5 bytes(227..282) }], span: #5 bytes(227..282) }, Ident { ident: "fn", span: #5 bytes(227..282) }, Ident { ident: "_f", span: #5 bytes(227..282) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #5 bytes(227..282) }, Group { delimiter: Brace, stream: TokenStream [], span: #5 bytes(227..282) }]
(I intentionally don't use syn and quote crates, as I'd like this to be lightweight.)

I've updated the example to .set_span(Span::call_site()) everywhere I can see possible, but an incorrect lint name still doesn't get reported: https://github.com/peter-kehl/nonexisting_lint_from_proc_macro/blob/main/proc_mac/src/lib.rs and https://github.com/peter-kehl/nonexisting_lint_from_proc_macro/blob/main/use_cases/src/lib.rs.

@clubby789
Copy link
Contributor

clubby789 commented Apr 11, 2023

Is this determined only by span

Yes, the lint is silenced if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s))

peter-kehl added a commit to peter-kehl/nonexisting_lint_from_proc_macro that referenced this issue May 21, 2023
@peter-kehl
Copy link
Contributor Author

peter-kehl commented May 21, 2023

If a non-existing lint comes from/through paste crate (a proc_macro crate: dtolnay/paste), the lint DOES get reported. I've updated my example.

Unfortunately, I can't use paste, because paste can generate Ident only, but I need to generate Path. I'll investigate if paste generates anything else different to how I generate.

@peter-kehl
Copy link
Contributor Author

Summary: The above problem was caused by (my) using Span::call_site() when generating Ident(s) for the lint name (or the lint path, if prefixed with clippy or rustdoc).

Solution: Use a Span object coming from the macro input. It's enough to use such a Span for the Ident(s) covering the lint name (or path). The rest of the generated code (that is: #[allow( and its matching )]) can use Span::call_site().

For prefixless (standard) lints (as opposed to clippy:: or rustdoc:: lints) you can use paste crate. (That doesn't work for clippy:: or rustdoc:: lints, because their lint is a path rather than just one identifier). See https://github.com/peter-kehl/nonexisting_lint_from_proc_macro/blob/main/use_cases/src/lib.rs#L18.

For the proof that a generated lint name fails to be reported as an incorrect lint when using Span::call_site(), you could modify 1.0.12 (the current version) of paste crate: https://github.com/dtolnay/paste/blob/58a4a8f86aad820d58010bf2377405f2bf2f56fd/src/lib.rs#L442 > fn pasted_to_tokens > change let ident = match panic::catch_unwind(|| Ident::new(&pasted, span)) {...} to let ident = match panic::catch_unwind(|| Ident::new(&pasted, Span::call_site())) {...} and the above example stops failing for incorrect lint names.

@peter-kehl
Copy link
Contributor Author

peter-kehl commented May 25, 2023

Request:
Could this be documented at Span::call_site(), and/or anywhere else (and linked to from docs of Span::call_site()), please? Because as-is, this is highly confusing/misleading.

Documentation of Span::call_site(): "...resolved as if they were written directly at the macro call location..." Based on that, one would expect it to be interchangeable with Span coming from the macro input stream, that is, the "macro call location" (except for a source code position shift).

grep -r in_external_macro in rust's sources shows some 163 occurrences, so could this affect other use cases?

peter-kehl added a commit to coop-rs/allow that referenced this issue May 25, 2023
…en prefixed lint name. Version 0.2.0 - but work in progress.
peter-kehl added a commit to coop-rs/allow that referenced this issue May 26, 2023
peter-kehl added a commit to coop-rs/allow that referenced this issue May 26, 2023
peter-kehl added a commit to coop-rs/allow that referenced this issue May 26, 2023
@jieyouxu jieyouxu added the L-unknown_lints Lint: unknown_lints label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) L-unknown_lints Lint: unknown_lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants