Implement #[proc_macro_lint] to generate LintId for macro-generated warnings

[dtolnay]

• API change proposal: Require a LintId for all proc-macro warnings libs-team#524

This PR unblocks an initial round of stabilizations of #54140 as discussed in #54140 (comment).

An id for a procedural macro warning is declared using #[proc_macro_lint] on a pub static contained in the crate root with type proc_macro::LintId. The attribute fills in a unique value for the static's value:

use proc_macro::{LintId, TokenStream};

#[proc_macro_lint]
pub static ambiguous_thing: LintId;

#[proc_macro_derive(Foo)]
pub fn derive_foo(input: TokenStream) -> TokenStream {
    if ... {
        proc_macro::warning(crate::ambiguous_thing, span, "...");
    }
    ...
}

Within the same proc macro crate, the static (ambiguous_thing) exists in the value namespace. It can be used as a value for passing to public APIs provided by the proc_macro crate. The value's type is proc_macro::LintId, which implements Copy and Debug.

Downstream of the proc macro crate, the same static exists in the macro namespace. It can be re-exported in the macro namespace using pub use. Currently it is not useful for anything else.

The use of 2 namespaces in such a way is identical to how all proc macros already work. For example, inside a proc macro crate containing #[proc_macro] pub fn foo(input: TokenStream) -> TokenStream, this function exists in the value namespace and is callable as a function. In downstream crates, the same function exists in the macro namespace and is callable as a function-like macro.

Future work

• Some of the public unstable API of proc_macro needs to be redesigned to require that a LintId must always be provided when a macro creates a warning. In this PR, I have made this change only for Diagnostic::span_warning and Diagnostic::warning. There is another constructor Diagnostic::new which takes a proc_macro::Level and can be passed Level::Warning. In this PR I have not touched that function, which means it is not on track for stabilizing. This is fine because it has already fallen out of favor in the tracking issue discussion and was not suggested for stabilization. See for example Tracking Issue: Procedural Macro Diagnostics (RFC 1566) #54140 (comment).

• Procedural macro LintId needs to be integrated into the allow/warn/deny lint level system. If a crate foo_macros defines a LintId called ambiguous_thing, and it is re-exported in the crate root of a crate foo, then users of foo need to be able to write allow(foo::ambiguous_thing) or deny(foo::ambiguous_thing).

• Procedural macro LintId needs to be integrated with the unknown_lints lint. If a user writes deny(foo::ambiguous_thing) when foo is not a proc macro crate declaring a ambiguous_thing lint id, nor is this resolved as a re-export of such a lint id, this needs to trigger unknown_lints.

• Rustdoc will need to render documentation for lint ids.

• Mechanism for a proc macro crate to set a default level for each of its lints. By default they are warn-by-default, but some lints might be better as allow-by-default or deny-by-default.

• A style lint that enforces a case convention for lint ids (snake_case).

• Rust-analyzer will want to provide autocomplete for proc macro lint ids inside allow/warn/deny.

Importantly, none of the above blocks stabilization of warning diagnostics APIs in proc_macro (as long as we do it carefully, i.e. not Diagnostic::new).

[rustbot]

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

HIR ty lowering was modified

cc @fmease

[rustbot]

HIR ty lowering was modified

cc @fmease

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[Nadrieril]

r? compiler

[rustbot]

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

[dtolnay]

@rustbot ready

[dtolnay]

Rebased to resolve conflict with #135826 in compiler/rustc_resolve/src/build_reduced_graph.rs and #134504 in compiler/rustc_middle/src/ty/sty.rs.

[dtolnay]

Updated with rename from #[proc_macro_warning] to #[proc_macro_lint] per library API team discussion (rust-lang/libs-team#524 (comment)).

[dtolnay]

Rebased to resolve conflict with #119286 in compiler/rustc_span/src/symbol.rs and #132156 in compiler/rustc_passes/src/check_attr.rs.

[dtolnay]

@rustbot labels +I-lang-nominated

#[proc_macro_lint] is a new attribute that cannot be implemented purely as a library feature, so in addition to the accepted library ACP (rust-lang/libs-team#524), this is also going to need a lang discussion.

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

[dtolnay]

Rebased to resolve conflict with #136751 in compiler/rustc_builtin_macros/src/proc_macro_harness.rs.

[bors]

☔ The latest upstream changes (presumably #136845) made this pull request unmergeable. Please resolve the merge conflicts.

[joshtriplett]

Seems reasonable. I think we should plan on being able to integrate the same LintId mechanism into declarative macros: ignoring the details of how such a macro emits diagnostics, there should be some way to declare a LintId in the macro-namespace (such that deny(path::to::the_lint) works) within an ordinary crate, not just a proc macro crate.

[joshtriplett]

We discussed this in today's @rust-lang/lang meeting. We're happy to approve this as a lang experiment. Using a path to identify a lint for the purposes of emitting it and suppressing/denying/etc it seems like the correct answer. (We definitely want to see paths used rather than strings.)

All the details of the proc_macro API to emit lints are going to be libs-api, here. From lang's perspective, we see the lang element as the mechanism to declare a LintId in a proc macro crate and have that become a path that can be used in an allow/warn/deny/etc in code that has pulled in that proc macro crate.

[petrochenkov]

This is a relatively major compiler change due to the newly introduced definitions, so I wanted to review the changes from that point of view before merging.
I'm back to work now, so I should be able to do it during the next weeks.

[traviscross]

Note that as a lang experiment without an accepted RFC, the proc_macro_diagnostic feature flag should be marked incomplete and emit the experimental lint.

[petrochenkov]

From the language point of view I don't like the amount of language extension that is done here to support a niche feature.

• The key new language extension here is support for general purpose path resolution in allow/warn/deny attributes (not yet implemented, but implied).
• The second extension is the new item kind and new names in macro namespace, however it's only required to support the first feature, without path resolution in lint attributes it will no longer be necessary.

Naturally, I'd prefer to avoid both of these extensions (with the second one automatically avoidable if the first one is removed).

Is it really necessary to support arbitrary exporting and renaming of proc macro lints (and require arbitrary path resolution to support it)?
The lints have two components namespace_name::lint_name.

• It's unlikely that someone is going to rename the second component, or use use namespace_name::lint_name; to make it usable without the namespace.
• For the first component, the 2-crate setup serde_macros reexported by serde is a very valid reason to not use the crate name as namespace_name literally.
  • However, this should be solvable by a new attribute like #![crate_name_but_only_for_lint_namespacing = "serde"] applied to serde_macros's root, which is a much smaller scale extension than resolving arbitrary paths.
  • As for the "canonical" namespace name like serde, I also don't expect it to be renamed by users.

[petrochenkov]

Technically, the implementation is mostly good, I'll leave some comments a bit later.

[petrochenkov]

Suggested change

	//~^ use of unstable library feature `proc_macro_diagnostic`
	//~^ ERROR use of unstable library feature `proc_macro_diagnostic`

Could you use full annotations in all of the tests?

[petrochenkov]

If string is used for the lint ID, then you can get ID collisions using macros 2.0.

macro m() { #[proc_macro_lint] pub static lint_name: LintId; }

m!();
m!(); // No "defined multiple times" error due to hygiene, but there is a lint ID collision

Could you add a test case for this?
No need to fix it right now though.

[petrochenkov]

Hmm, looks like we already have proc_macro_diagnostic as a library feature.
Is the intent to just put the new attribute under that umbrella feature for now?

[petrochenkov]

It is used for declarative macros as well.

[petrochenkov]

Is this custom reporting necessary?
static a: NonLintId = LintId::new("foo"); will report a type mismatch anyway.

(Also this looks a bit out of place in attribute checking.)

[petrochenkov]

It should be better to make LintId::new a lang item and add the static initializer in AST lowering using make_lang_item_path.
This will allow to

• avoid a mutable AST visitor, their use should be minimized because they stand in the way of plans like keeping AST on arena, etc.
• avoid the edition hack above, lang item collection will be used to "resolve" the LintId::new path instead

Same for adding the #[allow(non_upper_case_globals)] attribute, it's better to adjust the lint directly.