Merge pull request #816 from lorenzwalthert/issue-815

- Avoid adding braces to conditional expression in pipe (#816).

Author: lorenzwalthert

NEWS.md

@@ -54,6 +54,9 @@
 
 ## Minor changes and fixes
 
+* No curly braces are added to else statements if they are within a pipe, as 
+  this can change evaluation logic of code involving the magrittr dot in rare 
+  cases (#816).
 * line breaks between `}` and `else` are removed (#793).
 * in function calls, code after `= #\n` is indented correctly (#814).
 * styler won't format code chunks with explicit `tidy = FALSE` in an Rmd or Rnw 

R/nest.R

@@ -302,16 +302,6 @@ add_attributes_caching <- function(pd_flat, transformers, more_specs) {
   pd_flat
 }
 
-#' @describeIn add_token_terminal Removes column `terimnal_token_before`. Might
-#'   be used to prevent the use of invalidated information, e.g. if tokens were
-#'   added to the nested parse table.
-#' @keywords internal
-remove_terminal_token_before_and_after <- function(pd_flat) {
-  pd_flat$token_before <- NULL
-  pd_flat$token_after <- NULL
-  pd_flat
-}
-
 #' Helper for setting spaces
 #'
 #' @param spaces_after_prefix An integer vector with the number of spaces

R/rules-tokens.R

@@ -113,7 +113,8 @@ wrap_else_multiline_curly <- function(pd, indent_by = 2, space_after = 0) {
   if (contains_else_expr(pd) &&
     pd_is_multi_line(pd) &&
     contains_else_expr_that_needs_braces(pd) &&
-    !any(pd$stylerignore)) {
+    !any(pd$stylerignore) &&
+    pd$token_before[1] != "SPECIAL-PIPE") {
     else_idx <- which(pd$token == "ELSE")
     pd$spaces[else_idx] <- 1L
     all_to_be_wrapped_ind <- seq2(else_idx + 1L, nrow(pd))

R/style-guides.R

@@ -168,7 +168,6 @@ tidyverse_style <- function(scope = "tokens",
       force_assignment_op = force_assignment_op,
       resolve_semicolon = resolve_semicolon,
       add_brackets_in_pipe = add_brackets_in_pipe,
-      remove_terminal_token_before_and_after = remove_terminal_token_before_and_after,
       wrap_if_else_while_for_fun_multi_line_in_curly =
         if (strict) wrap_if_else_while_for_fun_multi_line_in_curly
     )

man/add_token_terminal.Rd

@@ -36,3 +36,10 @@ test_that("braces only added to pipe if RHS is a symbol", {
     transformer = style_text
   ), NA)
 })
+
+
+test_that("No braces are added if conditional statement is within pipe", {
+  expect_warning(test_collection("token_adding_removing", "else-pipe",
+    transformer = style_text
+  ), NA)
+})

tests/testthat/test-token_adding_removing.R

@@ -85,8 +85,7 @@ test_that("tidyverse transformers are correctly dropped", {
 
   names_tokens <- c(
     "fix_quotes",
-    if (getRversion() < 3.6) "force_assignment_op",
-    "remove_terminal_token_before_and_after"
+    if (getRversion() < 3.6) "force_assignment_op"
   )
   expect_setequal(names(t_fun$token), names_tokens)
 })

tests/testthat/test-transformers-drop.R

@@ -0,0 +1,9 @@
+mtcars %>%
+  mutate(
+    x = 1
+  ) %>%
+  if (FALSE) {
+    mutate(., country = 2)
+  } else .
+
+# adding braces around . in else changes evaluation

tests/testthat/token_adding_removing/else-pipe-in.R

@@ -0,0 +1,9 @@
+mtcars %>%
+  mutate(
+    x = 1
+  ) %>%
+  if (FALSE) {
+    mutate(., country = 2)
+  } else .
+
+# adding braces around . in else changes evaluation