Skip to content

Commit

Permalink
Handle #[expect(unfulfilled_lint_expectations)] with a lint message
Browse files Browse the repository at this point in the history
  • Loading branch information
xFrednet committed Mar 7, 2022
1 parent 165b558 commit d39d609
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 14 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,7 @@ impl Handler {

/// 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<LintExpectationId> {
assert!(
self.inner.borrow().unstable_expect_diagnostics.is_empty(),
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_lint/src/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
},
);
Expand Down
34 changes: 26 additions & 8 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::feature_err;
Expand Down Expand Up @@ -212,6 +212,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 {
specs.insert(id, (old_level, old_src));
} else {
Expand Down Expand Up @@ -344,6 +352,20 @@ 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 {
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,
Expand All @@ -353,10 +375,6 @@ impl<'s> LintLevelsBuilder<'s> {
self.check_gated_lint(id, attr.span);
self.insert_spec(&mut specs, id, (level, src));
}
if let Level::Expect(expect_id) = level {
self.lint_expectations
.push((expect_id, LintExpectation::new(reason, sp)));
}
}

CheckLintNameResult::Tool(result) => {
Expand All @@ -374,7 +392,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)) => {
Expand Down Expand Up @@ -414,7 +432,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, _)) => {
Expand Down Expand Up @@ -511,7 +529,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);
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,19 @@ pub struct LintExpectation {
pub reason: Option<Symbol>,
/// 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<Symbol>, attr_span: Span) -> Self {
Self { reason, emission_span: attr_span }
pub fn new(
reason: Option<Symbol>,
emission_span: Span,
is_unfulfilled_lint_expectations: bool,
) -> Self {
Self { reason, emission_span, is_unfulfilled_lint_expectations }
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit d39d609

Please sign in to comment.