Skip to content
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.

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

Implement "leading hole hugging" behavior #82

Merged
merged 5 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/air_r_formatter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ version = "0.0.0"
air_r_syntax = { workspace = true }
biome_formatter = { workspace = true }
biome_rowan = { workspace = true }
itertools = { workspace = true }
tracing = { workspace = true }

[dev-dependencies]
Expand Down
185 changes: 183 additions & 2 deletions crates/air_r_formatter/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use air_r_syntax::RIfStatement;
use air_r_syntax::RLanguage;
use air_r_syntax::RParenthesizedExpression;
use air_r_syntax::RSyntaxKind;
use air_r_syntax::RSyntaxToken;
use air_r_syntax::RWhileStatement;
use biome_formatter::comments::CommentKind;
use biome_formatter::comments::CommentPlacement;
Expand Down Expand Up @@ -65,15 +66,17 @@ impl CommentStyle for RCommentStyle {
.or_else(handle_if_statement_comment)
.or_else(handle_parenthesized_expression_comment)
.or_else(handle_argument_name_clause_comment)
.or_else(handle_argument_comment),
.or_else(handle_argument_comment)
.or_else(handle_hole_argument_comment),
CommentTextPosition::OwnLine => handle_for_comment(comment)
.or_else(handle_function_comment)
.or_else(handle_while_comment)
.or_else(handle_repeat_comment)
.or_else(handle_if_statement_comment)
.or_else(handle_parenthesized_expression_comment)
.or_else(handle_argument_name_clause_comment)
.or_else(handle_argument_comment),
.or_else(handle_argument_comment)
.or_else(handle_hole_argument_comment),
CommentTextPosition::SameLine => {
// Not applicable for R, we don't have `/* */` comments
CommentPlacement::Default(comment)
Expand Down Expand Up @@ -351,6 +354,184 @@ fn handle_argument_comment(comment: DecoratedComment<RLanguage>) -> CommentPlace
CommentPlacement::Default(comment)
}

/// Handle comments close to an argument hole
///
/// Hole comment handling is complicated by the fact that holes don't have any
/// associated tokens. This has two important consequences that we exploit:
///
/// - Comments are never enclosed by holes, i.e. `enclosing_node()` never
/// returns a hole.
///
/// - Comments will ALWAYS trail the hole by default, i.e. `preceding_node()`
/// is how you access the hole node connected to the comment.
///
/// Here is the logic for placing hole comments, assuming that we've already
/// determined that the `preceding_node()` is a hole. Note that this logic is
/// symmetric, which we quite like:
///
/// - If the following token is `,`, `)`, `]`, or `]]`, the comment is
/// "inside" the hole.
///
/// - If the previous sibling node before the hole exists and is not another
/// hole, attach the comment as trailing trivia on that.
///
/// - Else the comment is "after" the hole.
///
/// - If the following sibling node after the hole exists and is not another
/// hole, attach the comment as leading trivia on that.
///
/// - Otherwise attach the comment as leading on the hole itself. This happens
/// when there is not another preceding/following node, or that node is
/// another hole.
///
/// Note that the "following token" check skips following whitespace and
/// comments, which is what we want.
///
/// ## Comments "inside" the hole
///
/// Comment trails `a`. Following token is `,` and previous-sibling of hole is a
/// non-hole.
///
/// ```r
/// fn(
/// a,<hole> # comment
/// ,
/// b
/// )
/// ```
Comment on lines +395 to +401
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holes and comments are quite tricky to think about. In the following example:

fn(
  a, # comment
  ,
  b
)

you might think the formatter assumes the hole is here

fn(
  a, # comment
  <hole>,
  b
)

That would make # comment trailing on a automatically with the Default comment placement behavior.

But that isn't what happens! Instead, holes are placed "eagerly" like this:

fn(
  a,<hole># comment
  ,
  b
)

This causes # comment to be a trailing comment on the hole by default. This trailing comment behavior ALWAYS happens with holes, and is something we exploit as it simplifies our hole comment handling. It is also how the biome group handles array hole comments, so I feel good about it:
https://github.com/biomejs/biome/blob/66458a12a048b0e686b242d9d4dd0f0e90d1f92a/crates/biome_js_formatter/src/comments.rs#L241-L242

///
/// CommentB trails `b`. Following token is `)` and previous-sibling of hole is
/// a non-hole.
///
/// This example is particularly important. We want comments on trailing
/// `,` in `list2()` calls to get attached to the `b` non-hole node,
/// otherwise it will get moved to the next line if it stays attached to
/// the hole.
///
/// ```r
/// list2(
/// a, # commentA
/// b,<hole> # commentB
/// )
/// ```
///
/// Comment1 leads hole. Following token is `,` and there is no previous-sibling
/// of the hole. Note that `following_token()` skips `# comment2` here and jumps
/// straight to `,`, which is what we want.
///
/// Comment2 leads hole. Following token is `,` and there is no previous-sibling
/// of the hole.
///
/// ```r
/// fn(<hole># comment1
/// # comment2
/// ,
/// x
/// )
/// ```
///
/// Comment leads hole. Following token is `,` and the previous-sibling of
/// the hole is another hole.
///
/// ```r
/// fn(
/// a,<another-hole>
/// ,<hole># comment
/// ,
/// b
/// )
/// ```
///
/// Comment leads hole. Following token is `,` and the previous-sibling of
/// the hole is another hole.
///
/// ```r
/// fn(<another-hole>
/// ,<hole># comment
/// ,
/// x
/// )
/// ```
///
/// ## Comment "after" the hole
///
/// Comment leads `x`. Following token is not `,`, `)`, `]`, or `]]`, and the
/// following-sibling of the hole is a non-hole we can lead.
///
/// ```r
/// fn(
/// ,<hole>
/// ,# comment
/// x
/// )
/// ```
///
/// Comment1 leads `x`. Following token is not `,`, `)`, `]`, or `]]`, and the
/// following-sibling of the hole is a non-hole we can lead. Note that
/// `following_token()` skips `# comment2` here and jumps straight to `,`, which
/// is what we want.
///
/// Comment2 leads `x`. Following token is not `,`, `)`, `]`, or `]]`, and the
/// following-sibling of the hole is a non-hole we can lead.
///
/// ```r
/// fn(
/// ,<hole>
/// ,# comment1
/// # comment2
/// x
/// )
/// ```
///
/// We can't think of any scenario where we have a comment "after" the hole,
/// but we don't have a following-sibling to lead.
fn handle_hole_argument_comment(
comment: DecoratedComment<RLanguage>,
) -> CommentPlacement<RLanguage> {
let Some(hole) = comment
.preceding_node()
.and_then(RArgument::cast_ref)
.filter(RArgument::is_hole)
.map(RArgument::into_syntax)
else {
return CommentPlacement::Default(comment);
};

// Note that `following_token()` nicely skips over following comments
let is_comment_inside_hole = matches!(
comment.following_token().map(RSyntaxToken::kind),
Some(
RSyntaxKind::COMMA
| RSyntaxKind::R_PAREN
| RSyntaxKind::R_BRACK
| RSyntaxKind::R_BRACK2
)
);

#[allow(clippy::collapsible_else_if)]
if is_comment_inside_hole {
if let Some(previous) = hole
.prev_sibling()
.and_then(RArgument::cast)
.filter(|argument| !argument.is_hole())
.map(RArgument::into_syntax)
{
return CommentPlacement::trailing(previous, comment);
}
} else {
if let Some(following) = comment
.following_node()
.and_then(RArgument::cast_ref)
.filter(|argument| !argument.is_hole())
.map(RArgument::into_syntax)
{
return CommentPlacement::leading(following, comment);
}
}

CommentPlacement::leading(hole, comment)
}

/// Make line comments between a `)` token and a `body`:
/// - Leading comments of the first expression within `{}` if `body` is a braced expression
/// - Dangling comments of the `{}` if `body` is an empty braced expression
Expand Down
Loading
Loading