Skip to content

Commit

Permalink
Auto merge of #84064 - hyd-dev:unknown-lints, r=petrochenkov
Browse files Browse the repository at this point in the history 
Do not ignore path segments in the middle in `#[allow]`/`#[warn]`/`#[deny]`/`#[forbid]` attributes

Fixes #83477.
  • Loading branch information
@bors
bors committed Apr 18, 2021
2 parents 83ca4b7 + ac044e3 commit dc99219
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 44 deletions.
81 changes: 38 additions & 43 deletions compiler/rustc_lint/src/levels.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,9 @@ impl<'s> LintLevelsBuilder<'s> {
Some(lvl) => lvl,
};

let meta = unwrap_or!(attr.meta(), continue);
self.sess.mark_attr_used(attr);

let mut metas = unwrap_or!(meta.meta_item_list(), continue);
let mut metas = unwrap_or!(attr.meta_item_list(), continue);

if metas.is_empty() {
// FIXME (#55112): issue unused-attributes lint for `#[level()]`
Expand All @@ -255,8 +254,6 @@ impl<'s> LintLevelsBuilder<'s> {
ast::MetaItemKind::Word => {} // actual lint names handled later
ast::MetaItemKind::NameValue(ref name_value) => {
if item.path == sym::reason {
// found reason, reslice meta list to exclude it
metas = &metas[0..metas.len() - 1];
// FIXME (#55112): issue unused-attributes lint if we thereby
// don't have any lint names (`#[level(reason = "foo")]`)
if let ast::LitKind::Str(rationale, _) = name_value.kind {
Expand All @@ -275,6 +272,8 @@ impl<'s> LintLevelsBuilder<'s> {
.span_label(name_value.span, "reason must be a string literal")
.emit();
}
// found reason, reslice meta list to exclude it
metas.pop().unwrap();
} else {
bad_attr(item.span)
.span_label(item.span, "bad attribute argument")
Expand All @@ -288,10 +287,10 @@ impl<'s> LintLevelsBuilder<'s> {
}

for li in metas {
let meta_item = match li.meta_item() {
Some(meta_item) if meta_item.is_word() => meta_item,
let sp = li.span();
let mut meta_item = match li {
ast::NestedMetaItem::MetaItem(meta_item) if meta_item.is_word() => meta_item,
_ => {
let sp = li.span();
let mut err = bad_attr(sp);
let mut add_label = true;
if let Some(item) = li.meta_item() {
Expand Down Expand Up @@ -330,15 +329,19 @@ impl<'s> LintLevelsBuilder<'s> {
continue;
}

Some(tool_ident.name)
Some(meta_item.path.segments.remove(0).ident.name)
} else {
None
};
let name = meta_item.path.segments.last().expect("empty lint name").ident.name;
let lint_result = store.check_lint_name(&name.as_str(), tool_name);
let name = pprust::path_to_string(&meta_item.path);
let lint_result = store.check_lint_name(&name, tool_name);
match &lint_result {
CheckLintNameResult::Ok(ids) => {
let src = LintLevelSource::Node(name, li.span(), reason);
let src = LintLevelSource::Node(
meta_item.path.segments.last().expect("empty lint name").ident.name,
sp,
reason,
);
for &id in *ids {
self.check_gated_lint(id, attr.span);
self.insert_spec(&mut specs, id, (level, src));
Expand All @@ -351,7 +354,7 @@ impl<'s> LintLevelsBuilder<'s> {
let complete_name = &format!("{}::{}", tool_name.unwrap(), name);
let src = LintLevelSource::Node(
Symbol::intern(complete_name),
li.span(),
sp,
reason,
);
for id in ids {
Expand All @@ -367,7 +370,7 @@ impl<'s> LintLevelsBuilder<'s> {
lint,
lvl,
src,
Some(li.span().into()),
Some(sp.into()),
|lint| {
let msg = format!(
"lint name `{}` is deprecated \
Expand All @@ -376,7 +379,7 @@ impl<'s> LintLevelsBuilder<'s> {
);
lint.build(&msg)
.span_suggestion(
li.span(),
sp,
"change it to",
new_lint_name.to_string(),
Applicability::MachineApplicable,
Expand All @@ -387,7 +390,7 @@ impl<'s> LintLevelsBuilder<'s> {

let src = LintLevelSource::Node(
Symbol::intern(&new_lint_name),
li.span(),
sp,
reason,
);
for id in ids {
Expand All @@ -414,12 +417,12 @@ impl<'s> LintLevelsBuilder<'s> {
lint,
renamed_lint_level,
src,
Some(li.span().into()),
Some(sp.into()),
|lint| {
let mut err = lint.build(&msg);
if let Some(new_name) = &renamed {
err.span_suggestion(
li.span(),
sp,
"use the new name",
new_name.to_string(),
Applicability::MachineApplicable,
Expand All @@ -433,30 +436,23 @@ impl<'s> LintLevelsBuilder<'s> {
let lint = builtin::UNKNOWN_LINTS;
let (level, src) =
self.sets.get_lint_level(lint, self.cur, Some(&specs), self.sess);
struct_lint_level(
self.sess,
lint,
level,
src,
Some(li.span().into()),
|lint| {
let name = if let Some(tool_name) = tool_name {
format!("{}::{}", tool_name, name)
} else {
name.to_string()
};
let mut db = lint.build(&format!("unknown lint: `{}`", name));
if let Some(suggestion) = suggestion {
db.span_suggestion(
li.span(),
"did you mean",
suggestion.to_string(),
Applicability::MachineApplicable,
);
}
db.emit();
},
);
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)
} else {
name.to_string()
};
let mut db = lint.build(&format!("unknown lint: `{}`", name));
if let Some(suggestion) = suggestion {
db.span_suggestion(
sp,
"did you mean",
suggestion.to_string(),
Applicability::MachineApplicable,
);
}
db.emit();
});
}
}
// If this lint was renamed, apply the new lint instead of ignoring the attribute.
Expand All @@ -466,8 +462,7 @@ impl<'s> LintLevelsBuilder<'s> {
// 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) {
let src =
LintLevelSource::Node(Symbol::intern(&new_name), li.span(), reason);
let src = LintLevelSource::Node(Symbol::intern(&new_name), sp, reason);
for &id in ids {
self.check_gated_lint(id, attr.span);
self.insert_spec(&mut specs, id, (level, src));
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/lint/issue-83477.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// check-pass
#![warn(rustc::internal)]

#[allow(rustc::foo::bar::default_hash_types)]
//~^ WARN unknown lint: `rustc::foo::bar::default_hash_types`
//~| HELP did you mean
//~| SUGGESTION rustc::default_hash_types
#[allow(rustc::foo::default_hash_types)]
//~^ WARN unknown lint: `rustc::foo::default_hash_types`
//~| HELP did you mean
//~| SUGGESTION rustc::default_hash_types
fn main() {
let _ = std::collections::HashMap::<String, String>::new();
//~^ WARN Prefer FxHashMap over HashMap, it has better performance
//~| HELP use
}
30 changes: 30 additions & 0 deletions src/test/ui/lint/issue-83477.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
warning: unknown lint: `rustc::foo::bar::default_hash_types`
--> $DIR/issue-83477.rs:4:9
|
LL | #[allow(rustc::foo::bar::default_hash_types)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `rustc::default_hash_types`
|
= note: `#[warn(unknown_lints)]` on by default

warning: unknown lint: `rustc::foo::default_hash_types`
--> $DIR/issue-83477.rs:8:9
|
LL | #[allow(rustc::foo::default_hash_types)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `rustc::default_hash_types`

warning: Prefer FxHashMap over HashMap, it has better performance
--> $DIR/issue-83477.rs:13:31
|
LL | let _ = std::collections::HashMap::<String, String>::new();
| ^^^^^^^ help: use: `FxHashMap`
|
note: the lint level is defined here
--> $DIR/issue-83477.rs:2:9
|
LL | #![warn(rustc::internal)]
| ^^^^^^^^^^^^^^^
= note: `#[warn(rustc::default_hash_types)]` implied by `#[warn(rustc::internal)]`
= note: a `use rustc_data_structures::fx::FxHashMap` may be necessary

warning: 3 warnings emitted

2 changes: 1 addition & 1 deletion src/tools/clippy/tests/ui/filter_methods.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::all, clippy::pedantic)]
#![allow(clippy::clippy::let_underscore_drop)]
#![allow(clippy::let_underscore_drop)]
#![allow(clippy::missing_docs_in_private_items)]

fn main() {
Expand Down

0 comments on commit dc99219

Please sign in to comment.