Keep doc attribute order

Author: Veykril

crates/hir_def/src/attr.rs

@@ -9,7 +9,7 @@ use itertools::Itertools;
 use mbe::ast_to_token_tree;
 use syntax::{
     ast::{self, AstNode, AttrsOwner},
-    SmolStr,
+    AstToken, SmolStr,
 };
 use tt::Subtree;
 
@@ -110,18 +110,25 @@ impl Attrs {
     }
 
     pub(crate) fn new(owner: &dyn AttrsOwner, hygiene: &Hygiene) -> Attrs {
-        let docs = ast::CommentIter::from_syntax_node(owner.syntax()).doc_comment_text().map(
-            |docs_text| Attr {
-                input: Some(AttrInput::Literal(SmolStr::new(docs_text))),
-                path: ModPath::from(hir_expand::name!(doc)),
-            },
-        );
-        let mut attrs = owner.attrs().peekable();
-        let entries = if attrs.peek().is_none() && docs.is_none() {
+        let docs = ast::CommentIter::from_syntax_node(owner.syntax()).map(|docs_text| {
+            (
+                docs_text.syntax().text_range().start(),
+                docs_text.doc_comment().map(|doc| Attr {
+                    input: Some(AttrInput::Literal(SmolStr::new(doc))),
+                    path: ModPath::from(hir_expand::name!(doc)),
+                }),
+            )
+        });
+        let attrs = owner
+            .attrs()
+            .map(|attr| (attr.syntax().text_range().start(), Attr::from_src(attr, hygiene)));
+        // sort here by syntax node offset because the source can have doc attributes and doc strings be interleaved
+        let attrs: Vec<_> = docs.chain(attrs).sorted_by_key(|&(offset, _)| offset).collect();
+        let entries = if attrs.is_empty() {
             // Avoid heap allocation
             None
         } else {
-            Some(attrs.flat_map(|ast| Attr::from_src(ast, hygiene)).chain(docs).collect())
+            Some(attrs.into_iter().flat_map(|(_, attr)| attr).collect())
         };
         Attrs { entries }
     }
@@ -195,10 +202,15 @@ impl Attr {
     fn from_src(ast: ast::Attr, hygiene: &Hygiene) -> Option<Attr> {
         let path = ModPath::from_src(ast.path()?, hygiene)?;
         let input = if let Some(lit) = ast.literal() {
-            let value = if let ast::LiteralKind::String(string) = lit.kind() {
-                string.value()?.into()
-            } else {
-                lit.syntax().first_token()?.text().trim_matches('"').into()
+            // FIXME: escape?
+            let value = match lit.kind() {
+                ast::LiteralKind::String(string) if string.is_raw() => {
+                    let text = string.text().as_str();
+                    let text = &text[string.text_range_between_quotes()?
+                        - string.syntax().text_range().start()];
+                    text.into()
+                }
+                _ => lit.syntax().first_token()?.text().trim_matches('"').into(),
             };
             Some(AttrInput::Literal(value))
         } else if let Some(tt) = ast.token_tree() {

crates/syntax/src/ast/token_ext.rs

@@ -18,12 +18,33 @@ impl ast::Comment {
     }
 
     pub fn prefix(&self) -> &'static str {
-        let &(prefix, _kind) = CommentKind::BY_PREFIX
-            .iter()
-            .find(|&(prefix, kind)| self.kind() == *kind && self.text().starts_with(prefix))
-            .unwrap();
+        let &(prefix, _kind) = CommentKind::with_prefix_from_text(self.text());
         prefix
     }
+
+    pub fn kind_and_prefix(&self) -> &(&'static str, CommentKind) {
+        CommentKind::with_prefix_from_text(self.text())
+    }
+
+    /// Returns the textual content of a doc comment block as a single string.
+    /// That is, strips leading `///` (+ optional 1 character of whitespace),
+    /// trailing `*/`, trailing whitespace and then joins the lines.
+    pub fn doc_comment(&self) -> Option<&str> {
+        match self.kind_and_prefix() {
+            (prefix, CommentKind { shape, doc: Some(_) }) => {
+                let text = &self.text().as_str()[prefix.len()..];
+                let ws = text.chars().next().filter(|c| c.is_whitespace());
+                let text = ws.map_or(text, |ws| &text[ws.len_utf8()..]);
+                match shape {
+                    CommentShape::Block if text.ends_with("*/") => {
+                        Some(&text[..text.len() - "*/".len()])
+                    }
+                    _ => Some(text),
+                }
+            }
+            _ => None,
+        }
+    }
 }
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
@@ -67,12 +88,13 @@ impl CommentKind {
     ];
 
     pub(crate) fn from_text(text: &str) -> CommentKind {
-        let &(_prefix, kind) = CommentKind::BY_PREFIX
-            .iter()
-            .find(|&(prefix, _kind)| text.starts_with(prefix))
-            .unwrap();
+        let &(_prefix, kind) = Self::with_prefix_from_text(text);
         kind
     }
+
+    fn with_prefix_from_text(text: &str) -> &(&'static str, CommentKind) {
+        CommentKind::BY_PREFIX.iter().find(|&(prefix, _kind)| text.starts_with(prefix)).unwrap()
+    }
 }
 
 impl ast::Whitespace {

crates/syntax/src/ast/traits.rs

@@ -91,40 +91,12 @@ impl CommentIter {
     /// That is, strips leading `///` (+ optional 1 character of whitespace),
     /// trailing `*/`, trailing whitespace and then joins the lines.
     pub fn doc_comment_text(self) -> Option<String> {
-        let mut has_comments = false;
-        let docs = self
-            .filter(|comment| comment.kind().doc.is_some())
-            .map(|comment| {
-                has_comments = true;
-                let prefix_len = comment.prefix().len();
-
-                let line: &str = comment.text().as_str();
-
-                // Determine if the prefix or prefix + 1 char is stripped
-                let pos =
-                    if let Some(ws) = line.chars().nth(prefix_len).filter(|c| c.is_whitespace()) {
-                        prefix_len + ws.len_utf8()
-                    } else {
-                        prefix_len
-                    };
-
-                let end = if comment.kind().shape.is_block() && line.ends_with("*/") {
-                    line.len() - 2
-                } else {
-                    line.len()
-                };
-
-                // Note that we do not trim the end of the line here
-                // since whitespace can have special meaning at the end
-                // of a line in markdown.
-                line[pos..end].to_owned()
-            })
-            .join("\n");
-
-        if has_comments {
-            Some(docs)
-        } else {
+        let docs =
+            self.filter_map(|comment| comment.doc_comment().map(ToOwned::to_owned)).join("\n");
+        if docs.is_empty() {
             None
+        } else {
+            Some(docs)
         }
     }
 }