Skip to content

Commit

Permalink
Various style improvements to rustc_lint::levels
Browse files Browse the repository at this point in the history 
- 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
- Prepend `tool::` 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
  • Loading branch information
@Zalathar
Zalathar committed Mar 13, 2024
1 parent e61dcc7 commit 2cce211
Showing 1 changed file with 121 additions and 128 deletions.
249 changes: 121 additions & 128 deletions compiler/rustc_lint/src/levels.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,12 @@ impl LintLevelSets {
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) {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -643,63 +644,61 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
//
// 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;
}
}

Expand Down Expand Up @@ -770,15 +769,15 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {

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
Expand Down Expand Up @@ -834,21 +833,16 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
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;
}
};
Expand Down Expand Up @@ -987,11 +981,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
}

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 }
});
Expand All @@ -1005,27 +995,24 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
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),
);
}
}
}
Expand Down Expand Up @@ -1058,38 +1045,44 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
/// 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.
Expand Down

0 comments on commit 2cce211

Please sign in to comment.