From f2fcfe82af65dd7a2fbc20ca082e1a6794918d0e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 13 Mar 2024 11:53:36 +1100 Subject: [PATCH] Various style improvements to `rustc_lint::levels` - 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 --- compiler/rustc_lint/src/levels.rs | 249 +++++++++++++++--------------- 1 file changed, 121 insertions(+), 128 deletions(-) diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index e89df1c9840c6..74b6c92dbfed7 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -103,11 +103,12 @@ impl LintLevelSets { mut idx: LintStackIndex, aux: Option<&FxIndexMap>, ) -> (Option, 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 @@ 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; } } @@ -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 @@ -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; } }; @@ -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 } }); @@ -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), + ); } } } @@ -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.