diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2022-03-14 17:24:58 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-03-14 17:24:58 +0100 |
| commit | 6548a368c8c66c802ba710dd9fede228dc65587e (patch) | |
| tree | e16d7650112a4673530e53023d13536dd8450215 /compiler/rustc_lint/src | |
| parent | 774655da5fabdef01f862c50d1796abbe59efb7d (diff) | |
| parent | be84049570d5be6d6e76e471ef88a11ae46292ad (diff) | |
| download | rust-6548a368c8c66c802ba710dd9fede228dc65587e.tar.gz rust-6548a368c8c66c802ba710dd9fede228dc65587e.zip | |
Rollup merge of #94670 - xFrednet:rfc-2383-expect-impl-after-party, r=flip1995,wesleywiser
Improve `expect` impl and handle `#[expect(unfulfilled_lint_expectations)]` (RFC 2383) This PR updates unstable `ExpectationIds` in stashed diagnostics and adds some asserts to ensure that the stored expectations are really empty in the end. Additionally, it handles the `#[expect(unfulfilled_lint_expectations)]` case. According to the [Errors and lints docs](https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-levels) the `error` level should only be used _"when the compiler detects a problem that makes it unable to compile the program"_. As this isn't the case with `#[expect(unfulfilled_lint_expectations)]` I decided to only create a warning. To avoid adding a new lint only for this case, I simply emit a `unfulfilled_lint_expectations` diagnostic with an additional note. --- r? `@wesleywiser` I'm requesting a review from you since you reviewed the previous PR https://github.com/rust-lang/rust/pull/87835. You are welcome to reassign it if you're busy :upside_down_face: rfc: [RFC-2383](https://rust-lang.github.io/rfcs/2383-lint-reasons.html) tracking issue: https://github.com/rust-lang/rust/issues/85549 cc: `@flip1995` In case you're also interested in this :)
Diffstat (limited to 'compiler/rustc_lint/src')
| -rw-r--r-- | compiler/rustc_lint/src/expect.rs | 9 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/levels.rs | 36 |
2 files changed, 33 insertions, 12 deletions
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); |
