From c4f0861da38c4a05aff288ef8c49e1e124c33018 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Fri, 20 Dec 2024 13:51:01 -0500 Subject: [PATCH] Add support for magic line breaks in left assignment (#118) * Add support for magic line breaks in left assignment * Add changelog bullet --- CHANGELOG.md | 2 + .../src/r/auxiliary/binary_expression.rs | 93 +++++++++- .../tests/specs/r/binary_expression.R | 86 +++++++-- .../tests/specs/r/binary_expression.R.snap | 170 ++++++++++++++---- 4 files changed, 303 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4f63a6a..ebed5c0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ # Development version +- Magic line breaks are now supported in left assignment (#118). + # 0.1.1 diff --git a/crates/air_r_formatter/src/r/auxiliary/binary_expression.rs b/crates/air_r_formatter/src/r/auxiliary/binary_expression.rs index 8d0cf3d3..f2844367 100644 --- a/crates/air_r_formatter/src/r/auxiliary/binary_expression.rs +++ b/crates/air_r_formatter/src/r/auxiliary/binary_expression.rs @@ -77,24 +77,113 @@ fn fmt_binary_sticky( } /// Assignment expressions keep LHS and RHS on the same line, separated by a single space +/// +/// # Magic line breaks +/// +/// The one exception to this is if a newline already exists between a left assignment +/// operator and the rhs, which is a magic line break case, like: +/// +/// ```r +/// response <- +/// if (condition) { +/// "yes" +/// } else { +/// "no" +/// } +/// +/// resolved <- +/// this %||% +/// complex_that(a, b, c, d) %||% +/// complex_that(e, f, g, h) %||% +/// default() +/// ``` +/// +/// Walrus assignment is not considered when looking for magic line breaks because we +/// don't want the `:=` case below to look like a request for expansion. While `:=` is +/// technically parsed as a binary operator, we format it more like a named argument with +/// a simple `space()` between the operator and the right hand side. +/// +/// ```r +/// # `:=` here is technically a binary operator, `x := y` is technically an unnamed argument +/// fn( +/// x := +/// y +/// ) +/// +/// # `=` here is not a binary operator, `x = y` is a named argument and `y` will be +/// # forced onto the same line as `=`. We want to treat `:=` like this. +/// fn( +/// x = +/// y +/// ) +/// ``` +/// +/// Comment handling with magic line breaks here is a bit tricky. Consider this example: +/// +/// ```r +/// x <- +/// y # comment +/// ``` +/// +/// Note that `# comment` is actually attached to the whole binary expression node. When +/// determining default comment placement, the [DecoratedComment::enclosing_node()] is the +/// root node here, making the [DecoratedComment::preceding_node()] the binary expression +/// node. +/// +/// This means we can't use a simple [block_indent()] on `right`. The `block_indent()` +/// would force a newline after the `y` but before the comment, moving the comment to the +/// next line. +/// +/// Instead, we do the same trick that we do in [fmt_binary_chain()] (and in Biome with +/// binary chains) of using the "rarely needed" manual [indent()] and adding a leading +/// [hard_line_break()] but not a trailing one. By avoiding a trailing hard line break, +/// the trailing comment is allowed to be formatted on the same line as `y`. fn fmt_binary_assignment( left: AnyRExpression, operator: SyntaxToken, right: AnyRExpression, f: &mut Formatter, ) -> FormatResult<()> { + let right = format_with(|f| { + if binary_assignment_needs_user_requested_expansion(&operator, &right) { + write!( + f, + [indent(&format_args![hard_line_break(), right.format()])] + ) + } else { + write!(f, [space(), right.format()]) + } + }); + write!( f, [group(&format_args![ left.format(), space(), operator.format(), - space(), - right.format() + right ])] ) } +fn binary_assignment_needs_user_requested_expansion( + operator: &SyntaxToken, + right: &AnyRExpression, +) -> bool { + // TODO: This should be configurable by an option, since it is a case of + // irreversible formatting + + // Only for these kinds of left assignment + if !matches!( + operator.kind(), + RSyntaxKind::EQUAL | RSyntaxKind::ASSIGN | RSyntaxKind::SUPER_ASSIGN + ) { + return false; + } + + right.syntax().has_leading_newline() +} + /// Format a binary expression /// /// These expressions are not chainable, they use a simple diff --git a/crates/air_r_formatter/tests/specs/r/binary_expression.R b/crates/air_r_formatter/tests/specs/r/binary_expression.R index b6d6e6ba..2b25ef3c 100644 --- a/crates/air_r_formatter/tests/specs/r/binary_expression.R +++ b/crates/air_r_formatter/tests/specs/r/binary_expression.R @@ -46,21 +46,6 @@ argument_that_is_quite_long + argument_that_is_quite_long^argument_that_is_quite 1 ^ 2 1 : 2 -# The following assignments should start the LHS/RHS on the same -# line as the operator -fn = function(x) { - x -} -fn <- function(x) { - x -} -fn <<- function(x) { - x -} - -identity(1) -> x -identity(1) ->> x - # ----------------------------------------------------------------------------- # Help specific @@ -452,3 +437,74 @@ ggplot() + geom_line() + geom_bar() %>% identity() + +# ----------------------------------------------------------------------------- +# Assignment + +# The following assignments should start the LHS/RHS on the same +# line as the operator +fn = function(x) { + x +} +fn <- function(x) { + x +} +fn <<- function(x) { + x +} + +# Assignment comment tests +fn <- function(x) # comment1 + { # comment2 + x # comment3 + } # comment4 + +identity(1) -> x +identity(1) ->> x + +# ----------------------------------------------------------------------------- +# Assignment with magic line breaks + +# Magic line break after the left assignment +fn = + value +fn <- + value +fn <<- + value + +# Important that comment3 trails `value` here! +fn <- # comment1 + # comment2 + value # comment3 + +# No magic line break after walrus operator +fn := + value + +# We want these to match, neither support magic line breaks +call(fn := + value) +call(fn = + value) + +# No magic line break after right assignment +fn -> + value +fn ->> + value + +# https://github.com/posit-dev/air/issues/91 +is_condition_true <- + if (condition) { + "yes" + } else { + "no" + } + +# https://github.com/posit-dev/air/issues/91 +base_version <- + version %||% + b_get(brand, "defaults", "shiny", "theme", "version") %||% + b_get(brand, "defaults", "bootstrap", "version") %||% + version_default() diff --git a/crates/air_r_formatter/tests/specs/r/binary_expression.R.snap b/crates/air_r_formatter/tests/specs/r/binary_expression.R.snap index d36c9a76..8b76a13c 100644 --- a/crates/air_r_formatter/tests/specs/r/binary_expression.R.snap +++ b/crates/air_r_formatter/tests/specs/r/binary_expression.R.snap @@ -53,21 +53,6 @@ argument_that_is_quite_long + argument_that_is_quite_long^argument_that_is_quite 1 ^ 2 1 : 2 -# The following assignments should start the LHS/RHS on the same -# line as the operator -fn = function(x) { - x -} -fn <- function(x) { - x -} -fn <<- function(x) { - x -} - -identity(1) -> x -identity(1) ->> x - # ----------------------------------------------------------------------------- # Help specific @@ -460,6 +445,77 @@ ggplot() + geom_bar() %>% identity() +# ----------------------------------------------------------------------------- +# Assignment + +# The following assignments should start the LHS/RHS on the same +# line as the operator +fn = function(x) { + x +} +fn <- function(x) { + x +} +fn <<- function(x) { + x +} + +# Assignment comment tests +fn <- function(x) # comment1 + { # comment2 + x # comment3 + } # comment4 + +identity(1) -> x +identity(1) ->> x + +# ----------------------------------------------------------------------------- +# Assignment with magic line breaks + +# Magic line break after the left assignment +fn = + value +fn <- + value +fn <<- + value + +# Important that comment3 trails `value` here! +fn <- # comment1 + # comment2 + value # comment3 + +# No magic line break after walrus operator +fn := + value + +# We want these to match, neither support magic line breaks +call(fn := + value) +call(fn = + value) + +# No magic line break after right assignment +fn -> + value +fn ->> + value + +# https://github.com/posit-dev/air/issues/91 +is_condition_true <- + if (condition) { + "yes" + } else { + "no" + } + +# https://github.com/posit-dev/air/issues/91 +base_version <- + version %||% + b_get(brand, "defaults", "shiny", "theme", "version") %||% + b_get(brand, "defaults", "bootstrap", "version") %||% + version_default() + ``` @@ -530,21 +586,6 @@ argument_that_is_quite_long + 1^2 1:2 -# The following assignments should start the LHS/RHS on the same -# line as the operator -fn = function(x) { - x -} -fn <- function(x) { - x -} -fn <<- function(x) { - x -} - -identity(1) -> x -identity(1) ->> x - # ----------------------------------------------------------------------------- # Help specific @@ -1055,9 +1096,76 @@ ggplot() + geom_line() + geom_bar() %>% identity() + +# ----------------------------------------------------------------------------- +# Assignment + +# The following assignments should start the LHS/RHS on the same +# line as the operator +fn = function(x) { + x +} +fn <- function(x) { + x +} +fn <<- function(x) { + x +} + +# Assignment comment tests +fn <- function(x) { + # comment1 + # comment2 + x # comment3 +} # comment4 + +identity(1) -> x +identity(1) ->> x + +# ----------------------------------------------------------------------------- +# Assignment with magic line breaks + +# Magic line break after the left assignment +fn = + value +fn <- + value +fn <<- + value + +# Important that comment3 trails `value` here! +fn <- # comment1 + # comment2 + value # comment3 + +# No magic line break after walrus operator +fn := value + +# We want these to match, neither support magic line breaks +call(fn := value) +call(fn = value) + +# No magic line break after right assignment +fn -> value +fn ->> value + +# https://github.com/posit-dev/air/issues/91 +is_condition_true <- + if (condition) { + "yes" + } else { + "no" + } + +# https://github.com/posit-dev/air/issues/91 +base_version <- + version %||% + b_get(brand, "defaults", "shiny", "theme", "version") %||% + b_get(brand, "defaults", "bootstrap", "version") %||% + version_default() ``` # Lines exceeding max width of 80 characters ``` - 229: # TODO: Do we really want these chained? It seems like no good can come of that even though they have same precedence + 214: # TODO: Do we really want these chained? It seems like no good can come of that even though they have same precedence ```