Merge branch 'main' into issue-984

lorenzwalthert · web-flow · commit 44e3c641a9d4 · 2022-10-22T17:25:27.000+02:00
diff --git a/.github/workflows/check-full.yaml b/.github/workflows/check-full.yaml
@@ -1,13 +1,11 @@
 # Workflow derived from https://github.com/r-lib/actions/tree/v2/examples
 # Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
-
 on:
   push:
     branches: [main, master]
   pull_request:
     branches: [main, master]
 
-
 name: R-CMD-check
 
 jobs:
@@ -20,26 +18,27 @@ jobs:
       fail-fast: false
       matrix:
         config:
-          - {os: macOS-latest,   r: 'release'}
+          - {os: macos-latest,   r: 'release'}
 
           - {os: windows-latest, r: 'release'}
           # Use 3.6 to trigger usage of RTools35
           - {os: windows-latest, r: '3.6'}
+          # use 4.1 to check with rtools40's older compiler
+          - {os: windows-latest, r: '4.1'}
 
-          # Use older ubuntu to maximise backward compatibility
-          - {os: ubuntu-latest,  r: 'devel', http-user-agent: 'release'}
-          - {os: ubuntu-latest,  r: 'release'}
-          - {os: ubuntu-latest,  r: 'oldrel-1'}
-          - {os: ubuntu-latest,  r: 'oldrel-2'}
-          - {os: ubuntu-latest,  r: 'oldrel-3'}
-          - {os: ubuntu-latest,  r: 'oldrel-4'}
+          - {os: ubuntu-latest,   r: 'devel', http-user-agent: 'release'}
+          - {os: ubuntu-latest,   r: 'release'}
+          - {os: ubuntu-latest,   r: 'oldrel-1'}
+          - {os: ubuntu-latest,   r: 'oldrel-2'}
+          - {os: ubuntu-latest,   r: 'oldrel-3'}
+          - {os: ubuntu-latest,   r: 'oldrel-4'}
 
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
       R_KEEP_PKG_SOURCE: yes
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - uses: r-lib/actions/setup-pandoc@v2
 
@@ -56,10 +55,7 @@ jobs:
 
       - uses: r-lib/actions/check-r-package@v2
         with:
+          upload-snapshots: true
           error-on: '"note"'
         env:
           _R_CHECK_FORCE_SUGGESTS_: false
-
-concurrency:
-  group: ${{ github.workflow }}-${{ github.head_ref }}
-  cancel-in-progress: true
diff --git a/.github/workflows/pkgdown.yaml b/.github/workflows/pkgdown.yaml
@@ -20,7 +20,7 @@ jobs:
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - uses: r-lib/actions/setup-pandoc@v2
 
@@ -39,7 +39,7 @@ jobs:
 
       - name: Deploy to GitHub pages 🚀
         if: github.event_name != 'pull_request'
-        uses: JamesIves/github-pages-deploy-action@4.1.4
+        uses: JamesIves/github-pages-deploy-action@v4.4.1
         with:
           clean: false
           branch: gh-pages
diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml
@@ -1,50 +1,31 @@
+# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples
+# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
 on:
   push:
-    branches:
-      - main
+    branches: [main, master]
   pull_request:
-    branches:
-      - main
+    branches: [main, master]
 
 name: test-coverage
 
 jobs:
   test-coverage:
-    runs-on: macOS-latest
+    runs-on: ubuntu-latest
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
-    steps:
-      - uses: actions/checkout@v2
-
-      - uses: r-lib/actions/setup-r@master
 
-      - uses: r-lib/actions/setup-pandoc@master
-
-      - name: Query dependencies
-        run: |
-          install.packages('remotes')
-          saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2)
-          writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version")
-        shell: Rscript {0}
+    steps:
+      - uses: actions/checkout@v3
 
-      - name: Cache R packages
-        uses: actions/cache@v1
+      - uses: r-lib/actions/setup-r@v2
         with:
-          path: ${{ env.R_LIBS_USER }}
-          key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }}
-          restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-
+          use-public-rspm: true
 
-      - name: Install dependencies
-        run: |
-          install.packages(c("remotes"))
-          remotes::install_deps(dependencies = TRUE)
-          remotes::install_cran("covr")
-        shell: Rscript {0}
+      - uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          extra-packages: any::covr
+          needs: coverage
 
       - name: Test coverage
-        run: covr::codecov()
+        run: covr::codecov(quiet = FALSE)
         shell: Rscript {0}
-
-concurrency:
-  group: ${{ github.workflow }}-${{ github.head_ref }}
-  cancel-in-progress: true
diff --git a/R/expr-is.R b/R/expr-is.R
@@ -96,7 +96,7 @@ is_subset_expr <- function(pd) {
   if (is.null(pd) || nrow(pd) == 1) {
     return(FALSE)
   }
-  pd$token[2L] == "'['"
+  pd$token[2L] %in% subset_token_opening
 }
 
 
diff --git a/R/indent.R b/R/indent.R
@@ -87,8 +87,15 @@ indent_without_paren_if_else <- function(pd, indent_by) {
 #' example in if-else expressions, this is not the case and indenting
 #' everything between '(' and the penultimate token would result in the wrong
 #' formatting.
+#' @section Handing of `[[`:
+#' Since text `[[` has token `"LBB"` and text `]]` is parsed as two independent
+#' `]` (see 'Examples'), indention has to stop at the first `]`.
+# one token earlier
 #' @importFrom rlang seq2
 #' @keywords internal
+#' @examples
+#' styler:::parse_text("a[1]")
+#' styler:::parse_text("a[[1\n]]")
 compute_indent_indices <- function(pd,
                                    token_opening,
                                    token_closing = NULL) {
@@ -105,7 +112,8 @@ compute_indent_indices <- function(pd,
   if (is.null(token_closing)) {
     stop <- npd
   } else {
-    stop <- last(which(pd$token %in% token_closing)[needs_indention]) - 1
+    offset <- if (any(pd$token == "LBB")) 2L else 1L
+    stop <- last(which(pd$token %in% token_closing)[needs_indention]) - offset
   }
 
   seq2(start, stop)
diff --git a/R/rules-indention.R b/R/rules-indention.R
@@ -4,7 +4,7 @@
 indent_braces <- function(pd, indent_by) {
   indent_indices <- compute_indent_indices(
     pd,
-    token_opening = c("'('", "'['", "'{'"),
+    token_opening = c("'('", "'['", "'{'", "LBB"),
     token_closing = c("')'", "']'", "'}'")
   )
   pd$indent[indent_indices] <- pd$indent[indent_indices] + indent_by
diff --git a/R/rules-line-breaks.R b/R/rules-line-breaks.R
@@ -128,7 +128,7 @@ set_line_break_around_comma_and_or <- function(pd, strict) {
     (pd$token %in% ops) &
       (pd$lag_newlines > 0L) &
       (pd$token_before != "COMMENT") &
-      (lag(pd$token) != "'['")
+      !(lag(pd$token) %in% subset_token_opening)
 
   pd$lag_newlines[comma_with_line_break_that_can_be_removed_before] <- 0L
   pd$lag_newlines[lag(comma_with_line_break_that_can_be_removed_before)] <- 1L
@@ -137,7 +137,7 @@ set_line_break_around_comma_and_or <- function(pd, strict) {
     (pd$token == "EQ_SUB") &
       (pd$lag_newlines > 0L) &
       (pd$token_before != "COMMENT") &
-      (lag(pd$token) != "'['")
+      !(lag(pd$token) %in% subset_token_opening)
   )
 
   pd$lag_newlines[comma_with_line_break_that_can_be_moved_two_tokens_left] <- 0L
@@ -362,7 +362,12 @@ set_line_break_before_closing_call <- function(pd, except_token_before) {
     pd$lag_newlines[setdiff(npd, exception)] <- 0L
     return(pd)
   }
-  pd$lag_newlines[npd] <- 1L
+  idx_non_comment <- previous_non_comment(pd, npd)
+  if (pd$token[idx_non_comment] == "']'") {
+    pd$lag_newlines[idx_non_comment] <- 1L
+  } else {
+    pd$lag_newlines[npd] <- 1L
+  }
   pd
 }
 
diff --git a/R/testing.R b/R/testing.R
@@ -377,3 +377,11 @@ test_transformers_drop <- function(transformers) {
     }
   })
 }
+
+
+skip_during_parallel <- function() {
+  Sys.getenv("STYLER_TEST_IS_TRULY_PARALLEL", TRUE) %>%
+    toupper() %>%
+    as.logical() %>%
+    testthat::skip_if()
+}
diff --git a/R/token-define.R b/R/token-define.R
@@ -70,3 +70,5 @@ op_token <- c(
   "EQ_SUB", "ELSE", "IN",
   "EQ_FORMALS"
 )
+
+subset_token_opening <- c("'['", "LBB")
diff --git a/man/compute_indent_indices.Rd b/man/compute_indent_indices.Rd
diff --git a/tests/testthat.R b/tests/testthat.R
@@ -1,4 +1,8 @@
 library(testthat)
 library(styler)
+test_check("styler") # checks multiple files, in parallel
 
-test_check("styler")
+# checks file one by one, not parallel
+Sys.setenv(STYLER_TEST_IS_TRULY_PARALLEL = FALSE)
+test_file("testthat/test-cache-high-level-api.R")
+test_file("testthat/tests-cache-require-serial.R")
diff --git a/tests/testthat/indention_square_brackets/square_brackets_double_line_break-in.R b/tests/testthat/indention_square_brackets/square_brackets_double_line_break-in.R
@@ -0,0 +1,22 @@
+a[[b]]
+
+
+a[[
+  2
+]
+]
+
+a[[
+  2
+]]
+
+
+a[[
+2
+  ]]
+
+
+a[[
+  2
+] #
+]
diff --git a/tests/testthat/test-cache-high-level-api.R b/tests/testthat/test-cache-high-level-api.R
@@ -2,6 +2,7 @@ test_that("activated cache brings speedup on style_file() API", {
   local_test_setup()
   skip_on_cran()
   skip_on_covr()
+  skip_during_parallel()
   n <- n_times_faster_with_cache(
     test_path("reference-objects/caching.R"),
     test_path("reference-objects/caching.R"),
@@ -27,6 +28,7 @@ text <- c(
 test_that("activated cache brings speedup on style_text() API on character vector", {
   skip_on_cran()
   skip_on_covr()
+  skip_during_parallel()
   n <- n_times_faster_with_cache(
     text, text,
     fun = style_text
@@ -37,6 +39,7 @@ test_that("activated cache brings speedup on style_text() API on character vecto
 test_that("activated cache brings speedup on style_text() API on character scalar", {
   skip_on_cran()
   skip_on_covr()
+  skip_during_parallel()
   text2 <- paste0(text, collapse = "\n")
 
   n <- n_times_faster_with_cache(
@@ -56,6 +59,7 @@ test_that("trailing line breaks are ignored for caching", {
   expect_equal(cache_info(format = "tabular")$n, 3)
   skip_on_cran()
   skip_on_covr()
+  skip_during_parallel()
   n <- n_times_faster_with_cache(text1, text2)
   expect_gt(n, 55)
 })
@@ -69,6 +73,7 @@ test_that("trailing line breaks are ignored for caching in one scalar", {
   expect_equal(cache_info(format = "tabular")$n, 3)
   skip_on_cran()
   skip_on_covr()
+  skip_during_parallel()
   n <- n_times_faster_with_cache(text1, text2)
   expect_gt(n, 55)
 })
@@ -85,14 +90,15 @@ test_that("trailing line breaks are ignored for caching in one scalar", {
   expect_equal(cache_info(format = "tabular")$n, 3)
   skip_on_cran()
   skip_on_covr()
+  skip_during_parallel()
   n <- n_times_faster_with_cache(text1, text2)
   expect_gt(n, 55)
 })
 
 test_that("speedup higher when cached roxygen example code is multiple expressions", {
   skip_on_cran()
   skip_on_covr()
-
+  skip_during_parallel()
   text_long <- c(
     "#' Roxygen",
     "#' Comment",
@@ -132,6 +138,7 @@ test_that("speedup higher when cached roxygen example code is multiple expressio
 test_that("no speedup when tranformer changes", {
   skip_on_cran()
   skip_on_covr()
+  skip_during_parallel()
   local_test_setup()
   t1 <- tidyverse_style()
   first <- system.time(style_text(text, transformers = t1))
@@ -145,6 +152,7 @@ test_that("unactivated cache does not bring speedup", {
   skip_on_cran()
   skip_on_covr()
   local_test_setup()
+  skip_during_parallel()
   first <- system.time(style_file(test_path("reference-objects/caching.R")))
   second <- system.time(style_file(test_path("reference-objects/caching.R")))
   expect_false(first["elapsed"] / 4 > second["elapsed"])
diff --git a/tests/testthat/test-cache-interaction-roxygen-code-examples.R b/tests/testthat/test-cache-interaction-roxygen-code-examples.R
@@ -42,18 +42,6 @@ test_that("roxzgen code examples are written to cache as both individual express
   )
 })
 
-
-test_that("roxzgen code examples are written to cache as whole expressions bring speedgain", {
-  skip_on_cran()
-  local_test_setup(cache = TRUE)
-  text <- readLines(test_path("cache-with-r-cache/roxygen-cache-1.R"))
-  first <- system.time(styled <- style_text(text))
-  # don't use full cache, only roxygen cache
-  styled[1] <- "#' This is a nother text"
-  second <- system.time(style_text(styled))
-  expect_gt(first["elapsed"], 4 * second["elapsed"])
-})
-
 test_that("cache is deactivated at end of caching related testthat file", {
   expect_false(cache_is_activated())
 })
diff --git a/tests/testthat/test-cache-with-r-cache.R b/tests/testthat/test-cache-with-r-cache.R
diff --git a/tests/testthat/test-square_brackets.R b/tests/testthat/test-square_brackets.R
diff --git a/tests/testthat/tests-cache-require-serial.R b/tests/testthat/tests-cache-require-serial.R
diff --git a/vignettes/styler.Rmd b/vignettes/styler.Rmd

Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,7 @@ is_subset_expr <- function(pd) {`
`96`	`96`	`if (is.null(pd) \|\| nrow(pd) == 1) {`
`97`	`97`	`return(FALSE)`
`98`	`98`	`}`
`99`		`- pd$token[2L] == "'['"`
	`99`	`+ pd$token[2L] %in% subset_token_opening`
`100`	`100`	`}`
`101`	`101`
`102`	`102`
Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,7 @@`
`4`	`4`	`indent_braces <- function(pd, indent_by) {`
`5`	`5`	`indent_indices <- compute_indent_indices(`
`6`	`6`	`pd,`
`7`		`- token_opening = c("'('", "'['", "'{'"),`
	`7`	`+ token_opening = c("'('", "'['", "'{'", "LBB"),`
`8`	`8`	`token_closing = c("')'", "']'", "'}'")`
`9`	`9`	`)`
`10`	`10`	`pd$indent[indent_indices] <- pd$indent[indent_indices] + indent_by`
Original file line number	Diff line number	Diff line change
`@@ -377,3 +377,11 @@ test_transformers_drop <- function(transformers) {`
`377`	`377`	`}`
`378`	`378`	`})`
`379`	`379`	`}`
	`380`	`+`
	`381`	`+`
	`382`	`+skip_during_parallel <- function() {`
	`383`	`+ Sys.getenv("STYLER_TEST_IS_TRULY_PARALLEL", TRUE) %>%`
	`384`	`+ toupper() %>%`
	`385`	`+ as.logical() %>%`
	`386`	`+ testthat::skip_if()`
	`387`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -70,3 +70,5 @@ op_token <- c(`
`70`	`70`	`"EQ_SUB", "ELSE", "IN",`
`71`	`71`	`"EQ_FORMALS"`
`72`	`72`	`)`
	`73`	`+`
	`74`	`+subset_token_opening <- c("'['", "LBB")`