Skip to content

Support lint tool names in rustc command line options #86639

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

Merged
merged 5 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
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 @@ -334,9 +336,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 @@ -358,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 @@ -400,9 +417,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 @@ -1018,3 +1043,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 @@ -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,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 @@ -364,7 +345,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 +401,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 +452,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 @@ -474,7 +476,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 @@ -494,3 +494,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`.