bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

introduce future-compatibility warning for forbidden lint groups

Browse source 
We used to ignore `forbid(group)` scenarios completely. This changed
in #78864, but that led to a number of regressions (#80988, #81218).

This PR introduces a future compatibility warning for the case where
a group is forbidden but then an individual lint within that group
is allowed. We now issue a FCW when we see the "allow", but permit
it to take effect.
This commit is contained in:
Niko Matsakis 2021-01-29 19:06:00 -05:00
parent c0b64d97be
commit b6b897b02c
19 changed files with 554 additions and 54 deletions
Download patch file Download diff file Expand all files Collapse all files

15
compiler/rustc_lint/src/context.rs
View file

@ -39,6 +39,7 @@ use rustc_session::SessionLintStore;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::{symbol::Symbol, MultiSpan, Span, DUMMY_SP};
use rustc_target::abi::LayoutOf;
use tracing::debug;
use std::cell::Cell;
use std::slice;
@ -336,6 +337,20 @@ impl LintStore {
}
}
/// True if this symbol represents a lint group name.
pub fn is_lint_group(&self, lint_name: Symbol) -> bool {
debug!(
"is_lint_group(lint_name={:?}, lint_groups={:?})",
lint_name,
self.lint_groups.keys().collect::<Vec<_>>()
);
let lint_name_str = &*lint_name.as_str();
self.lint_groups.contains_key(&lint_name_str) || {
let warnings_name_str = crate::WARNINGS.name_lower();
lint_name_str == &*warnings_name_str
}
}
/// Checks the name of a lint for its existence, and whether it was
/// renamed or removed. Generates a DiagnosticBuilder containing a
/// warning for renamed and removed lints. This is over both lint

101
compiler/rustc_lint/src/levels.rs
View file

@ -5,7 +5,7 @@ use rustc_ast::attr;
use rustc_ast::unwrap_or;
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{struct_span_err, Applicability};
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
use rustc_hir::{intravisit, HirId};
@ -17,11 +17,15 @@ use rustc_middle::lint::{
};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_session::lint::{builtin, Level, Lint, LintId};
use rustc_session::lint::{
builtin::{self, FORBIDDEN_LINT_GROUPS},
Level, Lint, LintId,
};
use rustc_session::parse::feature_err;
use rustc_session::Session;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::{source_map::MultiSpan, Span, DUMMY_SP};
use tracing::debug;
use std::cmp;
@ -51,6 +55,7 @@ pub struct LintLevelsBuilder<'s> {
id_to_set: FxHashMap<HirId, u32>,
cur: u32,
warn_about_weird_lints: bool,
store: &'s LintStore,
}
pub struct BuilderPush {
@ -59,13 +64,14 @@ pub struct BuilderPush {
}
impl<'s> LintLevelsBuilder<'s> {
pub fn new(sess: &'s Session, warn_about_weird_lints: bool, store: &LintStore) -> Self {
pub fn new(sess: &'s Session, warn_about_weird_lints: bool, store: &'s LintStore) -> Self {
let mut builder = LintLevelsBuilder {
sess,
sets: LintLevelSets::new(),
cur: 0,
id_to_set: Default::default(),
warn_about_weird_lints,
store,
};
builder.process_command_line(sess, store);
assert_eq!(builder.sets.list.len(), 1);
@ -120,36 +126,75 @@ impl<'s> LintLevelsBuilder<'s> {
if let (Level::Forbid, old_src) =
self.sets.get_lint_level(id.lint, self.cur, Some(&specs), &self.sess)
{
let mut diag_builder = struct_span_err!(
self.sess,
src.span(),
E0453,
"{}({}) incompatible with previous forbid",
level.as_str(),
src.name(),
// Backwards compatibility check:
//
// We used to not consider `forbid(lint_group)`
// as preventing `allow(lint)` for some lint `lint` in
// `lint_group`. For now, issue a future-compatibility
// warning for this case.
let id_name = id.lint.name_lower();
let fcw_warning = match old_src {
LintLevelSource::Default => false,
LintLevelSource::Node(symbol, _, _) => self.store.is_lint_group(symbol),
LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
};
debug!(
"fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
fcw_warning, specs, old_src, id_name
);
diag_builder.span_label(src.span(), "overruled by previous forbid");
match old_src {
LintLevelSource::Default => {
diag_builder.note(&format!(
"`forbid` lint level is the default for {}",
id.to_string()
));
}
LintLevelSource::Node(_, forbid_source_span, reason) => {
diag_builder.span_label(forbid_source_span, "`forbid` level set here");
if let Some(rationale) = reason {
diag_builder.note(&rationale.as_str());
let decorate_diag_builder = |mut diag_builder: DiagnosticBuilder<'_>| {
diag_builder.span_label(src.span(), "overruled by previous forbid");
match old_src {
LintLevelSource::Default => {
diag_builder.note(&format!(
"`forbid` lint level is the default for {}",
id.to_string()
));
}
LintLevelSource::Node(_, forbid_source_span, reason) => {
diag_builder.span_label(forbid_source_span, "`forbid` level set here");
if let Some(rationale) = reason {
diag_builder.note(&rationale.as_str());
}
}
LintLevelSource::CommandLine(_, _) => {
diag_builder.note("`forbid` lint level was set on command line");
}
}
LintLevelSource::CommandLine(_, _) => {
diag_builder.note("`forbid` lint level was set on command line");
}
diag_builder.emit();
};
if !fcw_warning {
let diag_builder = struct_span_err!(
self.sess,
src.span(),
E0453,
"{}({}) incompatible with previous forbid",
level.as_str(),
src.name(),
);
decorate_diag_builder(diag_builder);
} else {
self.struct_lint(
FORBIDDEN_LINT_GROUPS,
Some(src.span().into()),
|diag_builder| {
let diag_builder = diag_builder.build(&format!(
"{}({}) incompatible with previous forbid",
level.as_str(),
src.name(),
));
decorate_diag_builder(diag_builder);
},
);
}
diag_builder.emit();
// Retain the forbid lint level
return;
// Retain the forbid lint level, unless we are
// issuing a FCW. In the FCW case, we want to
// respect the new setting.
if !fcw_warning {
return;
}
}
}
specs.insert(id, (level, src));