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.

Sign up for GitHub

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

Jump to bottom

Do not ignore path segments in the middle in #[allow]/#[warn]/#[deny]/#[forbid] attributes #84064

Merged
merged 1 commit into from Apr 18, 2021
Merged

Do not ignore path segments in the middle in #[allow]/#[warn]/#[deny]/#[forbid] attributes #84064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
81 changes: 38 additions & 43 deletions compiler/rustc_lint/src/levels.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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