Skip to content

Commit

Permalink
Rollup merge of rust-lang#86639 - eholk:lint-tool, r=petrochenkov
Browse files Browse the repository at this point in the history
Support lint tool names in rustc command line options

When rustc is running without a lint tool such as clippy enabled, options for lints such as `clippy::foo` are meant to be ignored. This was already working for those specified by attrs, such as `#![allow(clippy::foo)]`, but this did not work for command line arguments like `-A clippy::foo`. This PR fixes that issue.

Note that we discovered this issue while discussing rust-lang/cargo#5034.

Fixes rust-lang#86628.
  • Loading branch information
JohnTitor authored Jul 7, 2021
2 parents 91206a7 + 4a83a93 commit 424498c
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 34 deletions.
44 changes: 40 additions & 4 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 @@ -321,9 +323,17 @@ impl LintStore {
}
}

/// Checks the validity of lint names derived from the command line
pub fn check_lint_name_cmdline(&self, sess: &Session, lint_name: &str, level: Level) {
let db = match self.check_lint_name(lint_name, None) {
/// Checks the validity of lint names derived from the command line.
pub fn check_lint_name_cmdline(
&self,
sess: &Session,
lint_name: &str,
level: Level,
crate_attrs: &[ast::Attribute],
) {
let (tool_name, lint_name_only) = parse_lint_and_tool_name(lint_name);

let db = match self.check_lint_name(sess, lint_name_only, tool_name, crate_attrs) {
CheckLintNameResult::Ok(_) => None,
CheckLintNameResult::Warning(ref msg, _) => Some(sess.struct_warn(msg)),
CheckLintNameResult::NoLint(suggestion) => {
Expand All @@ -345,6 +355,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 @@ -387,9 +404,17 @@ impl LintStore {
/// printing duplicate warnings.
pub fn check_lint_name(
&self,
sess: &Session,
lint_name: &str,
tool_name: Option<Symbol>,
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;
}
}

let complete_name = if let Some(tool_name) = tool_name {
format!("{}::{}", tool_name, lint_name)
} else {
Expand Down Expand Up @@ -1005,3 +1030,14 @@ impl<'tcx> LayoutOf for LateContext<'tcx> {
self.tcx.layout_of(self.param_env.and(ty))
}
}

pub fn parse_lint_and_tool_name(lint_name: &str) -> (Option<Symbol>, &str) {
match lint_name.split_once("::") {
Some((tool_name, lint_name)) => {
let tool_name = Symbol::intern(tool_name);

(Some(tool_name), lint_name)
}
None => (None, lint_name),
}
}
64 changes: 34 additions & 30 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,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 @@ -111,7 +111,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 @@ -322,33 +322,14 @@ 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_name(sess, &name, tool_name, self.crate_attrs);
match &lint_result {
CheckLintNameResult::Ok(ids) => {
let src = LintLevelSource::Node(
Expand All @@ -365,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 @@ -420,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 @@ -451,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 All @@ -475,7 +477,9 @@ impl<'s> LintLevelsBuilder<'s> {
if let CheckLintNameResult::Warning(_, Some(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) = store.check_lint_name(&new_name, None) {
if let CheckLintNameResult::Ok(ids) =
store.check_lint_name(sess, &new_name, None, self.crate_attrs)
{
let src = LintLevelSource::Node(Symbol::intern(&new_name), sp, reason);
for &id in ids {
self.check_gated_lint(id, attr.span);
Expand Down Expand Up @@ -578,7 +582,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
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,3 +497,6 @@ fn register_internals(store: &mut LintStore) {
],
);
}

#[cfg(test)]
mod tests;
24 changes: 24 additions & 0 deletions compiler/rustc_lint/src/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
use crate::context::parse_lint_and_tool_name;
use rustc_span::{with_default_session_globals, Symbol};

#[test]
fn parse_lint_no_tool() {
with_default_session_globals(|| assert_eq!(parse_lint_and_tool_name("foo"), (None, "foo")));
}

#[test]
fn parse_lint_with_tool() {
with_default_session_globals(|| {
assert_eq!(parse_lint_and_tool_name("clippy::foo"), (Some(Symbol::intern("clippy")), "foo"))
});
}

#[test]
fn parse_lint_multiple_path() {
with_default_session_globals(|| {
assert_eq!(
parse_lint_and_tool_name("clippy::foo::bar"),
(Some(Symbol::intern("clippy")), "foo::bar")
)
});
}
7 changes: 7 additions & 0 deletions src/test/ui/lint/command-line-register-lint-tool.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// compile-flags: -A known_tool::foo
// check-pass

#![feature(register_tool)]
#![register_tool(known_tool)]

fn main() {}
4 changes: 4 additions & 0 deletions src/test/ui/lint/command-line-register-unknown-lint-tool.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// compile-flags: -A unknown_tool::foo
// error-pattern: unknown lint tool: `unknown_tool`

fn main() {}
11 changes: 11 additions & 0 deletions src/test/ui/lint/command-line-register-unknown-lint-tool.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0602]: unknown lint tool: `unknown_tool`
|
= note: requested on the command line with `-A unknown_tool::foo`

error[E0602]: unknown lint tool: `unknown_tool`
|
= note: requested on the command line with `-A unknown_tool::foo`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0602`.

0 comments on commit 424498c

Please sign in to comment.