Rollup merge of #122416 - Zalathar:levels, r=petrochenkov
Various style improvements to `rustc_lint::levels`
While reading this file, I noticed a few opportunities to make things a little nicer:
- Replace some nested if-let with let-chains
- Tweak a match pattern to allow shorthand struct syntax
- Fuse an `is_empty` check with getting the last element
- Merge some common code that emits `MalformedAttribute` and continues
- Format `"{tool}::{name}"` in a way that's consistent with other match arms
- Replace if-let-else-panic with let-else
- Use early-exit to flatten a method body
Some of these changes cause indentation churn, so ignoring whitespace is recommended.
This commit is contained in:
Matthias Krüger
GitHub
commit
0286591d59
@ -103,11 +103,12 @@ fn raw_lint_id_level(
|
||||
mut idx: LintStackIndex,
|
||||
aux: Option<&FxIndexMap<LintId, LevelAndSource>>,
|
||||
) -> (Option<Level>, LintLevelSource) {
|
||||
if let Some(specs) = aux {
|
||||
if let Some(&(level, src)) = specs.get(&id) {
|
||||
return (Some(level), src);
|
||||
}
|
||||
if let Some(specs) = aux
|
||||
&& let Some(&(level, src)) = specs.get(&id)
|
||||
{
|
||||
return (Some(level), src);
|
||||
}
|
||||
|
||||
loop {
|
||||
let LintSet { ref specs, parent } = self.list[idx];
|
||||
if let Some(&(level, src)) = specs.get(&id) {
|
||||
@ -177,7 +178,7 @@ fn shallow_lint_levels_on(tcx: TyCtxt<'_>, owner: hir::OwnerId) -> ShallowLintLe
|
||||
// There is only something to do if there are attributes at all.
|
||||
[] => {}
|
||||
// Most of the time, there is only one attribute. Avoid fetching HIR in that case.
|
||||
[(local_id, _)] => levels.add_id(HirId { owner, local_id: *local_id }),
|
||||
&[(local_id, _)] => levels.add_id(HirId { owner, local_id }),
|
||||
// Otherwise, we need to visit the attributes in source code order, so we fetch HIR and do
|
||||
// a standard visit.
|
||||
// FIXME(#102522) Just iterate on attrs once that iteration order matches HIR's.
|
||||
@ -643,63 +644,61 @@ fn insert_spec(&mut self, id: LintId, (mut level, src): LevelAndSource) {
|
||||
//
|
||||
// This means that this only errors if we're truly lowering the lint
|
||||
// level from forbid.
|
||||
if self.lint_added_lints && level != Level::Forbid {
|
||||
if let Level::Forbid = old_level {
|
||||
// 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 { name, .. } => self.store.is_lint_group(name),
|
||||
LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
|
||||
};
|
||||
debug!(
|
||||
"fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
|
||||
fcw_warning,
|
||||
self.current_specs(),
|
||||
old_src,
|
||||
id_name
|
||||
);
|
||||
let sub = match old_src {
|
||||
LintLevelSource::Default => {
|
||||
OverruledAttributeSub::DefaultSource { id: id.to_string() }
|
||||
}
|
||||
LintLevelSource::Node { span, reason, .. } => {
|
||||
OverruledAttributeSub::NodeSource { span, reason }
|
||||
}
|
||||
LintLevelSource::CommandLine(_, _) => OverruledAttributeSub::CommandLineSource,
|
||||
};
|
||||
if !fcw_warning {
|
||||
self.sess.dcx().emit_err(OverruledAttribute {
|
||||
span: src.span(),
|
||||
if self.lint_added_lints && level != Level::Forbid && old_level == Level::Forbid {
|
||||
// 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 { name, .. } => self.store.is_lint_group(name),
|
||||
LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
|
||||
};
|
||||
debug!(
|
||||
"fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
|
||||
fcw_warning,
|
||||
self.current_specs(),
|
||||
old_src,
|
||||
id_name
|
||||
);
|
||||
let sub = match old_src {
|
||||
LintLevelSource::Default => {
|
||||
OverruledAttributeSub::DefaultSource { id: id.to_string() }
|
||||
}
|
||||
LintLevelSource::Node { span, reason, .. } => {
|
||||
OverruledAttributeSub::NodeSource { span, reason }
|
||||
}
|
||||
LintLevelSource::CommandLine(_, _) => OverruledAttributeSub::CommandLineSource,
|
||||
};
|
||||
if !fcw_warning {
|
||||
self.sess.dcx().emit_err(OverruledAttribute {
|
||||
span: src.span(),
|
||||
overruled: src.span(),
|
||||
lint_level: level.as_str(),
|
||||
lint_source: src.name(),
|
||||
sub,
|
||||
});
|
||||
} else {
|
||||
self.emit_span_lint(
|
||||
FORBIDDEN_LINT_GROUPS,
|
||||
src.span().into(),
|
||||
OverruledAttributeLint {
|
||||
overruled: src.span(),
|
||||
lint_level: level.as_str(),
|
||||
lint_source: src.name(),
|
||||
sub,
|
||||
});
|
||||
} else {
|
||||
self.emit_span_lint(
|
||||
FORBIDDEN_LINT_GROUPS,
|
||||
src.span().into(),
|
||||
OverruledAttributeLint {
|
||||
overruled: src.span(),
|
||||
lint_level: level.as_str(),
|
||||
lint_source: src.name(),
|
||||
sub,
|
||||
},
|
||||
);
|
||||
}
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
// 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;
|
||||
}
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
|
||||
@ -770,15 +769,15 @@ fn add(&mut self, attrs: &[ast::Attribute], is_crate_node: bool, source_hir_id:
|
||||
|
||||
let Some(mut metas) = attr.meta_item_list() else { continue };
|
||||
|
||||
if metas.is_empty() {
|
||||
// Check whether `metas` is empty, and get its last element.
|
||||
let Some(tail_li) = metas.last() else {
|
||||
// This emits the unused_attributes lint for `#[level()]`
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
// Before processing the lint names, look for a reason (RFC 2383)
|
||||
// at the end.
|
||||
let mut reason = None;
|
||||
let tail_li = &metas[metas.len() - 1];
|
||||
if let Some(item) = tail_li.meta_item() {
|
||||
match item.kind {
|
||||
ast::MetaItemKind::Word => {} // actual lint names handled later
|
||||
@ -834,21 +833,16 @@ fn add(&mut self, attrs: &[ast::Attribute], is_crate_node: bool, source_hir_id:
|
||||
let meta_item = match li {
|
||||
ast::NestedMetaItem::MetaItem(meta_item) if meta_item.is_word() => meta_item,
|
||||
_ => {
|
||||
if let Some(item) = li.meta_item() {
|
||||
if let ast::MetaItemKind::NameValue(_) = item.kind {
|
||||
if item.path == sym::reason {
|
||||
sess.dcx().emit_err(MalformedAttribute {
|
||||
span: sp,
|
||||
sub: MalformedAttributeSub::ReasonMustComeLast(sp),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
sess.dcx().emit_err(MalformedAttribute {
|
||||
span: sp,
|
||||
sub: MalformedAttributeSub::BadAttributeArgument(sp),
|
||||
});
|
||||
let sub = if let Some(item) = li.meta_item()
|
||||
&& let ast::MetaItemKind::NameValue(_) = item.kind
|
||||
&& item.path == sym::reason
|
||||
{
|
||||
MalformedAttributeSub::ReasonMustComeLast(sp)
|
||||
} else {
|
||||
MalformedAttributeSub::BadAttributeArgument(sp)
|
||||
};
|
||||
|
||||
sess.dcx().emit_err(MalformedAttribute { span: sp, sub });
|
||||
continue;
|
||||
}
|
||||
};
|
||||
@ -987,11 +981,7 @@ fn add(&mut self, attrs: &[ast::Attribute], is_crate_node: bool, source_hir_id:
|
||||
}
|
||||
|
||||
CheckLintNameResult::NoLint(suggestion) => {
|
||||
let name = if let Some(tool_ident) = tool_ident {
|
||||
format!("{}::{}", tool_ident.name, name)
|
||||
} else {
|
||||
name.to_string()
|
||||
};
|
||||
let name = tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name);
|
||||
let suggestion = suggestion.map(|(replace, from_rustc)| {
|
||||
UnknownLintSuggestion::WithSpan { suggestion: sp, replace, from_rustc }
|
||||
});
|
||||
@ -1005,27 +995,24 @@ fn add(&mut self, attrs: &[ast::Attribute], is_crate_node: bool, source_hir_id:
|
||||
if let CheckLintNameResult::Renamed(new_name) = lint_result {
|
||||
// Ignore any errors or warnings that happen because the new name is inaccurate
|
||||
// NOTE: `new_name` already includes the tool name, so we don't have to add it again.
|
||||
if let CheckLintNameResult::Ok(ids) =
|
||||
let CheckLintNameResult::Ok(ids) =
|
||||
self.store.check_lint_name(&new_name, None, self.registered_tools)
|
||||
{
|
||||
let src = LintLevelSource::Node {
|
||||
name: Symbol::intern(&new_name),
|
||||
span: sp,
|
||||
reason,
|
||||
};
|
||||
for &id in ids {
|
||||
if self.check_gated_lint(id, attr.span, false) {
|
||||
self.insert_spec(id, (level, src));
|
||||
}
|
||||
}
|
||||
if let Level::Expect(expect_id) = level {
|
||||
self.provider.push_expectation(
|
||||
expect_id,
|
||||
LintExpectation::new(reason, sp, false, tool_name),
|
||||
);
|
||||
}
|
||||
} else {
|
||||
else {
|
||||
panic!("renamed lint does not exist: {new_name}");
|
||||
};
|
||||
|
||||
let src =
|
||||
LintLevelSource::Node { name: Symbol::intern(&new_name), span: sp, reason };
|
||||
for &id in ids {
|
||||
if self.check_gated_lint(id, attr.span, false) {
|
||||
self.insert_spec(id, (level, src));
|
||||
}
|
||||
}
|
||||
if let Level::Expect(expect_id) = level {
|
||||
self.provider.push_expectation(
|
||||
expect_id,
|
||||
LintExpectation::new(reason, sp, false, tool_name),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -1058,38 +1045,44 @@ fn add(&mut self, attrs: &[ast::Attribute], is_crate_node: bool, source_hir_id:
|
||||
/// Returns `true` if the lint's feature is enabled.
|
||||
#[track_caller]
|
||||
fn check_gated_lint(&self, lint_id: LintId, span: Span, lint_from_cli: bool) -> bool {
|
||||
if let Some(feature) = lint_id.lint.feature_gate {
|
||||
if !self.features.active(feature) {
|
||||
if self.lint_added_lints {
|
||||
let lint = builtin::UNKNOWN_LINTS;
|
||||
let (level, src) = self.lint_level(builtin::UNKNOWN_LINTS);
|
||||
// FIXME: make this translatable
|
||||
#[allow(rustc::diagnostic_outside_of_impl)]
|
||||
#[allow(rustc::untranslatable_diagnostic)]
|
||||
lint_level(
|
||||
self.sess,
|
||||
let feature = if let Some(feature) = lint_id.lint.feature_gate
|
||||
&& !self.features.active(feature)
|
||||
{
|
||||
// Lint is behind a feature that is not enabled; eventually return false.
|
||||
feature
|
||||
} else {
|
||||
// Lint is ungated or its feature is enabled; exit early.
|
||||
return true;
|
||||
};
|
||||
|
||||
if self.lint_added_lints {
|
||||
let lint = builtin::UNKNOWN_LINTS;
|
||||
let (level, src) = self.lint_level(builtin::UNKNOWN_LINTS);
|
||||
// FIXME: make this translatable
|
||||
#[allow(rustc::diagnostic_outside_of_impl)]
|
||||
#[allow(rustc::untranslatable_diagnostic)]
|
||||
lint_level(
|
||||
self.sess,
|
||||
lint,
|
||||
level,
|
||||
src,
|
||||
Some(span.into()),
|
||||
fluent::lint_unknown_gated_lint,
|
||||
|lint| {
|
||||
lint.arg("name", lint_id.lint.name_lower());
|
||||
lint.note(fluent::lint_note);
|
||||
rustc_session::parse::add_feature_diagnostics_for_issue(
|
||||
lint,
|
||||
level,
|
||||
src,
|
||||
Some(span.into()),
|
||||
fluent::lint_unknown_gated_lint,
|
||||
|lint| {
|
||||
lint.arg("name", lint_id.lint.name_lower());
|
||||
lint.note(fluent::lint_note);
|
||||
rustc_session::parse::add_feature_diagnostics_for_issue(
|
||||
lint,
|
||||
&self.sess,
|
||||
feature,
|
||||
GateIssue::Language,
|
||||
lint_from_cli,
|
||||
);
|
||||
},
|
||||
&self.sess,
|
||||
feature,
|
||||
GateIssue::Language,
|
||||
lint_from_cli,
|
||||
);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
},
|
||||
);
|
||||
}
|
||||
true
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
/// Find the lint level for a lint.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user