Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support lint tool names in rustc command line options #86639

Merged
merged 5 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Unify lint tool and lint name checking
This shares a little more code between checking command line and
attribute lint specifications.
  • Loading branch information
eholk committed Jul 7, 2021
commit 8b4f538320cbb40643bfb94f28313d4137126b01
44 changes: 37 additions & 7 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Symbol>),
/// 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<String>),
Expand Down Expand Up @@ -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<Level>,
) -> 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) => {
Expand All @@ -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 {
Expand Down Expand Up @@ -398,6 +408,22 @@ impl LintStore {
}
}

pub fn check_lint_and_tool_name(
&self,
sess: &Session,
tool_name: Option<Symbol>,
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
Expand Down Expand Up @@ -1028,7 +1054,11 @@ impl<'tcx> LayoutOf for LateContext<'tcx> {

pub fn parse_lint_and_tool_name(lint_name: &str) -> (Option<Symbol>, &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),
}
}
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
61 changes: 32 additions & 29 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"));
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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()
};
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand Down