1
Fork 0

Auto merge of #95542 - xFrednet:rfc-2383-expect-query, r=wesleywiser

Support tool lints with the `#[expect]` attribute (RFC 2383)

This PR fixes the ICE https://github.com/rust-lang/rust/issues/94953 by making the assert for converted expectation IDs conditional.

Additionally, it moves the lint expectation check into a separate query to support rustdoc and other tools. On the way, I've also added some tests to ensure that the attribute works for Clippy and rustdoc lints.

The number of changes comes from the long test file. This may look like a monster PR, this may smell like a monster PR and this may be a monster PR, but it's a harmless monster. 🦕

---

Closes: https://github.com/rust-lang/rust/issues/94953

cc: https://github.com/rust-lang/rust/issues/85549

r? `@wesleywiser`

cc: `@rust-lang/rustdoc`
This commit is contained in:
bors 2022-05-09 00:02:55 +00:00
commit a57117982a
18 changed files with 663 additions and 19 deletions

View file

@ -426,6 +426,13 @@ struct HandlerInner {
future_breakage_diagnostics: Vec<Diagnostic>,
/// The [`Self::unstable_expect_diagnostics`] should be empty when this struct is
/// dropped. However, it can have values if the compilation is stopped early
/// or is only partially executed. To avoid ICEs, like in rust#94953 we only
/// check if [`Self::unstable_expect_diagnostics`] is empty, if the expectation ids
/// have been converted.
check_unstable_expect_diagnostics: bool,
/// Expected [`Diagnostic`]s store a [`LintExpectationId`] as part of
/// the lint level. [`LintExpectationId`]s created early during the compilation
/// (before `HirId`s have been defined) are not stable and can therefore not be
@ -497,12 +504,14 @@ impl Drop for HandlerInner {
);
}
if self.check_unstable_expect_diagnostics {
assert!(
self.unstable_expect_diagnostics.is_empty(),
"all diagnostics with unstable expectations should have been converted",
);
}
}
}
impl Handler {
pub fn with_tty_emitter(
@ -574,6 +583,7 @@ impl Handler {
emitted_diagnostics: Default::default(),
stashed_diagnostics: Default::default(),
future_breakage_diagnostics: Vec::new(),
check_unstable_expect_diagnostics: false,
unstable_expect_diagnostics: Vec::new(),
fulfilled_expectations: Default::default(),
}),
@ -988,12 +998,13 @@ impl Handler {
&self,
unstable_to_stable: &FxHashMap<LintExpectationId, LintExpectationId>,
) {
let diags = std::mem::take(&mut self.inner.borrow_mut().unstable_expect_diagnostics);
let mut inner = self.inner.borrow_mut();
let diags = std::mem::take(&mut inner.unstable_expect_diagnostics);
inner.check_unstable_expect_diagnostics = true;
if diags.is_empty() {
return;
}
let mut inner = self.inner.borrow_mut();
for mut diag in diags.into_iter() {
diag.update_unstable_expectation_id(unstable_to_stable);

View file

@ -1009,6 +1009,10 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
});
}
);
// This check has to be run after all lints are done processing. We don't
// define a lint filter, as all lint checks should have finished at this point.
sess.time("check_lint_expectations", || tcx.check_expectations(None));
});
Ok(())

View file

@ -1,10 +1,16 @@
use crate::builtin;
use rustc_hir::HirId;
use rustc_middle::ty::query::Providers;
use rustc_middle::{lint::LintExpectation, ty::TyCtxt};
use rustc_session::lint::LintExpectationId;
use rustc_span::symbol::sym;
use rustc_span::Symbol;
pub fn check_expectations(tcx: TyCtxt<'_>) {
pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { check_expectations, ..*providers };
}
fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option<Symbol>) {
if !tcx.sess.features_untracked().enabled(sym::lint_reasons) {
return;
}
@ -13,7 +19,9 @@ pub fn check_expectations(tcx: TyCtxt<'_>) {
let lint_expectations = &tcx.lint_levels(()).lint_expectations;
for (id, expectation) in lint_expectations {
if !fulfilled_expectations.contains(id) {
if !fulfilled_expectations.contains(id)
&& tool_filter.map_or(true, |filter| expectation.lint_tool == Some(filter))
{
// This check will always be true, since `lint_expectations` only
// holds stable ids
if let LintExpectationId::Stable { hir_id, .. } = id {

View file

@ -503,7 +503,4 @@ pub fn check_crate<'tcx, T: LateLintPass<'tcx>>(
});
},
);
// This check has to be run after all lints are done processing for this crate
tcx.sess.time("check_lint_expectations", || crate::expect::check_expectations(tcx));
}

View file

@ -371,7 +371,12 @@ impl<'s> LintLevelsBuilder<'s> {
};
self.lint_expectations.push((
expect_id,
LintExpectation::new(reason, sp, is_unfulfilled_lint_expectations),
LintExpectation::new(
reason,
sp,
is_unfulfilled_lint_expectations,
tool_name,
),
));
}
let src = LintLevelSource::Node(
@ -400,8 +405,10 @@ impl<'s> LintLevelsBuilder<'s> {
self.insert_spec(*id, (level, src));
}
if let Level::Expect(expect_id) = level {
self.lint_expectations
.push((expect_id, LintExpectation::new(reason, sp, false)));
self.lint_expectations.push((
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
));
}
}
Err((Some(ids), ref new_lint_name)) => {
@ -444,8 +451,10 @@ impl<'s> LintLevelsBuilder<'s> {
self.insert_spec(*id, (level, src));
}
if let Level::Expect(expect_id) = level {
self.lint_expectations
.push((expect_id, LintExpectation::new(reason, sp, false)));
self.lint_expectations.push((
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
));
}
}
Err((None, _)) => {
@ -550,8 +559,10 @@ impl<'s> LintLevelsBuilder<'s> {
}
}
if let Level::Expect(expect_id) = level {
self.lint_expectations
.push((expect_id, LintExpectation::new(reason, sp, false)));
self.lint_expectations.push((
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
));
}
} else {
panic!("renamed lint does not exist: {}", new_name);

View file

@ -109,6 +109,7 @@ pub use rustc_session::lint::{LintArray, LintPass};
pub fn provide(providers: &mut Providers) {
levels::provide(providers);
expect::provide(providers);
*providers = Providers { lint_mod, ..*providers };
}

View file

@ -210,6 +210,10 @@ pub struct LintExpectation {
/// adjusted to include an additional note. Therefore, we have to track if
/// the expectation is for the lint.
pub is_unfulfilled_lint_expectations: bool,
/// This will hold the name of the tool that this lint belongs to. For
/// the lint `clippy::some_lint` the tool would be `clippy`, the same
/// goes for `rustdoc`. This will be `None` for rustc lints
pub lint_tool: Option<Symbol>,
}
impl LintExpectation {
@ -217,8 +221,9 @@ impl LintExpectation {
reason: Option<Symbol>,
emission_span: Span,
is_unfulfilled_lint_expectations: bool,
lint_tool: Option<Symbol>,
) -> Self {
Self { reason, emission_span, is_unfulfilled_lint_expectations }
Self { reason, emission_span, is_unfulfilled_lint_expectations, lint_tool }
}
}

View file

@ -157,6 +157,25 @@ rustc_queries! {
desc { "running analysis passes on this crate" }
}
/// This query checks the fulfillment of collected lint expectations.
/// All lint emitting queries have to be done before this is executed
/// to ensure that all expectations can be fulfilled.
///
/// This is an extra query to enable other drivers (like rustdoc) to
/// only execute a small subset of the `analysis` query, while allowing
/// lints to be expected. In rustc, this query will be executed as part of
/// the `analysis` query and doesn't have to be called a second time.
///
/// Tools can additionally pass in a tool filter. That will restrict the
/// expectations to only trigger for lints starting with the listed tool
/// name. This is useful for cases were not all linting code from rustc
/// was called. With the default `None` all registered lints will also
/// be checked for expectation fulfillment.
query check_expectations(key: Option<Symbol>) -> () {
eval_always
desc { "checking lint expectations (RFC 2383)" }
}
/// Maps from the `DefId` of an item (trait/struct/enum/fn) to its
/// associated generics.
query generics_of(key: DefId) -> ty::Generics {

View file

@ -435,6 +435,16 @@ impl Key for Symbol {
}
}
impl Key for Option<Symbol> {
#[inline(always)]
fn query_crate_is_local(&self) -> bool {
true
}
fn default_span(&self, _tcx: TyCtxt<'_>) -> Span {
DUMMY_SP
}
}
/// Canonical query goals correspond to abstract trait operations that
/// are not tied to any crate in particular.
impl<'tcx, T> Key for Canonical<'tcx, T> {

View file

@ -232,6 +232,8 @@ crate fn create_config(
rustc_lint::builtin::RENAMED_AND_REMOVED_LINTS.name.to_string(),
rustc_lint::builtin::UNKNOWN_LINTS.name.to_string(),
rustc_lint::builtin::UNEXPECTED_CFGS.name.to_string(),
// this lint is needed to support `#[expect]` attributes
rustc_lint::builtin::UNFULFILLED_LINT_EXPECTATIONS.name.to_string(),
];
lints_to_show.extend(crate::lint::RUSTDOC_LINTS.iter().map(|lint| lint.name.to_string()));
@ -463,6 +465,8 @@ crate fn run_global_ctxt(
}
}
tcx.sess.time("check_lint_expectations", || tcx.check_expectations(Some(sym::rustdoc)));
if tcx.sess.diagnostic().has_errors_or_lint_errors().is_some() {
rustc_errors::FatalError.raise();
}

View file

@ -0,0 +1,157 @@
// check-pass
#![feature(lint_reasons)]
//! This file tests the `#[expect]` attribute implementation for tool lints. The same
//! file is used to test clippy and rustdoc. Any changes to this file should be synced
//! to the other test files as well.
//!
//! Expectations:
//! * rustc: only rustc lint expectations are emitted
//! * clippy: rustc and Clippy's expectations are emitted
//! * rustdoc: only rustdoc lint expectations are emitted
//!
//! This test can't cover every lint from Clippy, rustdoc and potentially other
//! tools that will be developed. This therefore only tests a small subset of lints
#![expect(rustdoc::missing_crate_level_docs)]
//~^ WARNING this lint expectation is unfulfilled [unfulfilled_lint_expectations]
//~| NOTE `#[warn(unfulfilled_lint_expectations)]` on by default
mod rustc_ok {
//! See <https://doc.rust-lang.org/rustc/lints/index.html>
#[expect(dead_code)]
pub fn rustc_lints() {
let x = 42.0;
#[expect(illegal_floating_point_literal_pattern)]
match x {
5.0 => {}
6.0 => {}
_ => {}
}
}
}
mod rustc_warn {
//! See <https://doc.rust-lang.org/rustc/lints/index.html>
#[expect(dead_code)]
pub fn rustc_lints() {
let x = 42;
#[expect(illegal_floating_point_literal_pattern)]
match x {
5 => {}
6 => {}
_ => {}
}
}
}
pub mod rustdoc_ok {
//! See <https://doc.rust-lang.org/rustdoc/lints.html>
#[expect(rustdoc::broken_intra_doc_links)]
/// I want to link to [`Nonexistent`] but it doesn't exist!
pub fn foo() {}
#[expect(rustdoc::invalid_html_tags)]
/// <h1>
pub fn bar() {}
#[expect(rustdoc::bare_urls)]
/// http://example.org
pub fn baz() {}
}
pub mod rustdoc_warn {
//! See <https://doc.rust-lang.org/rustdoc/lints.html>
#[expect(rustdoc::broken_intra_doc_links)]
//~^ WARNING this lint expectation is unfulfilled [unfulfilled_lint_expectations]
/// I want to link to [`bar`] but it doesn't exist!
pub fn foo() {}
#[expect(rustdoc::invalid_html_tags)]
//~^ WARNING this lint expectation is unfulfilled [unfulfilled_lint_expectations]
/// <h1></h1>
pub fn bar() {}
#[expect(rustdoc::bare_urls)]
//~^ WARNING this lint expectation is unfulfilled [unfulfilled_lint_expectations]
/// <http://example.org>
pub fn baz() {}
}
mod clippy_ok {
//! See <https://rust-lang.github.io/rust-clippy/master/index.html>
#[expect(clippy::almost_swapped)]
fn foo() {
let mut a = 0;
let mut b = 9;
a = b;
b = a;
}
#[expect(clippy::bytes_nth)]
fn bar() {
let _ = "Hello".bytes().nth(3);
}
#[expect(clippy::if_same_then_else)]
fn baz() {
let _ = if true {
42
} else {
42
};
}
#[expect(clippy::logic_bug)]
fn burger() {
let a = false;
let b = true;
if a && b || a {}
}
}
mod clippy_warn {
//! See <https://rust-lang.github.io/rust-clippy/master/index.html>
#[expect(clippy::almost_swapped)]
fn foo() {
let mut a = 0;
let mut b = 9;
a = b;
}
#[expect(clippy::bytes_nth)]
fn bar() {
let _ = "Hello".as_bytes().get(3);
}
#[expect(clippy::if_same_then_else)]
fn baz() {
let _ = if true {
33
} else {
42
};
}
#[expect(clippy::logic_bug)]
fn burger() {
let a = false;
let b = true;
let c = false;
if a && b || c {}
}
}
fn main() {
rustc_warn::rustc_lints();
}

View file

@ -0,0 +1,28 @@
warning: this lint expectation is unfulfilled
--> $DIR/expect-tool-lint-rfc-2383.rs:16:11
|
LL | #![expect(rustdoc::missing_crate_level_docs)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unfulfilled_lint_expectations)]` on by default
warning: this lint expectation is unfulfilled
--> $DIR/expect-tool-lint-rfc-2383.rs:71:14
|
LL | #[expect(rustdoc::broken_intra_doc_links)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: this lint expectation is unfulfilled
--> $DIR/expect-tool-lint-rfc-2383.rs:76:14
|
LL | #[expect(rustdoc::invalid_html_tags)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: this lint expectation is unfulfilled
--> $DIR/expect-tool-lint-rfc-2383.rs:81:14
|
LL | #[expect(rustdoc::bare_urls)]
| ^^^^^^^^^^^^^^^^^^
warning: 4 warnings emitted

View file

@ -0,0 +1,155 @@
// check-pass
#![feature(lint_reasons)]
//! This file tests the `#[expect]` attribute implementation for tool lints. The same
//! file is used to test clippy and rustdoc. Any changes to this file should be synced
//! to the other test files.
//!
//! Expectations:
//! * rustc: only rustc lint expectations are emitted
//! * clippy: rustc and Clippy's expectations are emitted
//! * rustdoc: only rustdoc lint expectations are emitted
//!
//! This test can't cover every lint from Clippy, rustdoc and potentially other
//! tools that will be developed. This therefore only tests a small subset of lints
#![expect(rustdoc::missing_crate_level_docs)]
mod rustc_ok {
//! See <https://doc.rust-lang.org/rustc/lints/index.html>
#[expect(dead_code)]
pub fn rustc_lints() {
let x = 42.0;
#[expect(illegal_floating_point_literal_pattern)]
match x {
5.0 => {}
6.0 => {}
_ => {}
}
}
}
mod rustc_warn {
//! See <https://doc.rust-lang.org/rustc/lints/index.html>
#[expect(dead_code)]
//~^ WARNING this lint expectation is unfulfilled [unfulfilled_lint_expectations]
//~| NOTE `#[warn(unfulfilled_lint_expectations)]` on by default
pub fn rustc_lints() {
let x = 42;
#[expect(illegal_floating_point_literal_pattern)]
//~^ WARNING this lint expectation is unfulfilled [unfulfilled_lint_expectations]
match x {
5 => {}
6 => {}
_ => {}
}
}
}
pub mod rustdoc_ok {
//! See <https://doc.rust-lang.org/rustdoc/lints.html>
#[expect(rustdoc::broken_intra_doc_links)]
/// I want to link to [`Nonexistent`] but it doesn't exist!
pub fn foo() {}
#[expect(rustdoc::invalid_html_tags)]
/// <h1>
pub fn bar() {}
#[expect(rustdoc::bare_urls)]
/// http://example.org
pub fn baz() {}
}
pub mod rustdoc_warn {
//! See <https://doc.rust-lang.org/rustdoc/lints.html>
#[expect(rustdoc::broken_intra_doc_links)]
/// I want to link to [`bar`] but it doesn't exist!
pub fn foo() {}
#[expect(rustdoc::invalid_html_tags)]
/// <h1></h1>
pub fn bar() {}
#[expect(rustdoc::bare_urls)]
/// <http://example.org>
pub fn baz() {}
}
mod clippy_ok {
//! See <https://rust-lang.github.io/rust-clippy/master/index.html>
#[expect(clippy::almost_swapped)]
fn foo() {
let mut a = 0;
let mut b = 9;
a = b;
b = a;
}
#[expect(clippy::bytes_nth)]
fn bar() {
let _ = "Hello".bytes().nth(3);
}
#[expect(clippy::if_same_then_else)]
fn baz() {
let _ = if true {
42
} else {
42
};
}
#[expect(clippy::logic_bug)]
fn burger() {
let a = false;
let b = true;
if a && b || a {}
}
}
mod clippy_warn {
//! See <https://rust-lang.github.io/rust-clippy/master/index.html>
#[expect(clippy::almost_swapped)]
fn foo() {
let mut a = 0;
let mut b = 9;
a = b;
}
#[expect(clippy::bytes_nth)]
fn bar() {
let _ = "Hello".as_bytes().get(3);
}
#[expect(clippy::if_same_then_else)]
fn baz() {
let _ = if true {
33
} else {
42
};
}
#[expect(clippy::logic_bug)]
fn burger() {
let a = false;
let b = true;
let c = false;
if a && b || c {}
}
}
fn main() {
rustc_warn::rustc_lints();
}

View file

@ -0,0 +1,16 @@
warning: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:37:14
|
LL | #[expect(dead_code)]
| ^^^^^^^^^
|
= note: `#[warn(unfulfilled_lint_expectations)]` on by default
warning: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:43:18
|
LL | #[expect(illegal_floating_point_literal_pattern)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: 2 warnings emitted

View file

@ -0,0 +1,16 @@
// This ensures that ICEs like rust#94953 don't happen
// check-pass
// compile-flags: -Z unpretty=expanded
#![feature(lint_reasons)]
// This `expect` will create an expectation with an unstable expectation id
#[expect(while_true)]
fn create_early_lint_pass_expectation() {
// `while_true` is an early lint
while true {}
}
fn main() {
create_early_lint_pass_expectation();
}

View file

@ -0,0 +1,20 @@
#![feature(prelude_import)]
#![no_std]
// This ensures that ICEs like rust#94953 don't happen
// check-pass
// compile-flags: -Z unpretty=expanded
#![feature(lint_reasons)]
#[prelude_import]
use ::std::prelude::rust_2015::*;
#[macro_use]
extern crate std;
// This `expect` will create an expectation with an unstable expectation id
#[expect(while_true)]
fn create_early_lint_pass_expectation() {
// `while_true` is an early lint
while true {}
}
fn main() { create_early_lint_pass_expectation(); }

View file

@ -0,0 +1,142 @@
// check-pass
#![feature(lint_reasons)]
//! This file tests the `#[expect]` attribute implementation for tool lints. The same
//! file is used to test clippy and rustdoc. Any changes to this file should be synced
//! to the other test files as well.
//!
//! Expectations:
//! * rustc: only rustc lint expectations are emitted
//! * clippy: rustc and Clippy's expectations are emitted
//! * rustdoc: only rustdoc lint expectations are emitted
//!
//! This test can't cover every lint from Clippy, rustdoc and potentially other
//! tools that will be developed. This therefore only tests a small subset of lints
#![expect(rustdoc::missing_crate_level_docs)]
mod rustc_ok {
//! See <https://doc.rust-lang.org/rustc/lints/index.html>
#[expect(dead_code)]
pub fn rustc_lints() {
let x = 42.0;
#[expect(illegal_floating_point_literal_pattern)]
match x {
5.0 => {}
6.0 => {}
_ => {}
}
}
}
mod rustc_warn {
//! See <https://doc.rust-lang.org/rustc/lints/index.html>
#[expect(dead_code)]
pub fn rustc_lints() {
let x = 42;
#[expect(illegal_floating_point_literal_pattern)]
match x {
5 => {}
6 => {}
_ => {}
}
}
}
pub mod rustdoc_ok {
//! See <https://doc.rust-lang.org/rustdoc/lints.html>
#[expect(rustdoc::broken_intra_doc_links)]
/// I want to link to [`Nonexistent`] but it doesn't exist!
pub fn foo() {}
#[expect(rustdoc::invalid_html_tags)]
/// <h1>
pub fn bar() {}
#[expect(rustdoc::bare_urls)]
/// http://example.org
pub fn baz() {}
}
pub mod rustdoc_warn {
//! See <https://doc.rust-lang.org/rustdoc/lints.html>
#[expect(rustdoc::broken_intra_doc_links)]
/// I want to link to [`bar`] but it doesn't exist!
pub fn foo() {}
#[expect(rustdoc::invalid_html_tags)]
/// <h1></h1>
pub fn bar() {}
#[expect(rustdoc::bare_urls)]
/// <http://example.org>
pub fn baz() {}
}
mod clippy_ok {
//! See <https://rust-lang.github.io/rust-clippy/master/index.html>
#[expect(clippy::almost_swapped)]
fn foo() {
let mut a = 0;
let mut b = 9;
a = b;
b = a;
}
#[expect(clippy::bytes_nth)]
fn bar() {
let _ = "Hello".bytes().nth(3);
}
#[expect(clippy::if_same_then_else)]
fn baz() {
let _ = if true { 42 } else { 42 };
}
#[expect(clippy::logic_bug)]
fn burger() {
let a = false;
let b = true;
if a && b || a {}
}
}
mod clippy_warn {
//! See <https://rust-lang.github.io/rust-clippy/master/index.html>
#[expect(clippy::almost_swapped)]
fn foo() {
let mut a = 0;
let mut b = 9;
a = b;
}
#[expect(clippy::bytes_nth)]
fn bar() {
let _ = "Hello".as_bytes().get(3);
}
#[expect(clippy::if_same_then_else)]
fn baz() {
let _ = if true { 33 } else { 42 };
}
#[expect(clippy::logic_bug)]
fn burger() {
let a = false;
let b = true;
let c = false;
if a && b || c {}
}
}
fn main() {
rustc_warn::rustc_lints();
}

View file

@ -0,0 +1,40 @@
error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:35:14
|
LL | #[expect(dead_code)]
| ^^^^^^^^^
|
= note: `-D unfulfilled-lint-expectations` implied by `-D warnings`
error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:39:18
|
LL | #[expect(illegal_floating_point_literal_pattern)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:113:14
|
LL | #[expect(clippy::almost_swapped)]
| ^^^^^^^^^^^^^^^^^^^^^^
error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:120:14
|
LL | #[expect(clippy::bytes_nth)]
| ^^^^^^^^^^^^^^^^^
error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:125:14
|
LL | #[expect(clippy::if_same_then_else)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:130:14
|
LL | #[expect(clippy::logic_bug)]
| ^^^^^^^^^^^^^^^^^
error: aborting due to 6 previous errors