Skip to content

Commit

Permalink
in which lint reasons are restricted to come last in the attribute
Browse files Browse the repository at this point in the history
Vadim Petrochenkov suggested this in review ("an error? just to be
conservative"), and it turns out to be convenient from the
implementer's perspective: in the initial proposed implementation (or
`HEAD~2`, as some might prefer to call it), we were doing an entire
whole iteration over the meta items just to find the reason (before
iterating over them to set the actual lint levels). This way, we can
just peek at the end rather than adding that extra loop (or
restructuring the existing code). The RFC doesn't seem to take a
position on this, and there's some precedent for restricting things to
be at the end of a sequence (we only allow `..` at the end of a struct
pattern, even if it would be possible to let it appear anywhere in the
sequence).
  • Loading branch information
zackmdavis committed Oct 27, 2018
1 parent dc0609c commit f90de11
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 36 deletions.
73 changes: 42 additions & 31 deletions src/librustc/lint/levels.rs
Expand Up @@ -21,6 +21,7 @@ use rustc_data_structures::stable_hasher::{HashStable, ToStableHashKey,
use session::Session;
use syntax::ast;
use syntax::attr;
use syntax::feature_gate;
use syntax::source_map::MultiSpan;
use syntax::symbol::Symbol;
use util::nodemap::FxHashMap;
Expand Down Expand Up @@ -210,60 +211,70 @@ impl<'a> LintLevelsBuilder<'a> {
let meta = unwrap_or!(attr.meta(), continue);
attr::mark_used(attr);

let metas = if let Some(metas) = meta.meta_item_list() {
let mut metas = if let Some(metas) = meta.meta_item_list() {
metas
} else {
let mut err = bad_attr(meta.span);
err.emit();
continue
};

// Before processing the lint names, look for a reason (RFC 2383).
// Before processing the lint names, look for a reason (RFC 2383)
// at the end.
let mut reason = None;
for li in metas {
if let Some(item) = li.meta_item() {
match item.node {
ast::MetaItemKind::Word => {} // actual lint names handled later
ast::MetaItemKind::NameValue(ref name_value) => {
let gate_reasons = !self.sess.features_untracked().lint_reasons;
let name_ident = item.ident.segments[0].ident;
let name = name_ident.name.as_str();

if name == "reason" {
if let ast::LitKind::Str(rationale, _) = name_value.node {
if gate_reasons {
feature_gate::emit_feature_err(
&self.sess.parse_sess,
"lint_reasons",
item.span,
feature_gate::GateIssue::Language,
"lint reasons are experimental"
);
} else {
reason = Some(rationale);
}
let tail_li = &metas[metas.len()-1];
if let Some(item) = tail_li.meta_item() {
match item.node {
ast::MetaItemKind::Word => {} // actual lint names handled later
ast::MetaItemKind::NameValue(ref name_value) => {
let gate_reasons = !self.sess.features_untracked().lint_reasons;
if item.ident == "reason" {
// found reason, reslice meta list to exclude it
metas = &metas[0..metas.len()-1];
if let ast::LitKind::Str(rationale, _) = name_value.node {
if gate_reasons {
feature_gate::emit_feature_err(
&self.sess.parse_sess,
"lint_reasons",
item.span,
feature_gate::GateIssue::Language,
"lint reasons are experimental"
);
} else {
let mut err = bad_attr(name_value.span);
err.help("reason must be a string literal");
err.emit();
reason = Some(rationale);
}
} else {
let mut err = bad_attr(item.span);
let mut err = bad_attr(name_value.span);
err.help("reason must be a string literal");
err.emit();
}
},
ast::MetaItemKind::List(_) => {
} else {
let mut err = bad_attr(item.span);
err.emit();
}
},
ast::MetaItemKind::List(_) => {
let mut err = bad_attr(item.span);
err.emit();
}
}
}

for li in metas {
let word = match li.word() {
Some(word) => word,
None => { continue; }
None => {
let mut err = bad_attr(li.span);
if let Some(item) = li.meta_item() {
if let ast::MetaItemKind::NameValue(_) = item.node {
if item.ident == "reason" {
err.help("reason in lint attribute must come last");
}
}
}
err.emit();
continue;
}
};
let tool_name = if let Some(lint_tool) = word.is_scoped() {
if !attr::is_known_lint_tool(lint_tool) {
Expand Down
8 changes: 7 additions & 1 deletion src/test/ui/lint/reasons-erroneous.rs
Expand Up @@ -12,7 +12,13 @@
//~^ ERROR malformed lint attribute
#![warn(elided_lifetimes_in_paths, reason("disrespectful to ancestors", "irresponsible to heirs"))]
//~^ ERROR malformed lint attribute
#![warn(ellipsis_inclusive_range_patterns, reason)]
#![warn(ellipsis_inclusive_range_patterns, reason = "born barren", reason = "a freak growth")]
//~^ ERROR malformed lint attribute
//~| HELP reason in lint attribute must come last
#![warn(keyword_idents, reason = "root in rubble", macro_use_extern_crate)]
//~^ ERROR malformed lint attribute
//~| HELP reason in lint attribute must come last
#![warn(missing_copy_implementations, reason)]
//~^ WARN unknown lint

fn main() {}
24 changes: 20 additions & 4 deletions src/test/ui/lint/reasons-erroneous.stderr
Expand Up @@ -32,14 +32,30 @@ error[E0452]: malformed lint attribute
LL | #![warn(elided_lifetimes_in_paths, reason("disrespectful to ancestors", "irresponsible to heirs"))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unknown lint: `reason`
error[E0452]: malformed lint attribute
--> $DIR/reasons-erroneous.rs:15:44
|
LL | #![warn(ellipsis_inclusive_range_patterns, reason)]
| ^^^^^^
LL | #![warn(ellipsis_inclusive_range_patterns, reason = "born barren", reason = "a freak growth")]
| ^^^^^^^^^^^^^^^^^^^^^^
|
= help: reason in lint attribute must come last

error[E0452]: malformed lint attribute
--> $DIR/reasons-erroneous.rs:18:25
|
LL | #![warn(keyword_idents, reason = "root in rubble", macro_use_extern_crate)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: reason in lint attribute must come last

warning: unknown lint: `reason`
--> $DIR/reasons-erroneous.rs:21:39
|
LL | #![warn(missing_copy_implementations, reason)]
| ^^^^^^
|
= note: #[warn(unknown_lints)] on by default

error: aborting due to 5 previous errors
error: aborting due to 7 previous errors

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

0 comments on commit f90de11

Please sign in to comment.