Auto merge of #67151 - petrochenkov:docomm, r=estebank

doc comments: Less attribute mimicking

Make sure doc comments are not converted into intermediate meta-items, or not mixed with `doc(inline)` or something like that.

Follow-up to #65750.

Author: bors

src/librustc_lint/builtin.rs

@@ -314,6 +314,10 @@ pub struct MissingDoc {
 impl_lint_pass!(MissingDoc => [MISSING_DOCS]);
 
 fn has_doc(attr: &ast::Attribute) -> bool {
+    if attr.is_doc_comment() {
+        return true;
+    }
+
     if !attr.check_name(sym::doc) {
         return false;
     }
@@ -768,7 +772,7 @@ impl UnusedDocComment {
 
             let span = sugared_span.take().unwrap_or_else(|| attr.span);
 
-            if attr.check_name(sym::doc) {
+            if attr.is_doc_comment() || attr.check_name(sym::doc) {
                 let mut err = cx.struct_span_lint(UNUSED_DOC_COMMENTS, span, "unused doc comment");
 
                 err.span_label(

src/librustc_lint/unused.rs

@@ -271,6 +271,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes {
     fn check_attribute(&mut self, cx: &LateContext<'_, '_>, attr: &ast::Attribute) {
         debug!("checking attribute: {:?}", attr);
 
+        if attr.is_doc_comment() {
+            return;
+        }
+
         let attr_info = attr.ident().and_then(|ident| self.builtin_attributes.get(&ident.name));
 
         if let Some(&&(name, ty, ..)) = attr_info {

src/librustc_parse/validate_attr.rs

@@ -4,16 +4,17 @@ use crate::parse_in;
 
 use rustc_errors::{Applicability, PResult};
 use rustc_feature::{AttributeTemplate, BUILTIN_ATTRIBUTE_MAP};
-use syntax::ast::{
-    self, AttrKind, Attribute, Ident, MacArgs, MacDelimiter, MetaItem, MetaItemKind,
-};
-use syntax::attr::mk_name_value_item_str;
+use syntax::ast::{self, Attribute, MacArgs, MacDelimiter, MetaItem, MetaItemKind};
 use syntax::early_buffered_lints::ILL_FORMED_ATTRIBUTE_INPUT;
 use syntax::sess::ParseSess;
 use syntax::tokenstream::DelimSpan;
 use syntax_pos::{sym, Symbol};
 
 pub fn check_meta(sess: &ParseSess, attr: &Attribute) {
+    if attr.is_doc_comment() {
+        return;
+    }
+
     let attr_info =
         attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name)).map(|a| **a);
 
@@ -33,26 +34,22 @@ pub fn check_meta(sess: &ParseSess, attr: &Attribute) {
 }
 
 pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, MetaItem> {
-    Ok(match attr.kind {
-        AttrKind::Normal(ref item) => MetaItem {
-            span: attr.span,
-            path: item.path.clone(),
-            kind: match &attr.get_normal_item().args {
-                MacArgs::Empty => MetaItemKind::Word,
-                MacArgs::Eq(_, t) => {
-                    let v = parse_in(sess, t.clone(), "name value", |p| p.parse_unsuffixed_lit())?;
-                    MetaItemKind::NameValue(v)
-                }
-                MacArgs::Delimited(dspan, delim, t) => {
-                    check_meta_bad_delim(sess, *dspan, *delim, "wrong meta list delimiters");
-                    let nmis = parse_in(sess, t.clone(), "meta list", |p| p.parse_meta_seq_top())?;
-                    MetaItemKind::List(nmis)
-                }
-            },
+    let item = attr.get_normal_item();
+    Ok(MetaItem {
+        span: attr.span,
+        path: item.path.clone(),
+        kind: match &item.args {
+            MacArgs::Empty => MetaItemKind::Word,
+            MacArgs::Eq(_, t) => {
+                let v = parse_in(sess, t.clone(), "name value", |p| p.parse_unsuffixed_lit())?;
+                MetaItemKind::NameValue(v)
+            }
+            MacArgs::Delimited(dspan, delim, t) => {
+                check_meta_bad_delim(sess, *dspan, *delim, "wrong meta list delimiters");
+                let nmis = parse_in(sess, t.clone(), "meta list", |p| p.parse_meta_seq_top())?;
+                MetaItemKind::List(nmis)
+            }
         },
-        AttrKind::DocComment(comment) => {
-            mk_name_value_item_str(Ident::new(sym::doc, attr.span), comment, attr.span)
-        }
     })
 }
 

src/librustc_save_analysis/lib.rs

@@ -815,15 +815,15 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> {
         let mut result = String::new();
 
         for attr in attrs {
-            if attr.check_name(sym::doc) {
-                if let Some(val) = attr.value_str() {
-                    if attr.is_doc_comment() {
-                        result.push_str(&strip_doc_comment_decoration(&val.as_str()));
-                    } else {
-                        result.push_str(&val.as_str());
-                    }
-                    result.push('\n');
-                } else if let Some(meta_list) = attr.meta_item_list() {
+            if let Some(val) = attr.doc_str() {
+                if attr.is_doc_comment() {
+                    result.push_str(&strip_doc_comment_decoration(&val.as_str()));
+                } else {
+                    result.push_str(&val.as_str());
+                }
+                result.push('\n');
+            } else if attr.check_name(sym::doc) {
+                if let Some(meta_list) = attr.meta_item_list() {
                     meta_list
                         .into_iter()
                         .filter(|it| it.check_name(sym::include))

src/librustdoc/clean/types.rs

@@ -17,10 +17,10 @@ use rustc::ty::layout::VariantIdx;
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
 use rustc_index::vec::IndexVec;
 use rustc_target::spec::abi::Abi;
-use syntax::ast::{self, AttrKind, AttrStyle, Attribute, Ident};
+use syntax::ast::{self, AttrStyle, Ident};
 use syntax::attr;
 use syntax::source_map::DUMMY_SP;
-use syntax::util::comments;
+use syntax::util::comments::strip_doc_comment_decoration;
 use syntax_pos::hygiene::MacroKind;
 use syntax_pos::symbol::{sym, Symbol};
 use syntax_pos::{self, FileName};
@@ -502,58 +502,33 @@ impl Attributes {
         let mut cfg = Cfg::True;
         let mut doc_line = 0;
 
-        /// If `attr` is a doc comment, strips the leading and (if present)
-        /// trailing comments symbols, e.g. `///`, `/**`, and `*/`. Otherwise,
-        /// returns `attr` unchanged.
-        pub fn with_doc_comment_markers_stripped<T>(
-            attr: &Attribute,
-            f: impl FnOnce(&Attribute) -> T,
-        ) -> T {
-            match attr.kind {
-                AttrKind::Normal(_) => f(attr),
-                AttrKind::DocComment(comment) => {
-                    let comment =
-                        Symbol::intern(&comments::strip_doc_comment_decoration(&comment.as_str()));
-                    f(&Attribute {
-                        kind: AttrKind::DocComment(comment),
-                        id: attr.id,
-                        style: attr.style,
-                        span: attr.span,
-                    })
-                }
-            }
-        }
-
         let other_attrs = attrs
             .iter()
             .filter_map(|attr| {
-                with_doc_comment_markers_stripped(attr, |attr| {
+                if let Some(value) = attr.doc_str() {
+                    let (value, mk_fragment): (_, fn(_, _, _) -> _) = if attr.is_doc_comment() {
+                        (strip_doc_comment_decoration(&value.as_str()), DocFragment::SugaredDoc)
+                    } else {
+                        (value.to_string(), DocFragment::RawDoc)
+                    };
+
+                    let line = doc_line;
+                    doc_line += value.lines().count();
+                    doc_strings.push(mk_fragment(line, attr.span, value));
+
+                    if sp.is_none() {
+                        sp = Some(attr.span);
+                    }
+                    None
+                } else {
                     if attr.check_name(sym::doc) {
                         if let Some(mi) = attr.meta() {
-                            if let Some(value) = mi.value_str() {
-                                // Extracted #[doc = "..."]
-                                let value = value.to_string();
-                                let line = doc_line;
-                                doc_line += value.lines().count();
-
-                                if attr.is_doc_comment() {
-                                    doc_strings
-                                        .push(DocFragment::SugaredDoc(line, attr.span, value));
-                                } else {
-                                    doc_strings.push(DocFragment::RawDoc(line, attr.span, value));
-                                }
-
-                                if sp.is_none() {
-                                    sp = Some(attr.span);
-                                }
-                                return None;
-                            } else if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
+                            if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
                                 // Extracted #[doc(cfg(...))]
                                 match Cfg::parse(cfg_mi) {
                                     Ok(new_cfg) => cfg &= new_cfg,
                                     Err(e) => diagnostic.span_err(e.span, e.msg),
                                 }
-                                return None;
                             } else if let Some((filename, contents)) =
                                 Attributes::extract_include(&mi)
                             {
@@ -566,7 +541,7 @@ impl Attributes {
                         }
                     }
                     Some(attr.clone())
-                })
+                }
             })
             .collect();
 
@@ -589,7 +564,7 @@ impl Attributes {
 
         let inner_docs = attrs
             .iter()
-            .filter(|a| a.check_name(sym::doc))
+            .filter(|a| a.doc_str().is_some())
             .next()
             .map_or(true, |a| a.style == AttrStyle::Inner);
 

src/libsyntax/ast.rs

@@ -2364,10 +2364,6 @@ pub enum AttrKind {
     /// A doc comment (e.g. `/// ...`, `//! ...`, `/** ... */`, `/*! ... */`).
     /// Doc attributes (e.g. `#[doc="..."]`) are represented with the `Normal`
     /// variant (which is much less compact and thus more expensive).
-    ///
-    /// Note: `self.has_name(sym::doc)` and `self.check_name(sym::doc)` succeed
-    /// for this variant, but this may change in the future.
-    /// ```
     DocComment(Symbol),
 }
 

src/libsyntax/attr/builtin.rs

@@ -16,7 +16,7 @@ use syntax_pos::{symbol::sym, symbol::Symbol, Span};
 use rustc_error_codes::*;
 
 pub fn is_builtin_attr(attr: &Attribute) -> bool {
-    attr.ident().filter(|ident| is_builtin_attr_name(ident.name)).is_some()
+    attr.is_doc_comment() || attr.ident().filter(|ident| is_builtin_attr_name(ident.name)).is_some()
 }
 
 enum AttrError {

src/libsyntax/attr/mod.rs

@@ -139,7 +139,7 @@ impl Attribute {
     pub fn has_name(&self, name: Symbol) -> bool {
         match self.kind {
             AttrKind::Normal(ref item) => item.path == name,
-            AttrKind::DocComment(_) => name == sym::doc,
+            AttrKind::DocComment(_) => false,
         }
     }
 
@@ -163,7 +163,7 @@ impl Attribute {
                     None
                 }
             }
-            AttrKind::DocComment(_) => Some(Ident::new(sym::doc, self.span)),
+            AttrKind::DocComment(_) => None,
         }
     }
     pub fn name_or_empty(&self) -> Symbol {
@@ -173,7 +173,7 @@ impl Attribute {
     pub fn value_str(&self) -> Option<Symbol> {
         match self.kind {
             AttrKind::Normal(ref item) => item.meta(self.span).and_then(|meta| meta.value_str()),
-            AttrKind::DocComment(comment) => Some(comment),
+            AttrKind::DocComment(..) => None,
         }
     }
 
@@ -279,27 +279,35 @@ impl Attribute {
         }
     }
 
+    pub fn doc_str(&self) -> Option<Symbol> {
+        match self.kind {
+            AttrKind::DocComment(symbol) => Some(symbol),
+            AttrKind::Normal(ref item) if item.path == sym::doc => {
+                item.meta(self.span).and_then(|meta| meta.value_str())
+            }
+            _ => None,
+        }
+    }
+
     pub fn get_normal_item(&self) -> &AttrItem {
         match self.kind {
             AttrKind::Normal(ref item) => item,
-            AttrKind::DocComment(_) => panic!("unexpected sugared doc"),
+            AttrKind::DocComment(_) => panic!("unexpected doc comment"),
         }
     }
 
     pub fn unwrap_normal_item(self) -> AttrItem {
         match self.kind {
             AttrKind::Normal(item) => item,
-            AttrKind::DocComment(_) => panic!("unexpected sugared doc"),
+            AttrKind::DocComment(_) => panic!("unexpected doc comment"),
         }
     }
 
     /// Extracts the MetaItem from inside this Attribute.
     pub fn meta(&self) -> Option<MetaItem> {
         match self.kind {
             AttrKind::Normal(ref item) => item.meta(self.span),
-            AttrKind::DocComment(comment) => {
-                Some(mk_name_value_item_str(Ident::new(sym::doc, self.span), comment, self.span))
-            }
+            AttrKind::DocComment(..) => None,
         }
     }
 }

src/libsyntax_expand/parse/tests.rs

@@ -3,12 +3,11 @@ use crate::tests::{matches_codepattern, string_to_stream, with_error_checking_pa
 use errors::PResult;
 use rustc_parse::new_parser_from_source_str;
 use syntax::ast::{self, Name, PatKind};
-use syntax::attr::first_attr_value_str_by_name;
 use syntax::print::pprust::item_to_string;
 use syntax::ptr::P;
 use syntax::sess::ParseSess;
 use syntax::source_map::FilePathMapping;
-use syntax::symbol::{kw, sym};
+use syntax::symbol::{kw, sym, Symbol};
 use syntax::token::{self, Token};
 use syntax::tokenstream::{DelimSpan, TokenStream, TokenTree};
 use syntax::visit;
@@ -244,25 +243,20 @@ fn crlf_doc_comments() {
         let name_1 = FileName::Custom("crlf_source_1".to_string());
         let source = "/// doc comment\r\nfn foo() {}".to_string();
         let item = parse_item_from_source_str(name_1, source, &sess).unwrap().unwrap();
-        let doc = first_attr_value_str_by_name(&item.attrs, sym::doc).unwrap();
+        let doc = item.attrs.iter().filter_map(|at| at.doc_str()).next().unwrap();
         assert_eq!(doc.as_str(), "/// doc comment");
 
         let name_2 = FileName::Custom("crlf_source_2".to_string());
         let source = "/// doc comment\r\n/// line 2\r\nfn foo() {}".to_string();
         let item = parse_item_from_source_str(name_2, source, &sess).unwrap().unwrap();
-        let docs = item
-            .attrs
-            .iter()
-            .filter(|a| a.has_name(sym::doc))
-            .map(|a| a.value_str().unwrap().to_string())
-            .collect::<Vec<_>>();
-        let b: &[_] = &["/// doc comment".to_string(), "/// line 2".to_string()];
+        let docs = item.attrs.iter().filter_map(|at| at.doc_str()).collect::<Vec<_>>();
+        let b: &[_] = &[Symbol::intern("/// doc comment"), Symbol::intern("/// line 2")];
         assert_eq!(&docs[..], b);
 
         let name_3 = FileName::Custom("clrf_source_3".to_string());
         let source = "/** doc comment\r\n *  with CRLF */\r\nfn foo() {}".to_string();
         let item = parse_item_from_source_str(name_3, source, &sess).unwrap().unwrap();
-        let doc = first_attr_value_str_by_name(&item.attrs, sym::doc).unwrap();
+        let doc = item.attrs.iter().filter_map(|at| at.doc_str()).next().unwrap();
         assert_eq!(doc.as_str(), "/** doc comment\n *  with CRLF */");
     });
 }