diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 39ebd57b4b2..5c36c3c55b5 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -5,7 +5,8 @@ use crate::Substitution; use crate::SubstitutionPart; use crate::SuggestionStyle; use crate::ToolMetadata; -use rustc_lint_defs::Applicability; +use rustc_data_structures::stable_map::FxHashMap; +use rustc_lint_defs::{Applicability, LintExpectationId}; use rustc_serialize::json::Json; use rustc_span::edition::LATEST_STABLE_EDITION; use rustc_span::{MultiSpan, Span, DUMMY_SP}; @@ -138,6 +139,28 @@ impl Diagnostic { } } + pub fn update_unstable_expectation_id( + &mut self, + unstable_to_stable: &FxHashMap, + ) { + if let Level::Expect(expectation_id) = &mut self.level { + if expectation_id.is_stable() { + return; + } + + // The unstable to stable map only maps the unstable `AttrId` to a stable `HirId` with an attribute index. + // The lint index inside the attribute is manually transferred here. + let lint_index = expectation_id.get_lint_index(); + expectation_id.set_lint_index(None); + let mut stable_id = *unstable_to_stable + .get(&expectation_id) + .expect("each unstable `LintExpectationId` must have a matching stable id"); + + stable_id.set_lint_index(lint_index); + *expectation_id = stable_id; + } + } + pub fn has_future_breakage(&self) -> bool { match self.code { Some(DiagnosticId::Lint { has_future_breakage, .. }) => has_future_breakage, diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 3641c38f9dc..345247b0700 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -522,6 +522,11 @@ impl Drop for HandlerInner { "no warnings or errors encountered even though `delayed_good_path_bugs` issued", ); } + + assert!( + self.unstable_expect_diagnostics.is_empty(), + "all diagnostics with unstable expectations should have been converted", + ); } } @@ -942,29 +947,30 @@ impl Handler { let mut inner = self.inner.borrow_mut(); for mut diag in diags.into_iter() { - let mut unstable_id = diag + diag.update_unstable_expectation_id(unstable_to_stable); + + let stable_id = diag .level .get_expectation_id() .expect("all diagnostics inside `unstable_expect_diagnostics` must have a `LintExpectationId`"); - - // The unstable to stable map only maps the unstable `AttrId` to a stable `HirId` with an attribute index. - // The lint index inside the attribute is manually transferred here. - let lint_index = unstable_id.get_lint_index(); - unstable_id.set_lint_index(None); - let mut stable_id = *unstable_to_stable - .get(&unstable_id) - .expect("each unstable `LintExpectationId` must have a matching stable id"); - - stable_id.set_lint_index(lint_index); - diag.level = Level::Expect(stable_id); inner.fulfilled_expectations.insert(stable_id); (*TRACK_DIAGNOSTICS)(&diag); } + + inner + .stashed_diagnostics + .values_mut() + .for_each(|diag| diag.update_unstable_expectation_id(unstable_to_stable)); + inner + .future_breakage_diagnostics + .iter_mut() + .for_each(|diag| diag.update_unstable_expectation_id(unstable_to_stable)); } /// This methods steals all [`LintExpectationId`]s that are stored inside /// [`HandlerInner`] and indicate that the linked expectation has been fulfilled. + #[must_use] pub fn steal_fulfilled_expectation_ids(&self) -> FxHashSet { assert!( self.inner.borrow().unstable_expect_diagnostics.is_empty(), diff --git a/compiler/rustc_lint/src/expect.rs b/compiler/rustc_lint/src/expect.rs index e6c9d0b0ab0..74fef0be9e9 100644 --- a/compiler/rustc_lint/src/expect.rs +++ b/compiler/rustc_lint/src/expect.rs @@ -30,10 +30,6 @@ fn emit_unfulfilled_expectation_lint( hir_id: HirId, expectation: &LintExpectation, ) { - // FIXME: The current implementation doesn't cover cases where the - // `unfulfilled_lint_expectations` is actually expected by another lint - // expectation. This can be added here by checking the lint level and - // retrieving the `LintExpectationId` if it was expected. tcx.struct_span_lint_hir( builtin::UNFULFILLED_LINT_EXPECTATIONS, hir_id, @@ -43,6 +39,11 @@ fn emit_unfulfilled_expectation_lint( if let Some(rationale) = expectation.reason { diag.note(&rationale.as_str()); } + + if expectation.is_unfulfilled_lint_expectations { + diag.note("the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message"); + } + diag.emit(); }, ); diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index 7b018e7f75f..47899f8625d 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -14,7 +14,7 @@ use rustc_middle::lint::{ use rustc_middle::ty::query::Providers; use rustc_middle::ty::{RegisteredTools, TyCtxt}; use rustc_session::lint::{ - builtin::{self, FORBIDDEN_LINT_GROUPS}, + builtin::{self, FORBIDDEN_LINT_GROUPS, UNFULFILLED_LINT_EXPECTATIONS}, Level, Lint, LintExpectationId, LintId, }; use rustc_session::parse::{add_feature_diagnostics, feature_err}; @@ -218,6 +218,14 @@ impl<'s> LintLevelsBuilder<'s> { } } } + + // The lint `unfulfilled_lint_expectations` can't be expected, as it would suppress itself. + // Handling expectations of this lint would add additional complexity with little to no + // benefit. The expect level for this lint will therefore be ignored. + if let Level::Expect(_) = level && id == LintId::of(UNFULFILLED_LINT_EXPECTATIONS) { + return; + } + if let Level::ForceWarn = old_level { self.current_specs_mut().insert(id, (old_level, old_src)); } else { @@ -350,6 +358,22 @@ impl<'s> LintLevelsBuilder<'s> { self.store.check_lint_name(&name, tool_name, self.registered_tools); match &lint_result { CheckLintNameResult::Ok(ids) => { + // This checks for instances where the user writes `#[expect(unfulfilled_lint_expectations)]` + // in that case we want to avoid overriding the lint level but instead add an expectation that + // can't be fulfilled. The lint message will include an explanation, that the + // `unfulfilled_lint_expectations` lint can't be expected. + if let Level::Expect(expect_id) = level { + // The `unfulfilled_lint_expectations` lint is not part of any lint groups. Therefore. we + // only need to check the slice if it contains a single lint. + let is_unfulfilled_lint_expectations = match ids { + [lint] => *lint == LintId::of(UNFULFILLED_LINT_EXPECTATIONS), + _ => false, + }; + self.lint_expectations.push(( + expect_id, + LintExpectation::new(reason, sp, is_unfulfilled_lint_expectations), + )); + } let src = LintLevelSource::Node( meta_item.path.segments.last().expect("empty lint name").ident.name, sp, @@ -360,10 +384,6 @@ 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))); - } } CheckLintNameResult::Tool(result) => { @@ -381,7 +401,7 @@ impl<'s> LintLevelsBuilder<'s> { } if let Level::Expect(expect_id) = level { self.lint_expectations - .push((expect_id, LintExpectation::new(reason, sp))); + .push((expect_id, LintExpectation::new(reason, sp, false))); } } Err((Some(ids), ref new_lint_name)) => { @@ -425,7 +445,7 @@ impl<'s> LintLevelsBuilder<'s> { } if let Level::Expect(expect_id) = level { self.lint_expectations - .push((expect_id, LintExpectation::new(reason, sp))); + .push((expect_id, LintExpectation::new(reason, sp, false))); } } Err((None, _)) => { @@ -531,7 +551,7 @@ impl<'s> LintLevelsBuilder<'s> { } if let Level::Expect(expect_id) = level { self.lint_expectations - .push((expect_id, LintExpectation::new(reason, sp))); + .push((expect_id, LintExpectation::new(reason, sp, false))); } } else { panic!("renamed lint does not exist: {}", new_name); diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index cd328b08735..4842f7ef4b9 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -54,7 +54,7 @@ pub enum Applicability { /// Expected `Diagnostic`s get the lint level `Expect` which stores the `LintExpectationId` /// to match it with the actual expectation later on. /// -/// The `LintExpectationId` has to be has stable between compilations, as diagnostic +/// The `LintExpectationId` has to be stable between compilations, as diagnostic /// instances might be loaded from cache. Lint messages can be emitted during an /// `EarlyLintPass` operating on the AST and during a `LateLintPass` traversing the /// HIR tree. The AST doesn't have enough information to create a stable id. The @@ -71,7 +71,7 @@ pub enum Applicability { #[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash, Encodable, Decodable)] pub enum LintExpectationId { /// Used for lints emitted during the `EarlyLintPass`. This id is not - /// has stable and should not be cached. + /// hash stable and should not be cached. Unstable { attr_id: AttrId, lint_index: Option }, /// The [`HirId`] that the lint expectation is attached to. This id is /// stable and can be cached. The additional index ensures that nodes with @@ -113,7 +113,9 @@ impl HashStable for LintExpectationId { lint_index.hash_stable(hcx, hasher); } _ => { - unreachable!("HashStable should only be called for a filled `LintExpectationId`") + unreachable!( + "HashStable should only be called for filled and stable `LintExpectationId`" + ) } } } diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index 894947fa70d..1b301629b9c 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -204,11 +204,19 @@ pub struct LintExpectation { pub reason: Option, /// The [`Span`] of the attribute that this expectation originated from. pub emission_span: Span, + /// Lint messages for the `unfulfilled_lint_expectations` lint will be + /// adjusted to include an additional note. Therefore, we have to track if + /// the expectation is for the lint. + pub is_unfulfilled_lint_expectations: bool, } impl LintExpectation { - pub fn new(reason: Option, attr_span: Span) -> Self { - Self { reason, emission_span: attr_span } + pub fn new( + reason: Option, + emission_span: Span, + is_unfulfilled_lint_expectations: bool, + ) -> Self { + Self { reason, emission_span, is_unfulfilled_lint_expectations } } } diff --git a/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.rs b/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.rs new file mode 100644 index 00000000000..d38e6553386 --- /dev/null +++ b/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.rs @@ -0,0 +1,39 @@ +// check-pass +// ignore-tidy-linelength + +#![feature(lint_reasons)] +#![warn(unused_mut)] + +#![expect(unfulfilled_lint_expectations, reason = "idk why you would expect this")] +//~^ WARNING this lint expectation is unfulfilled +//~| NOTE `#[warn(unfulfilled_lint_expectations)]` on by default +//~| NOTE idk why you would expect this +//~| NOTE the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + +#[expect(unfulfilled_lint_expectations, reason = "a local: idk why you would expect this")] +//~^ WARNING this lint expectation is unfulfilled +//~| NOTE a local: idk why you would expect this +//~| NOTE the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message +pub fn normal_test_fn() { + #[expect(unused_mut, reason = "this expectation will create a diagnostic with the default lint level")] + //~^ WARNING this lint expectation is unfulfilled + //~| NOTE this expectation will create a diagnostic with the default lint level + let mut v = vec![1, 1, 2, 3, 5]; + v.sort(); + + // Check that lint lists including `unfulfilled_lint_expectations` are also handled correctly + #[expect(unused, unfulfilled_lint_expectations, reason = "the expectation for `unused` should be fulfilled")] + //~^ WARNING this lint expectation is unfulfilled + //~| NOTE the expectation for `unused` should be fulfilled + //~| NOTE the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + let value = "I'm unused"; +} + +#[expect(warnings, reason = "this suppresses all warnings and also suppresses itself. No warning will be issued")] +pub fn expect_warnings() { + // This lint trigger will be suppressed + #[warn(unused_mut)] + let mut v = vec![1, 1, 2, 3, 5]; +} + +fn main() {} diff --git a/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.stderr b/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.stderr new file mode 100644 index 00000000000..9bfee79b03d --- /dev/null +++ b/src/test/ui/lint/rfc-2383-lint-reason/expect_unfulfilled_expectation.stderr @@ -0,0 +1,38 @@ +warning: this lint expectation is unfulfilled + --> $DIR/expect_unfulfilled_expectation.rs:7:11 + | +LL | #![expect(unfulfilled_lint_expectations, reason = "idk why you would expect this")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unfulfilled_lint_expectations)]` on by default + = note: idk why you would expect this + = note: the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + +warning: this lint expectation is unfulfilled + --> $DIR/expect_unfulfilled_expectation.rs:13:10 + | +LL | #[expect(unfulfilled_lint_expectations, reason = "a local: idk why you would expect this")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: a local: idk why you would expect this + = note: the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + +warning: this lint expectation is unfulfilled + --> $DIR/expect_unfulfilled_expectation.rs:18:14 + | +LL | #[expect(unused_mut, reason = "this expectation will create a diagnostic with the default lint level")] + | ^^^^^^^^^^ + | + = note: this expectation will create a diagnostic with the default lint level + +warning: this lint expectation is unfulfilled + --> $DIR/expect_unfulfilled_expectation.rs:25:22 + | +LL | #[expect(unused, unfulfilled_lint_expectations, reason = "the expectation for `unused` should be fulfilled")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the expectation for `unused` should be fulfilled + = note: the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message + +warning: 4 warnings emitted +