Merge pull request #647 from lorenzwalthert/issue-646

Invalidate cache on style guide argument change

Author: lorenzwalthert

API

@@ -6,7 +6,7 @@ cache_activate(cache_name = NULL, verbose = TRUE)
 cache_clear(cache_name = NULL, ask = TRUE)
 cache_deactivate(verbose = TRUE)
 cache_info(cache_name = NULL, format = "both")
-create_style_guide(initialize = default_style_guide_attributes, line_break = NULL, space = NULL, token = NULL, indention = NULL, use_raw_indention = FALSE, reindention = tidyverse_reindention(), style_guide_name = NULL, style_guide_version = NULL)
+create_style_guide(initialize = default_style_guide_attributes, line_break = NULL, space = NULL, token = NULL, indention = NULL, use_raw_indention = FALSE, reindention = tidyverse_reindention(), style_guide_name = NULL, style_guide_version = NULL, more_specs = NULL)
 default_style_guide_attributes(pd_flat)
 specify_math_token_spacing(zero = "'^'", one = c("'+'", "'-'", "'*'", "'/'"))
 specify_reindention(regex_pattern = NULL, indention = 0, comments_only = TRUE)

R/style-guides.R

@@ -57,6 +57,7 @@ tidyverse_style <- function(scope = "tokens",
                             start_comments_with_one_space = FALSE,
                             reindention = tidyverse_reindention(),
                             math_token_spacing = tidyverse_math_token_spacing()) {
+  args <- as.list(environment())
   scope <- character_to_ordered(
     scope,
     c("none", "spaces", "indention", "line_breaks", "tokens")
@@ -175,7 +176,8 @@ tidyverse_style <- function(scope = "tokens",
     use_raw_indention   =              use_raw_indention,
     reindention         =                    reindention,
     style_guide_name    =               style_guide_name,
-    style_guide_version =                 styler_version
+    style_guide_version =                 styler_version,
+    more_specs          =                           args
   )
 }
 
@@ -184,8 +186,12 @@ tidyverse_style <- function(scope = "tokens",
 #' This is a helper function to create a style guide, which is technically
 #' speaking a named list of groups of transformer functions where each
 #' transformer function corresponds to one styling rule. The output of this
-#' function can be used as an argument for \code{style} in top level functions
-#' like [style_text()] and friends.
+#' function can be used as an argument for `style` in top level functions
+#' like [style_text()] and friends. Note that for caching to work properly,
+#' unquote all inputs to the transformer function if possible with rlang's `!!`,
+#' otherwise, they will be passed as references (generic variable names) instead
+#' of literals and `styler:::is_cached()` won't pick up changes. See how it's
+#' done in [tidyverse_style()] with `indent_by` and other arguments.
 #' @param initialize The bare name of a function that initializes various
 #'   variables on each level of nesting.
 #' @param line_break A list of transformer functions that manipulate line_break
@@ -207,6 +213,12 @@ tidyverse_style <- function(scope = "tokens",
 #'   attribute inside the created style guide, for example for caching. This
 #'   should correspond to the version of the R package that exports the
 #'   style guide.
+#' @param more_specs Named vector (coercible to character) specifying arguments
+#'   `args` in `transformer <- list(t1 = purrr::partial(f, arg)` because when
+#'   such functions are converted to characters in [styler::cache_make_key()],
+#'   they will yield generic code and we loose the specific value of `arg` (see
+#'   [styler::cache_make_key()]), even when unquoting these inputs with `!!`
+#'   beforehand in `purrr::partial()`.
 #' @examples
 #' set_line_break_before_curly_opening <- function(pd_flat) {
 #'   op <- pd_flat$token %in% "'{'"
@@ -232,7 +244,8 @@ create_style_guide <- function(initialize = default_style_guide_attributes,
                                use_raw_indention = FALSE,
                                reindention = tidyverse_reindention(),
                                style_guide_name = NULL,
-                               style_guide_version = NULL) {
+                               style_guide_version = NULL,
+                               more_specs = NULL) {
   lst(
     # transformer functions
     initialize = lst(initialize),
@@ -244,7 +257,8 @@ create_style_guide <- function(initialize = default_style_guide_attributes,
     use_raw_indention,
     reindention,
     style_guide_name,
-    style_guide_version
+    style_guide_version,
+    more_specs
   ) %>%
     map(compact)
 }

R/testing-public-api.R

@@ -51,9 +51,9 @@ test_dry <- function(path, styler, styled = FALSE) {
   testthat::expect_true(identical(before, readLines(path)))
 
   if (styled) {
-    expect_error(styler(path, dry = "fail"), NA)
+    testthat::expect_error(styler(path, dry = "fail"), NA)
   } else {
-    expect_error(styler(path, dry = "fail"), "would be modified")
+    testthat::expect_error(styler(path, dry = "fail"), "would be modified")
   }
-  expect_error(styler(path, dry = "other option"), "one of")
+  testthat::expect_error(styler(path, dry = "other option"), "one of")
 }

R/utils-cache.R

@@ -52,6 +52,10 @@ is_cached <- function(text, transformers, cache_dir = cache_dir_default()) {
 #'   is to replace a transformer with the same function body (but changing
 #'   the function definition of the functions called in that body) interactively
 #'   without changing version number of name at the same time.
+#'   Remaining problem: `purrr::partial()` calls will render generic code, e.g.
+#'   see `as.character(list(purrr::partial(sum, x = 4)))`. For that reason,
+#'   all arguments passed to a `purrr::partial()` call must be put in the
+#'   style guide under `more_specs`.
 #' @section Experiments:
 #'
 #' There is unexplainable behavior in conjunction with hashing and
@@ -87,7 +91,9 @@ cache_make_key <- function(text, transformers) {
     text = hash_standardize(text),
     style_guide_name = transformers$style_guide_name,
     style_guide_version = transformers$style_guide_version,
-    style_guide_text = as.character(transformers)
+    style_guide_text = as.character(transformers),
+    more_specs = as.character(transformers$more_specs) %>%
+      set_names(names(transformers$more_specs))
   )
 }
 

inst/WORDLIST

@@ -12,6 +12,7 @@ cancelling
 chnages
 CMD
 codecov
+coercible
 compat
 config
 CONST
@@ -28,6 +29,7 @@ dplyr
 DSLs
 emacs
 emph
+env
 EOF
 EOL
 EOLs
@@ -38,10 +40,12 @@ fileext
 filetype
 forcond
 funct
+getChecksum
 getOption
 github
 gitsum
 GSOC
+href
 http
 https
 infinitively
@@ -51,13 +55,15 @@ interaces
 invasiveness
 io
 ixmypi
+ized
 Kirill
 knitr
 labelled
 levelName
 LF
 lifecycle
 lorenzwalthert
+lst
 macOS
 md
 Müller
@@ -75,6 +81,8 @@ precommit
 prefill
 prettycode
 PRs
+purrr
+purrrpartial
 RcppExports
 rds
 readme
@@ -88,6 +96,7 @@ renv
 repo
 reprex
 rlang
+rlang's
 rmarkdown
 RMarkdown
 rmd
@@ -105,6 +114,7 @@ RStudio
 RStudio's
 saamwerk
 seealso
+stackoverflow
 StackOverflow
 STR
 styler

man/cache_make_key.Rd

@@ -117,9 +117,9 @@ capture.output(test_that("no speedup when tranformer changes", {
   fresh_testthat_cache()
   t1 <- tidyverse_style()
   first <- system.time(style_text(text, transformers = t1))
-  t1$use_raw_indention <- !t1$use_raw_indention
+  t1 <- tidyverse_style(indent_by = 4)
   second <- system.time(style_text(text, transformers = t1))
-  expect_false(first["elapsed"] / 2 > second["elapsed"])
+  expect_false(first["elapsed"] / 1.2 > second["elapsed"])
 }))