in which lint reasons are restricted to come last in the attribute

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).

diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs
index ee878da00660efad5165a7ee88999004bc1d3c60..d44facedc8b7c31a85a94972c874d69f1aedc40e 100644 (file)
--- a/src/librustc/lint/levels.rs
+++ b/src/librustc/lint/levels.rs
@@ -21,6 +21,7 @@
 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;
@@ -210,7 +211,7 @@ pub fn push(&mut self, attrs: &[ast::Attribute]) -> BuilderPush {
             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);
@@ -218,44 +219,43 @@ pub fn push(&mut self, attrs: &[ast::Attribute]) -> BuilderPush {
                 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();
                     }
                 }
             }
@@ -263,7 +263,18 @@ pub fn push(&mut self, attrs: &[ast::Attribute]) -> BuilderPush {
             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) {

diff --git a/src/test/ui/lint/reasons-erroneous.rs b/src/test/ui/lint/reasons-erroneous.rs
index 90111a9c4e1e996d6a380139c78d329b22dc59c5..e42b329338b5a2c71d47759b0e7051dd0ef1209d 100644 (file)
--- a/src/test/ui/lint/reasons-erroneous.rs
+++ b/src/test/ui/lint/reasons-erroneous.rs
@@ -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() {}

diff --git a/src/test/ui/lint/reasons-erroneous.stderr b/src/test/ui/lint/reasons-erroneous.stderr
index f66999e92720302e7a7c8929e903f3806a35216e..6842686ecbab581a38343874bb5bdc2ec3c179a2 100644 (file)
--- a/src/test/ui/lint/reasons-erroneous.stderr
+++ b/src/test/ui/lint/reasons-erroneous.stderr
@@ -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`.