Merge pull request #891 from lorenzwalthert/issue-890

Rules that create tokens must be stylerignore aware

Author: lorenzwalthert

NEWS.md

@@ -27,6 +27,7 @@
 
 * Piped function without brackets `substitute(x %>% y)` don't get `()` added
   anymore, as this can change outcome of the code (#876).
+* rules that add tokens don't break stylerignore sequences anymore (#891).
 * Add vignette on distributing style guide (#846, #861).
 * Alignment detection respects stylerignore (#850).
 * `Warning: Unknown or uninitialised column:` was fixed (#885).
@@ -35,9 +36,9 @@
 * ensure a trailing blank line also if the input is cached (#867).
 * Preserve trailing blank line in roxygen examples to simplify concatenation of
   examples (#880).
-* Fix argument name `filetype` in Example for `style_dir()` (#855).
 * An error is now thrown on styling if input unicode characters can't be 
   correctly parsed for Windows and R < 4.2 (#883).
+* Fix argument name `filetype` in Example for `style_dir()` (#855).
 
 
 **Infrastructure**

R/detect-alignment-utils.R

@@ -61,6 +61,8 @@ alignment_ensure_trailing_comma <- function(pd_by_line) {
       lag_newlines = 0L,
       spaces = 0L,
       pos_ids = NA,
+      stylerignore = last_pd$stylerignore[1],
+      indents = last_pd$indent[1]
     )
     tokens$.lag_spaces <- 0
 

R/relevel.R

@@ -96,7 +96,9 @@ wrap_expr_in_expr <- function(pd) {
     "expr", "",
     pos_ids = create_pos_ids(pd, 1, after = FALSE),
     child = pd,
-    terminal = FALSE
+    terminal = FALSE,
+    stylerignore = pd$stylerignore[1],
+    indents = pd$indent[1]
   )
 }
 
@@ -209,6 +211,8 @@ find_block_id <- function(pd) {
 relocate_eq_assign_one <- function(pd) {
   idx_eq_assign <- which(pd$token == "EQ_ASSIGN")
   eq_ind <- seq2(idx_eq_assign[1] - 1L, last(idx_eq_assign) + 1L)
+  # initialize because wrap_expr_in_expr -> create_tokens -> requires it
+  pd$indent <- 0
   eq_expr <- pd[eq_ind, ] %>%
     wrap_expr_in_expr() %>%
     add_line_col_to_wrapped_expr() %>%
@@ -218,6 +222,7 @@ relocate_eq_assign_one <- function(pd) {
     ))
   eq_expr$id <- NA
   eq_expr$parent <- NA
+  pd$indent <- NULL
   non_eq_expr <- pd[-eq_ind, ]
   pd <- bind_rows(eq_expr, non_eq_expr) %>%
     arrange_pos_id()

R/rules-tokens.R

@@ -35,10 +35,21 @@ add_brackets_in_pipe_one <- function(pd, pos) {
     child <- pd$child[[next_non_comment]]
     new_pos_ids <- create_pos_ids(child, 1, after = TRUE, n = 2)
     new_pd <- create_tokens(
-      tokens = c("'('", "')'"),
       texts = c("(", ")"),
+      lag_newlines = rep(0L, 2),
+      spaces = 0,
       pos_ids = new_pos_ids,
-      lag_newlines = rep(0L, 2)
+      token_before = c(child$token[1], "'('"),
+      token_after = c("')'", child$token_after[1]),
+      indention_ref_pos_ids = NA,
+      indents = child$indent[1],
+      tokens = c("'('", "')'"),
+      terminal = TRUE,
+      child = NULL,
+      stylerignore = child$stylerignore[1],
+      # block???
+      block = NA,
+      is_cached = FALSE
     )
     pd$child[[next_non_comment]] <- bind_rows(
       pd$child[[next_non_comment]],

R/token-create.R

@@ -19,7 +19,7 @@
 #'   not.
 #' @param child The children of the tokens.
 #' @param stylerignore Boolean to indicate if the line should be ignored by
-#'   styler.
+#'   styler. Must take value from token before, can't have a default.
 #' @param block The block (of caching) to which the token belongs. An integer.
 #' @param is_cached Whether the token is cached already.
 #' @family token creators
@@ -32,10 +32,10 @@ create_tokens <- function(tokens,
                           token_before = NA,
                           token_after = NA,
                           indention_ref_pos_ids = NA,
-                          indents = 0L,
+                          indents,
                           terminal = TRUE,
                           child = NULL,
-                          stylerignore = FALSE,
+                          stylerignore,
                           block = NA,
                           is_cached = FALSE) {
   len_text <- length(texts)
@@ -149,14 +149,16 @@ wrap_expr_in_curly <- function(pd,
   opening <- create_tokens("'{'", "{",
     pos_ids = create_pos_ids(pd, 1, after = FALSE),
     spaces = 1 - as.integer(stretch_out[1]),
-    stylerignore = pd$stylerignore[1]
+    stylerignore = pd$stylerignore[1],
+    indents = pd$indent[1]
   )
 
   closing <- create_tokens(
     "'}'", "}",
     spaces = space_after, lag_newlines = as.integer(stretch_out[2]),
     pos_ids = create_pos_ids(pd, nrow(pd), after = TRUE),
-    stylerignore = pd$stylerignore[1]
+    stylerignore = pd$stylerignore[1],
+    indents = pd$indent[1]
   )
 
   bind_rows(opening, pd, closing) %>%

man/create_tokens.Rd

@@ -1,3 +1,3 @@
+*.dll
 *.o
 *.so
-*.dll

tests/testthat/public-api/xyzpackage-rmd/src/.gitignore

@@ -1,3 +1,3 @@
+*.dll
 *.o
 *.so
-*.dll

tests/testthat/public-api/xyzpackage-rnw/src/.gitignore

@@ -9,17 +9,20 @@ test_that("can create a token that has relevant columns", {
   )
 
   expect_equal(
-    names(create_tokens("'{'", "{", pos_ids = 3)),
+    names(create_tokens("'{'", "{", pos_ids = 3, stylerignore = FALSE, indents = 0)),
     pd_names
   )
 })
 
 test_that("pos_id can be created", {
-  pd <- create_tokens("XZY_TEST", "test", pos_ids = 3)
+  pd <- create_tokens("XZY_TEST", "test", pos_ids = 3, stylerignore = FALSE, indents = 0)
   new_id <- create_pos_ids(pd, 1L, by = 0.4)
   expect_error(
     bind_rows(
-      create_tokens("XZY_TEST", "test", pos_ids = new_id),
+      create_tokens("XZY_TEST", "test",
+        pos_ids = new_id,
+        stylerignore = FALSE, indents = 0
+      ),
       pd
     ),
     NA
@@ -28,21 +31,30 @@ test_that("pos_id can be created", {
 
 
 test_that("unambiguous pos_id won't be created (down)", {
-  pd <- create_tokens("XZY_TEST", "test", pos_ids = 3)
+  pd <- create_tokens("XZY_TEST", "test",
+    pos_ids = 3,
+    stylerignore = FALSE, indents = 0
+  )
   new_id <- create_pos_ids(pd, 1L, by = 0.4)
   pd <- bind_rows(
-    create_tokens("XZY_TEST", "test", pos_ids = new_id),
+    create_tokens("XZY_TEST", "test",
+      pos_ids = new_id,
+      stylerignore = FALSE, indents = 0
+    ),
     pd
   )
   expect_error(create_pos_id(pd, 1L, by = 0.4))
 })
 
 test_that("unambiguous pos_id won't be created (up)", {
-  pd <- create_tokens("XZY_TEST", "test", pos_ids = 3)
+  pd <- create_tokens("XZY_TEST", "test",
+    pos_ids = 3,
+    stylerignore = FALSE, indents = 0
+  )
   new_id <- create_pos_ids(pd, 1L, by = 0.4, after = TRUE)
 
   pd <- bind_rows(
-    create_tokens("XZY_TEST", "test", pos_ids = new_id),
+    create_tokens("XZY_TEST", "test", pos_ids = new_id, stylerignore = FALSE, indents = 0),
     pd
   )
   expect_error(create_pos_id(pd, 1L, by = 0.4, after = TRUE))

tests/testthat/test-create_token.R

@@ -38,6 +38,7 @@ test_that("braces only added to pipe if RHS is a symbol", {
 })
 
 
+
 test_that("No braces are added if conditional statement is within pipe", {
   expect_warning(test_collection("token_adding_removing", "else-pipe",
     transformer = style_text
@@ -49,3 +50,16 @@ test_that("No brace is added within `substitute()`", {
     transformer = style_text
   ), NA)
 })
+
+
+test_that("stylreignore interacts correctly with wrap_expr_in_curly", {
+  expect_warning(test_collection("token_adding_removing", "if_else_stylerignore",
+    transformer = style_text
+  ), NA)
+})
+
+test_that("stylreignore interacts correctly with wrap_expr_in_curly", {
+  expect_warning(test_collection("token_adding_removing", "for_while_stylerignore",
+    transformer = style_text
+  ), NA)
+})