Skip to content

internal: Represent doc comments using their own node and not as COMMENT trivia #23218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ChayimFriedman2 wants to merge 1 commit into
base: master
Choose a base branch
from
+696 −706
Open

internal: Represent doc comments using their own node and not as COMMENT trivia #23218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions crates/hir-def/src/attrs/docs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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(),
Expand Down
51 changes: 19 additions & 32 deletions crates/ide-assists/src/handlers/convert_comment_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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(
Expand All @@ -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();
Expand All @@ -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);

Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down
79 changes: 20 additions & 59 deletions crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
//
Expand All @@ -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 {
Expand All @@ -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)| {
Expand All @@ -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 {
Expand All @@ -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)| {
Expand Down Expand Up @@ -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.
Expand All @@ -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());
Expand Down
Loading