From 8b4f538320cbb40643bfb94f28313d4137126b01 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Tue, 6 Jul 2021 17:19:20 -0700 Subject: [PATCH] Unify lint tool and lint name checking This shares a little more code between checking command line and attribute lint specifications. --- compiler/rustc_lint/src/context.rs | 44 ++++++++++--- compiler/rustc_lint/src/levels.rs | 61 ++++++++++--------- ...and-line-register-unknown-lint-tool.stderr | 8 +-- 3 files changed, 73 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 76ac4cfdf84..263ed83ed5a 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -16,7 +16,7 @@ use self::TargetLint::*; -use crate::levels::LintLevelsBuilder; +use crate::levels::{is_known_lint_tool, LintLevelsBuilder}; use crate::passes::{EarlyLintPassObject, LateLintPassObject}; use rustc_ast as ast; use rustc_data_structures::fx::FxHashMap; @@ -129,6 +129,8 @@ pub enum CheckLintNameResult<'a> { Ok(&'a [LintId]), /// Lint doesn't exist. Potentially contains a suggestion for a correct lint name. NoLint(Option), + /// The lint refers to a tool that has not been registered. + NoTool, /// The lint is either renamed or removed. This is the warning /// message, and an optional new name (`None` if removed). Warning(String, Option), @@ -334,16 +336,17 @@ impl LintStore { } } - /// Checks the validity of lint names derived from the command line. Returns - /// true if the lint is valid, false otherwise. + /// Checks the validity of lint names derived from the command line. pub fn check_lint_name_cmdline( &self, sess: &Session, lint_name: &str, - level: Option, - ) -> bool { + level: Level, + crate_attrs: &[ast::Attribute], + ) { let (tool_name, lint_name) = parse_lint_and_tool_name(lint_name); - let db = match self.check_lint_name(lint_name, tool_name) { + + let db = match self.check_lint_and_tool_name(sess, tool_name, lint_name, crate_attrs) { CheckLintNameResult::Ok(_) => None, CheckLintNameResult::Warning(ref msg, _) => Some(sess.struct_warn(msg)), CheckLintNameResult::NoLint(suggestion) => { @@ -365,6 +368,13 @@ impl LintStore { ))), _ => None, }, + CheckLintNameResult::NoTool => Some(struct_span_err!( + sess, + DUMMY_SP, + E0602, + "unknown lint tool: `{}`", + tool_name.unwrap() + )), }; if let Some(mut db) = db { @@ -398,6 +408,22 @@ impl LintStore { } } + pub fn check_lint_and_tool_name( + &self, + sess: &Session, + tool_name: Option, + lint_name: &str, + crate_attrs: &[ast::Attribute], + ) -> CheckLintNameResult<'_> { + if let Some(tool_name) = tool_name { + if !is_known_lint_tool(tool_name, sess, crate_attrs) { + return CheckLintNameResult::NoTool; + } + } + + self.check_lint_name(lint_name, tool_name) + } + /// 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 @@ -1028,7 +1054,11 @@ impl<'tcx> LayoutOf for LateContext<'tcx> { pub fn parse_lint_and_tool_name(lint_name: &str) -> (Option, &str) { match lint_name.split_once("::") { - Some((tool_name, lint_name)) => (Some(Symbol::intern(tool_name)), lint_name), + Some((tool_name, lint_name)) => { + let tool_name = Symbol::intern(tool_name); + + (Some(tool_name), lint_name) + } None => (None, lint_name), } } diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index c0a059b92aa..30ee8c9b6ae 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -88,7 +88,7 @@ impl<'s> LintLevelsBuilder<'s> { self.sets.lint_cap = sess.opts.lint_cap.unwrap_or(Level::Forbid); for &(ref lint_name, level) in &sess.opts.lint_opts { - store.check_lint_name_cmdline(sess, &lint_name, level); + store.check_lint_name_cmdline(sess, &lint_name, level, self.crate_attrs); let orig_level = level; // If the cap is less than this specified level, e.g., if we've got @@ -110,7 +110,7 @@ impl<'s> LintLevelsBuilder<'s> { } for lint_name in &sess.opts.force_warns { - store.check_lint_name_cmdline(sess, lint_name, Level::ForceWarn); + store.check_lint_name_cmdline(sess, lint_name, Level::ForceWarn, self.crate_attrs); let lints = store .find_lints(lint_name) .unwrap_or_else(|_| bug!("A valid lint failed to produce a lint ids")); @@ -321,33 +321,15 @@ impl<'s> LintLevelsBuilder<'s> { continue; } }; - let tool_name = if meta_item.path.segments.len() > 1 { - let tool_ident = meta_item.path.segments[0].ident; - if !is_known_lint_tool(tool_ident.name, sess, &self.crate_attrs) { - let mut err = struct_span_err!( - sess, - tool_ident.span, - E0710, - "unknown tool name `{}` found in scoped lint: `{}`", - tool_ident.name, - pprust::path_to_string(&meta_item.path), - ); - if sess.is_nightly_build() { - err.help(&format!( - "add `#![register_tool({})]` to the crate root", - tool_ident.name - )); - } - err.emit(); - continue; - } - - Some(meta_item.path.segments.remove(0).ident.name) + let tool_ident = if meta_item.path.segments.len() > 1 { + Some(meta_item.path.segments.remove(0).ident) } else { None }; + let tool_name = tool_ident.map(|ident| ident.name); let name = pprust::path_to_string(&meta_item.path); - let lint_result = store.check_lint_name(&name, tool_name); + let lint_result = + store.check_lint_and_tool_name(sess, tool_name, &name, self.crate_attrs); match &lint_result { CheckLintNameResult::Ok(ids) => { let src = LintLevelSource::Node( @@ -364,7 +346,8 @@ impl<'s> LintLevelsBuilder<'s> { CheckLintNameResult::Tool(result) => { match *result { Ok(ids) => { - let complete_name = &format!("{}::{}", tool_name.unwrap(), name); + let complete_name = + &format!("{}::{}", tool_ident.unwrap().name, name); let src = LintLevelSource::Node( Symbol::intern(complete_name), sp, @@ -419,6 +402,26 @@ impl<'s> LintLevelsBuilder<'s> { } } + &CheckLintNameResult::NoTool => { + let mut err = struct_span_err!( + sess, + tool_ident.map_or(DUMMY_SP, |ident| ident.span), + E0710, + "unknown tool name `{}` found in scoped lint: `{}::{}`", + tool_name.unwrap(), + tool_name.unwrap(), + pprust::path_to_string(&meta_item.path), + ); + if sess.is_nightly_build() { + err.help(&format!( + "add `#![register_tool({})]` to the crate root", + tool_name.unwrap() + )); + } + err.emit(); + continue; + } + _ if !self.warn_about_weird_lints => {} CheckLintNameResult::Warning(msg, renamed) => { @@ -450,8 +453,8 @@ impl<'s> LintLevelsBuilder<'s> { let (level, src) = self.sets.get_lint_level(lint, self.cur, Some(&specs), self.sess); struct_lint_level(self.sess, lint, level, src, Some(sp.into()), |lint| { - let name = if let Some(tool_name) = tool_name { - format!("{}::{}", tool_name, name) + let name = if let Some(tool_ident) = tool_ident { + format!("{}::{}", tool_ident.name, name) } else { name.to_string() }; @@ -578,7 +581,7 @@ impl<'s> LintLevelsBuilder<'s> { } } -fn is_known_lint_tool(m_item: Symbol, sess: &Session, attrs: &[ast::Attribute]) -> bool { +pub fn is_known_lint_tool(m_item: Symbol, sess: &Session, attrs: &[ast::Attribute]) -> bool { if [sym::clippy, sym::rustc, sym::rustdoc].contains(&m_item) { return true; } diff --git a/src/test/ui/lint/command-line-register-unknown-lint-tool.stderr b/src/test/ui/lint/command-line-register-unknown-lint-tool.stderr index 9e0177c47da..b7c5893a0ea 100644 --- a/src/test/ui/lint/command-line-register-unknown-lint-tool.stderr +++ b/src/test/ui/lint/command-line-register-unknown-lint-tool.stderr @@ -1,10 +1,10 @@ -error[E0602]: unknown lint: `unknown_tool::foo` +error[E0602]: unknown lint tool: `unknown_tool` | - = note: requested on the command line with `-A unknown_tool::foo` + = note: requested on the command line with `-A foo` -error[E0602]: unknown lint: `unknown_tool::foo` +error[E0602]: unknown lint tool: `unknown_tool` | - = note: requested on the command line with `-A unknown_tool::foo` + = note: requested on the command line with `-A foo` error: aborting due to 2 previous errors