make R.cache an import

lorenzwalthert · lorenzwalthert · commit 7332ac53c303 · 2020-02-10T21:06:04.000+01:00
This remove one step in the styler setup for caching. Now, the only thing an interactive user has to do is to allow R.cache to set up a permanent cache. I believe this is worth the three extra recursive dependencies because there is actually no reason why you are strictly against using caching, even on CI/CD it will speed up things.
diff --git a/.travis.yml b/.travis.yml
@@ -38,9 +38,6 @@ matrix:
   - r: release
     env:
     - BUILD_PKGDOWN: true
-  - r: release
-    env:
-    - R_REMOVE_RCACHE: true
   - r: devel
 
 #env
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -21,6 +21,7 @@ Imports:
     cli (>= 1.1.0),
     magrittr (>= 1.0.1),
     purrr (>= 0.2.3),
+    R.cache (>= 0.14.0),
     rematch2 (>= 2.0.1),
     rlang (>= 0.1.1),
     rprojroot (>= 1.1),
@@ -35,7 +36,6 @@ Suggests:
     here,
     knitr,
     prettycode,
-    R.cache (>= 0.14.0),
     rmarkdown,
     rstudioapi (>= 0.7),
     testthat (>= 2.1.0)
diff --git a/R/communicate.R b/R/communicate.R
@@ -34,32 +34,3 @@ assert_data.tree_installation <- function() {
     abort("The package data.tree needs to be installed for this functionality.")
   }
 }
-
-#' Assert the R.cache installation in conjunction with the cache config
-#'
-#' R.cache needs to be installed if caching functionality is enabled
-#' @keywords internal
-assert_R.cache_installation <- function(action = "abort") {
-  # fail if R.cache is not installed but feature is actiavted.
-  if (!rlang::is_installed("R.cache")) {
-    msg_basic <- paste(
-      "R package R.cache is not installed, which is needed when the caching ",
-      "feature is activated. Please install the package with ",
-      "`install.packages('R.cache')` and then restart R to enable the ",
-      "caching feature of styler or permanently deactivate the feature by ",
-      "adding `styler::cache_deactivate()` to your .Rprofile, e.g. via ",
-      "`usethis::edit_r_profile()`.",
-      sep = ""
-    )
-
-    if (action == "abort") {
-      rlang::abort(msg_basic)
-    } else {
-      rlang::warn(paste0(
-        msg_basic, " ",
-        "Deactivating the caching feature for the current session."
-      ))
-      cache_deactivate(verbose = FALSE)
-    }
-  }
-}
diff --git a/R/transform-files.R b/R/transform-files.R
@@ -78,10 +78,9 @@ make_transformer <- function(transformers,
                              warn_empty = TRUE) {
   force(transformers)
   assert_transformers(transformers)
-  if (cache_is_activated()) assert_R.cache_installation(action = "warn")
 
   function(text) {
-    should_use_cache <- cache_is_activated() && rlang::is_installed("R.cache")
+    should_use_cache <- cache_is_activated()
 
     if (should_use_cache) {
       use_cache <- is_cached(text, transformers)
diff --git a/R/ui-caching.R b/R/ui-caching.R
@@ -14,7 +14,6 @@
 #' @family cache managers
 #' @export
 cache_clear <- function(cache_name = NULL, ask = TRUE) {
-  assert_R.cache_installation()
   path_cache <- cache_find_path(cache_name)
   R.cache::clearCache(path_cache, prompt = ask)
   cache_deactivate(verbose = FALSE)
@@ -26,20 +25,23 @@ cache_clear <- function(cache_name = NULL, ask = TRUE) {
 #' Caching makes styler faster on repeated styling. It does not cache input
 #' but output code. That means if you style code that already complies to a
 #' style guide and you have previously styled that code, it will be quicker.
-#' Code is cached by expression and the cache is shared across all APIs.
+#' Code is cached by expression and the cache is shared across all APIs (e.g.
+#' `style_text()` and Addin.
 #'
 #' @section Setup:
-#' styler by default uses caching. It may prompt you to install the R package
-#' `R.cache` the first time you want to use it. R.cache will also ask you to let
-#' it create a permanent cache on your file system that styler will use.
+#' styler by default uses caching, via the `{R.cache}` package. You will be
+#' asked you to let it create a permanent cache on your file system that styler
+#' will use in case you have not set that up already for another tool that
+#' uses `{R.cache}`.
 #' This is needed if you want to cache across R sessions and not just within.
 #'
 #' @section Non-interactive use:
 #' Note that if you have never authorized `{R.cache}` to create the cache in a
 #' permenent directory, it wil build the cache in a temporary directory. To
 #' create a permenent cache, just open an interactive R session and type
 #' `cache_info()`. You can see under `Location:` if a permanent directory is
-#' used and if not, `{R.cache}` will ask you to create one.
+#' used and if not, `{R.cache}` will ask you to create one the first time you
+#' use `{R.cache}` in an R session.
 #'
 #' @section Invalidation:
 #' The cache is specific to a version of styler by default, because different
@@ -77,7 +79,6 @@ NULL
 #' @family cache managers
 #' @export
 cache_info <- function(cache_name = NULL, format = "both") {
-  assert_R.cache_installation()
   rlang::arg_match(format, c("tabular", "lucid", "both"))
   path_cache <- cache_find_path(cache_name)
   files <- list.files(path_cache, full.names = TRUE)
@@ -119,7 +120,6 @@ cache_info <- function(cache_name = NULL, format = "both") {
 #' @family cache managers
 #' @export
 cache_activate <- function(cache_name = NULL, verbose = TRUE) {
-  assert_R.cache_installation()
   if (!is.null(cache_name)) {
     options("styler.cache_name" = cache_name)
   } else {
diff --git a/man/assert_R.cache_installation.Rd b/man/assert_R.cache_installation.Rd
diff --git a/man/caching.Rd b/man/caching.Rd
diff --git a/tests/testthat/test-cache-high-level-api.R b/tests/testthat/test-cache-high-level-api.R
@@ -1,5 +1,4 @@
 capture.output(test_that("activated cache brings speedup on style_file() API", {
-  skip_if_not_installed("R.cache")
   on.exit(clear_testthat_cache())
   clear_testthat_cache()
   activate_testthat_cache()
@@ -25,7 +24,7 @@ text <- c(
   rep(10)
 
 capture.output(test_that("activated cache brings speedup on style_text() API on character vector", {
-  skip_if_not_installed("R.cache")
+
   activate_testthat_cache()
   on.exit(clear_testthat_cache())
   clear_testthat_cache()
@@ -37,7 +36,7 @@ capture.output(test_that("activated cache brings speedup on style_text() API on
 }))
 
 capture.output(test_that("activated cache brings speedup on style_text() API on character scalar", {
-  skip_if_not_installed("R.cache")
+
   activate_testthat_cache()
   on.exit(clear_testthat_cache())
   clear_testthat_cache()
@@ -50,7 +49,7 @@ capture.output(test_that("activated cache brings speedup on style_text() API on
 
 
 capture.output(test_that("no speedup when tranformer changes", {
-  skip_if_not_installed("R.cache")
+
   activate_testthat_cache()
   on.exit(clear_testthat_cache())
   clear_testthat_cache()
@@ -68,7 +67,6 @@ capture.output(
     "activated cache brings speedup on style_text() API on ",
     "character scalar and character vector (mixed)"
   ), {
-    skip_if_not_installed("R.cache")
     activate_testthat_cache()
     on.exit(clear_testthat_cache())
     clear_testthat_cache()
@@ -81,7 +79,7 @@ capture.output(
 
 
 capture.output(test_that("unactivated cache does not bring speedup", {
-  skip_if_not_installed("R.cache")
+
   on.exit(clear_testthat_cache())
   clear_testthat_cache()
   cache_deactivate()
@@ -93,7 +91,6 @@ capture.output(test_that("unactivated cache does not bring speedup", {
 
 capture.output(test_that("avoid deleting comments #584 (see commit messages)", {
 
-  skip_if_not_installed("R.cache")
   on.exit(clear_testthat_cache())
   clear_testthat_cache()
   activate_testthat_cache()
@@ -119,7 +116,6 @@ capture.output(test_that("avoid deleting comments #584 (see commit messages)", {
 
 capture.output(test_that("avoid removing roxygen mask (see commit messages in #584)", {
 
-  skip_if_not_installed("R.cache")
   on.exit(clear_testthat_cache())
   clear_testthat_cache()
   activate_testthat_cache()
diff --git a/tests/testthat/test-cache-with-r-cache.R b/tests/testthat/test-cache-with-r-cache.R
@@ -1,22 +1,10 @@
-
-capture.output(test_that("No warnings are issued when R.cache is installed", {
-  skip_if_not_installed("R.cache")
-  on.exit(clear_testthat_cache())
-  expect_silent(assert_R.cache_installation())
-  expect_warning(style_text("1+1"), NA)
-  expect_warning(activate_testthat_cache(), NA)
-  expect_warning(style_text("1+1"), NA)
-  expect_silent(assert_R.cache_installation())
-}))
-
-capture.output(test_that("Cache management works when R.cache is installed", {
-  skip_if_not_installed("R.cache")
+capture.output(test_that("Cache management works", {
   on.exit(clear_testthat_cache())
   clear_testthat_cache()
   # clearing a cache inactivates the caching functionality.
   expect_false(cache_info(format = "tabular")$activated)
   activate_testthat_cache()
-  # at fresh startup, with R.cache installed
+  # at fresh startup
   expect_s3_class(cache_info(format = "tabular"), "tbl_df")
   expect_error(cache_info(), NA)
   expect_equal(basename(cache_activate()), styler_version)
diff --git a/tests/testthat/test-cache-without-r-cache.R b/tests/testthat/test-cache-without-r-cache.R
diff --git a/tic.R b/tic.R
@@ -1,11 +1,3 @@
-get_stage("after_install") %>%
-  add_code_step(
-    {
-      if (isTRUE(as.logical(toupper(Sys.getenv("R_REMOVE_RCACHE"))))) {
-        remove.packages("R.cache")
-      }
-    }
-  )
 do_package_checks(error_on = ifelse(getRversion() >= "3.2", "note", "error"))
 
 if (Sys.getenv("id_rsa") != "" && ci()$get_branch() == "master" && Sys.getenv("BUILD_PKGDOWN") != "") {
