Auto merge of #34676 - aravind-pg:inner-attr, r=brson

Better error message for inner attribute following doc comment

Before it was always just "an inner attribute is not permitted in this context", whereas now we add a special case for when an inner attr follows an outer attr. If the outer attr is a doc comment, then the error is "an inner attr is not permitted following a doc comment", and otherwise it's "an inner attr is not permitted following an outer attribute". In all other cases it's still  "an inner attribute is not permitted in this context".

Note that the public API and behaviour of `parse_attribute` is unchanged. Also, all new names are very open to bikeshedding -- they're arguably clunky.

Fixes #34516. cc @brson

src/libsyntax/parse/attr.rs

@@ -18,23 +18,43 @@ use parse::token;
 use parse::parser::{Parser, TokenType};
 use ptr::P;
 
+#[derive(PartialEq, Eq, Debug)]
+enum InnerAttributeParsePolicy<'a> {
+    Permitted,
+    NotPermitted { reason: &'a str },
+}
+
+const DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG: &'static str = "an inner attribute is not \
+                                                             permitted in this context";
+
 impl<'a> Parser<'a> {
     /// Parse attributes that appear before an item
     pub fn parse_outer_attributes(&mut self) -> PResult<'a, Vec<ast::Attribute>> {
         let mut attrs: Vec<ast::Attribute> = Vec::new();
+        let mut just_parsed_doc_comment = false;
         loop {
             debug!("parse_outer_attributes: self.token={:?}", self.token);
             match self.token {
                 token::Pound => {
-                    attrs.push(self.parse_attribute(false)?);
+                    let inner_error_reason = if just_parsed_doc_comment {
+                        "an inner attribute is not permitted following an outer doc comment"
+                    } else if !attrs.is_empty() {
+                        "an inner attribute is not permitted following an outer attribute"
+                    } else {
+                        DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG
+                    };
+                    let inner_parse_policy =
+                        InnerAttributeParsePolicy::NotPermitted { reason: inner_error_reason };
+                    attrs.push(self.parse_attribute_with_inner_parse_policy(inner_parse_policy)?);
+                    just_parsed_doc_comment = false;
                 }
                 token::DocComment(s) => {
                     let attr = ::attr::mk_sugared_doc_attr(
-                    attr::mk_attr_id(),
-                    self.id_to_interned_str(ast::Ident::with_empty_ctxt(s)),
-                    self.span.lo,
-                    self.span.hi
-                );
+                        attr::mk_attr_id(),
+                        self.id_to_interned_str(ast::Ident::with_empty_ctxt(s)),
+                        self.span.lo,
+                        self.span.hi
+                    );
                     if attr.node.style != ast::AttrStyle::Outer {
                         let mut err = self.fatal("expected outer doc comment");
                         err.note("inner doc comments like this (starting with \
@@ -43,6 +63,7 @@ impl<'a> Parser<'a> {
                     }
                     attrs.push(attr);
                     self.bump();
+                    just_parsed_doc_comment = true;
                 }
                 _ => break,
             }
@@ -55,26 +76,46 @@ impl<'a> Parser<'a> {
     /// If permit_inner is true, then a leading `!` indicates an inner
     /// attribute
     pub fn parse_attribute(&mut self, permit_inner: bool) -> PResult<'a, ast::Attribute> {
-        debug!("parse_attributes: permit_inner={:?} self.token={:?}",
+        debug!("parse_attribute: permit_inner={:?} self.token={:?}",
                permit_inner,
                self.token);
+        let inner_parse_policy = if permit_inner {
+            InnerAttributeParsePolicy::Permitted
+        } else {
+            InnerAttributeParsePolicy::NotPermitted
+                { reason: DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG }
+        };
+        self.parse_attribute_with_inner_parse_policy(inner_parse_policy)
+    }
+
+    /// The same as `parse_attribute`, except it takes in an `InnerAttributeParsePolicy`
+    /// that prescribes how to handle inner attributes.
+    fn parse_attribute_with_inner_parse_policy(&mut self,
+                                               inner_parse_policy: InnerAttributeParsePolicy)
+                                               -> PResult<'a, ast::Attribute> {
+        debug!("parse_attribute_with_inner_parse_policy: inner_parse_policy={:?} self.token={:?}",
+               inner_parse_policy,
+               self.token);
         let (span, value, mut style) = match self.token {
             token::Pound => {
                 let lo = self.span.lo;
                 self.bump();
 
-                if permit_inner {
+                if inner_parse_policy == InnerAttributeParsePolicy::Permitted {
                     self.expected_tokens.push(TokenType::Token(token::Not));
                 }
                 let style = if self.token == token::Not {
                     self.bump();
-                    if !permit_inner {
+                    if let InnerAttributeParsePolicy::NotPermitted { reason } = inner_parse_policy
+                    {
                         let span = self.span;
                         self.diagnostic()
-                            .struct_span_err(span,
-                                             "an inner attribute is not permitted in this context")
-                            .help("place inner attribute at the top of the module or \
-                                   block")
+                            .struct_span_err(span, reason)
+                            .note("inner attributes and doc comments, like `#![no_std]` or \
+                                   `//! My crate`, annotate the item enclosing them, and are \
+                                   usually found at the beginning of source files. Outer \
+                                   attributes and doc comments, like `#[test]` and
+                                   `/// My function`, annotate the item following them.")
                             .emit()
                     }
                     ast::AttrStyle::Inner
@@ -95,7 +136,8 @@ impl<'a> Parser<'a> {
             }
         };
 
-        if permit_inner && self.token == token::Semi {
+        if inner_parse_policy == InnerAttributeParsePolicy::Permitted &&
+           self.token == token::Semi {
             self.bump();
             self.span_warn(span,
                            "this inner attribute syntax is deprecated. The new syntax is \

src/test/parse-fail/inner-attr-after-doc-comment.rs

@@ -0,0 +1,20 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -Z parse-only
+
+#![feature(lang_items)]
+/**
+ * My module
+ */
+
+#![recursion_limit="100"]
+//~^ ERROR an inner attribute is not permitted following an outer doc comment
+fn main() {}

src/test/parse-fail/inner-attr.rs

@@ -0,0 +1,16 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -Z parse-only
+
+#[feature(lang_items)]
+
+#![recursion_limit="100"] //~ ERROR an inner attribute is not permitted following an outer attribute
+fn main() {}