Represent doc comments using their own node and not as COMMENT trivia

For two reasons:

 - This simplifies my work to fix #23088; to fix that issue, macros must have to be able to return doc comments (and not just desugared doc comments), and code in `syntax-bridge` doesn't expect macros to return trivia. Making doc comments non-trivia solves that.
 - It should simplify the work to attach trivia to tokens; doc comments have no obvious place to attach (for example, when between two attributes we must attach them to either the preceding `]` or the following `#`, both will complicate code handling them).

Furthermore, arguably doc comments are really not a trivia: it's an error to put them in an unexpected place, and reason 2 above reveals that they're more like a kind of an attribute than a comment.

This touches a lot of places (especially assists etc.) subtly; I fixed what I found and the tests helped reveal more, but it's certainly possible some places are still not handling them correctly now.

Author: ChayimFriedman2

crates/hir-def/src/attrs/docs.rs

@@ -22,8 +22,8 @@ use hir_expand::{
 };
 use span::AstIdMap;
 use syntax::{
-    AstNode, AstToken, SyntaxNode,
-    ast::{self, AttrDocCommentIter, IsString},
+    AstNode, SyntaxNode,
+    ast::{self, IsString},
 };
 use thin_vec::ThinVec;
 use tt::{TextRange, TextSize};
@@ -199,10 +199,10 @@ impl Docs {
         ));
     }
 
-    fn extend_with_doc_comment(&mut self, comment: ast::Comment, indent: &mut usize) {
-        let Some((doc, offset)) = comment.doc_comment() else { return };
-        let offset = comment.syntax().text_range().start() + offset;
-        self.extend_with_doc_str(doc, offset, indent, comment.kind().shape);
+    fn extend_with_doc_comment(&mut self, comment: ast::DocComment, indent: &mut usize) {
+        let doc = comment.text();
+        let offset = comment.syntax().text_range().start() + ast::DocComment::PREFIX_LEN;
+        self.extend_with_doc_str(doc, offset, indent, comment.shape());
     }
 
     fn extend_with_doc_attr(&mut self, value: ast::String, indent: &mut usize) {
@@ -591,13 +591,13 @@ fn extend_with_attrs<'a, 'db>(
     let mut expander = None;
 
     expand_cfg_attr_with_doc_comments::<_, Infallible>(
-        AttrDocCommentIter::from_syntax_node(node).filter(|attr| match attr {
-            Either::Left(attr) => attr.kind().is_inner() == expect_inner_attrs,
-            Either::Right(comment) => comment
-                .kind()
-                .doc
-                .is_some_and(|kind| (kind == ast::CommentPlacement::Inner) == expect_inner_attrs),
-        }),
+        node.children()
+            .filter_map(ast::AnyAttr::cast)
+            .filter(|attr| attr.kind().is_inner() == expect_inner_attrs)
+            .map(|attr| match attr {
+                ast::AnyAttr::Attr(it) => Either::Left(it),
+                ast::AnyAttr::DocComment(it) => Either::Right(it),
+            }),
         || *cfg_options.get_or_insert_with(get_cfg_options),
         |attr| {
             match attr {
@@ -727,7 +727,7 @@ pub(crate) fn extract_docs<'a, 'db>(
 mod tests {
     use expect_test::expect;
     use hir_expand::InFile;
-    use syntax::{AstToken, ast};
+    use syntax::{AstNode, ast};
     use test_fixture::WithFixture;
     use thin_vec::ThinVec;
     use tt::{TextRange, TextSize};
@@ -911,8 +911,8 @@ mod tests {
         let comment = syntax::SourceFile::parse(source, span::Edition::CURRENT)
             .syntax_node()
             .descendants_with_tokens()
-            .filter_map(|it| it.into_token())
-            .find_map(ast::Comment::cast)
+            .filter_map(|it| it.into_node())
+            .find_map(ast::DocComment::cast)
             .expect("no comment in the fixture");
         let mut docs = Docs {
             docs: String::new(),

crates/ide-assists/src/handlers/convert_comment_block.rs

@@ -1,7 +1,7 @@
 use itertools::Itertools;
 use syntax::{
-    AstToken, Direction, SyntaxElement, TextRange,
-    ast::{self, Comment, CommentKind, CommentShape, Whitespace, edit::IndentLevel},
+    AstToken, SyntaxToken, TextRange,
+    ast::{self, CommentKind, CommentShape, Whitespace, edit::IndentLevel},
 };
 
 use crate::{AssistContext, AssistId, Assists};
@@ -22,19 +22,19 @@ use crate::{AssistContext, AssistId, Assists};
 //   */
 // ```
 pub(crate) fn convert_comment_block(acc: &mut Assists, ctx: &AssistContext<'_, '_>) -> Option<()> {
-    let comment = ctx.find_token_at_offset::<ast::Comment>()?;
+    let comment = ctx.find_token_at_offset::<ast::AnyComment>()?;
     // Only allow comments which are alone on their line
     if let Some(prev) = comment.syntax().prev_token() {
         Whitespace::cast(prev).filter(|w| w.text().contains('\n'))?;
     }
 
-    match comment.kind().shape {
+    match comment.shape() {
         ast::CommentShape::Block => block_to_line(acc, comment),
         ast::CommentShape::Line => line_to_block(acc, comment),
     }
 }
 
-fn block_to_line(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
+fn block_to_line(acc: &mut Assists, comment: ast::AnyComment) -> Option<()> {
     let target = comment.syntax().text_range();
 
     acc.add(
@@ -45,9 +45,7 @@ fn block_to_line(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
             let indentation = IndentLevel::from_token(comment.syntax());
             let line_prefix = CommentKind { shape: CommentShape::Line, ..comment.kind() }.prefix();
 
-            let text = comment.text();
-            let text = &text[comment.prefix().len()..(text.len() - "*/".len())].trim();
-
+            let text = comment.text().trim();
             let lines = text.lines().peekable();
 
             let indent_spaces = indentation.to_string();
@@ -69,7 +67,7 @@ fn block_to_line(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
     )
 }
 
-fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
+fn line_to_block(acc: &mut Assists, comment: ast::AnyComment) -> Option<()> {
     // Find all the comments we'll be collapsing into a block
     let comments = relevant_line_comments(&comment);
 
@@ -109,37 +107,26 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
 /// The line -> block assist can  be invoked from anywhere within a sequence of line comments.
 /// relevant_line_comments crawls backwards and forwards finding the complete sequence of comments that will
 /// be joined.
-pub(crate) fn relevant_line_comments(comment: &ast::Comment) -> Vec<Comment> {
-    // The prefix identifies the kind of comment we're dealing with
-    let prefix = comment.prefix();
-    let same_prefix = |c: &ast::Comment| c.prefix() == prefix;
+pub(crate) fn relevant_line_comments(comment: &ast::AnyComment) -> Vec<ast::AnyComment> {
+    let expected_kind = comment.kind();
+    let same_kind = |c: &ast::AnyComment| c.kind() == expected_kind;
 
     // These tokens are allowed to exist between comments
-    let skippable = |not: &SyntaxElement| {
-        not.clone()
-            .into_token()
-            .and_then(Whitespace::cast)
-            .map(|w| !w.spans_multiple_lines())
-            .unwrap_or(false)
+    let skippable = |not: &SyntaxToken| {
+        Whitespace::cast(not.clone()).map(|w| !w.spans_multiple_lines()).unwrap_or(false)
     };
 
     // Find all preceding comments (in reverse order) that have the same prefix
-    let prev_comments = comment
-        .syntax()
-        .siblings_with_tokens(Direction::Prev)
+    let prev_comments = std::iter::successors(Some(comment.syntax().clone()), |it| it.prev_token())
         .filter(|s| !skippable(s))
-        .map(|not| not.into_token().and_then(Comment::cast).filter(same_prefix))
-        .take_while(|opt_com| opt_com.is_some())
-        .flatten()
+        .map_while(ast::AnyComment::cast)
+        .take_while(same_kind)
         .skip(1); // skip the first element so we don't duplicate it in next_comments
 
-    let next_comments = comment
-        .syntax()
-        .siblings_with_tokens(Direction::Next)
+    let next_comments = std::iter::successors(Some(comment.syntax().clone()), |it| it.next_token())
         .filter(|s| !skippable(s))
-        .map(|not| not.into_token().and_then(Comment::cast).filter(same_prefix))
-        .take_while(|opt_com| opt_com.is_some())
-        .flatten();
+        .map_while(ast::AnyComment::cast)
+        .take_while(same_kind);
 
     let mut comments: Vec<_> = prev_comments.collect();
     comments.reverse();
@@ -161,7 +148,7 @@ pub(crate) fn relevant_line_comments(comment: &ast::Comment) -> Vec<Comment> {
 //              */
 //
 // But since such comments aren't idiomatic we're okay with this.
-pub(crate) fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> String {
+pub(crate) fn line_comment_text(indentation: IndentLevel, comm: ast::AnyComment) -> String {
     let text = comm.text();
     let contents_without_prefix = text.strip_prefix(comm.prefix()).unwrap_or(text);
     let contents = contents_without_prefix.strip_prefix(' ').unwrap_or(contents_without_prefix);

crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs

@@ -1,10 +1,12 @@
 use itertools::Itertools;
 use syntax::{
-    AstToken, Direction, SyntaxElement, TextRange,
-    ast::{self, Comment, CommentPlacement, Whitespace, edit::IndentLevel},
+    AstToken, TextRange,
+    ast::{self, AttrKind, Whitespace, edit::IndentLevel},
 };
 
-use crate::{AssistContext, AssistId, Assists};
+use crate::{
+    AssistContext, AssistId, Assists, handlers::convert_comment_block::relevant_line_comments,
+};
 
 // Assist: comment_to_doc
 //
@@ -23,15 +25,15 @@ pub(crate) fn convert_comment_from_or_to_doc(
     acc: &mut Assists,
     ctx: &AssistContext<'_, '_>,
 ) -> Option<()> {
-    let comment = ctx.find_token_at_offset::<ast::Comment>()?;
+    let comment = ctx.find_token_at_offset::<ast::AnyComment>()?;
 
     match comment.kind().doc {
         Some(_) => doc_to_comment(acc, comment),
         None => can_be_doc_comment(&comment).and_then(|style| comment_to_doc(acc, comment, style)),
     }
 }
 
-fn doc_to_comment(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
+fn doc_to_comment(acc: &mut Assists, comment: ast::AnyComment) -> Option<()> {
     let target = if comment.kind().shape.is_line() {
         line_comments_text_range(&comment)?
     } else {
@@ -52,15 +54,15 @@ fn doc_to_comment(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
                     let prefix = format!("{indentation}//");
                     relevant_line_comments(&comment)
                         .iter()
-                        .map(|comment| comment.text())
+                        .map(|comment| comment.text_with_markers())
                         .flat_map(|text| text.lines())
                         .map(|line| line.replacen(line_start, &prefix, 1))
                         .join("\n")
                 }
                 ast::CommentShape::Block => {
                     let block_start = comment.prefix();
                     comment
-                        .text()
+                        .text_with_markers()
                         .lines()
                         .enumerate()
                         .map(|(idx, line)| {
@@ -78,7 +80,7 @@ fn doc_to_comment(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
     )
 }
 
-fn comment_to_doc(acc: &mut Assists, comment: ast::Comment, style: CommentPlacement) -> Option<()> {
+fn comment_to_doc(acc: &mut Assists, comment: ast::AnyComment, style: AttrKind) -> Option<()> {
     let target = if comment.kind().shape.is_line() {
         line_comments_text_range(&comment)?
     } else {
@@ -96,23 +98,23 @@ fn comment_to_doc(acc: &mut Assists, comment: ast::Comment, style: CommentPlacem
                 ast::CommentShape::Line => {
                     let indentation = IndentLevel::from_token(comment.syntax());
                     let line_start = match style {
-                        CommentPlacement::Inner => format!("{indentation}//!"),
-                        CommentPlacement::Outer => format!("{indentation}///"),
+                        AttrKind::Inner => format!("{indentation}//!"),
+                        AttrKind::Outer => format!("{indentation}///"),
                     };
                     relevant_line_comments(&comment)
                         .iter()
-                        .map(|comment| comment.text())
+                        .map(|comment| comment.text_with_markers())
                         .flat_map(|text| text.lines())
                         .map(|line| line.replacen("//", &line_start, 1))
                         .join("\n")
                 }
                 ast::CommentShape::Block => {
                     let block_start = match style {
-                        CommentPlacement::Inner => "/*!",
-                        CommentPlacement::Outer => "/**",
+                        AttrKind::Inner => "/*!",
+                        AttrKind::Outer => "/**",
                     };
                     comment
-                        .text()
+                        .text_with_markers()
                         .lines()
                         .enumerate()
                         .map(|(idx, line)| {
@@ -176,7 +178,7 @@ fn comment_to_doc(acc: &mut Assists, comment: ast::Comment, style: CommentPlacem
 ///     // Modules only normally get inner documentation when they are defined as a separate file.
 /// }
 /// ```
-fn can_be_doc_comment(comment: &ast::Comment) -> Option<CommentPlacement> {
+fn can_be_doc_comment(comment: &ast::AnyComment) -> Option<AttrKind> {
     use syntax::SyntaxKind::*;
 
     // if the comment is not on its own line, then we do not propose anything.
@@ -186,59 +188,18 @@ fn can_be_doc_comment(comment: &ast::Comment) -> Option<CommentPlacement> {
             Whitespace::cast(prev).filter(|w| w.text().contains('\n'))?;
         }
         // There is no previous token, this is the start of the file.
-        None => return Some(CommentPlacement::Inner),
+        None => return Some(AttrKind::Inner),
     }
 
     // check if comment is followed by: `struct`, `trait`, `mod`, `fn`, `type`, `extern crate`,
     // `use` or `const`.
     let parent = comment.syntax().parent();
     let par_kind = parent.as_ref().map(|parent| parent.kind());
     matches!(par_kind, Some(STRUCT | TRAIT | MODULE | FN | TYPE_ALIAS | EXTERN_CRATE | USE | CONST))
-        .then_some(CommentPlacement::Outer)
-}
-
-/// The line -> block assist can  be invoked from anywhere within a sequence of line comments.
-/// relevant_line_comments crawls backwards and forwards finding the complete sequence of comments that will
-/// be joined.
-pub(crate) fn relevant_line_comments(comment: &ast::Comment) -> Vec<Comment> {
-    // The prefix identifies the kind of comment we're dealing with
-    let prefix = comment.prefix();
-    let same_prefix = |c: &ast::Comment| c.prefix() == prefix;
-
-    // These tokens are allowed to exist between comments
-    let skippable = |not: &SyntaxElement| {
-        not.clone()
-            .into_token()
-            .and_then(Whitespace::cast)
-            .map(|w| !w.spans_multiple_lines())
-            .unwrap_or(false)
-    };
-
-    // Find all preceding comments (in reverse order) that have the same prefix
-    let prev_comments = comment
-        .syntax()
-        .siblings_with_tokens(Direction::Prev)
-        .filter(|s| !skippable(s))
-        .map(|not| not.into_token().and_then(Comment::cast).filter(same_prefix))
-        .take_while(|opt_com| opt_com.is_some())
-        .flatten()
-        .skip(1); // skip the first element so we don't duplicate it in next_comments
-
-    let next_comments = comment
-        .syntax()
-        .siblings_with_tokens(Direction::Next)
-        .filter(|s| !skippable(s))
-        .map(|not| not.into_token().and_then(Comment::cast).filter(same_prefix))
-        .take_while(|opt_com| opt_com.is_some())
-        .flatten();
-
-    let mut comments: Vec<_> = prev_comments.collect();
-    comments.reverse();
-    comments.extend(next_comments);
-    comments
+        .then_some(AttrKind::Outer)
 }
 
-fn line_comments_text_range(comment: &ast::Comment) -> Option<TextRange> {
+fn line_comments_text_range(comment: &ast::AnyComment) -> Option<TextRange> {
     let comments = relevant_line_comments(comment);
     let first = comments.first()?;
     let indentation = IndentLevel::from_token(first.syntax());