Make cyclocomp_linter() optional (#2555)

IndrajeetPatil · MichaelChirico · web-flow · commit cbd0619eb2e4 · 2024-05-07T23:14:40.000-07:00
* Make `cyclocomp_linter()` optional closes #2554 * update NAMESPACE * error if cyclocomp is not installed * Update cyclocomp_linter.R * more conditional skips * Update error message * Update NEWS.md * Update NEWS.md Co-authored-by: Michael Chirico <michaelchirico4@gmail.com> * remove unnecessary skip * ignore rd xref note * fix file ext * Revert "fix file ext" This reverts commit c625249. * Revert "ignore rd xref note" This reverts commit baf9512. * fix Rd issue * fix rd issue again * Update lint.yaml * does clearing cache first help? * Revert "does clearing cache first help?" This reverts commit 1a07a08. * fix broken test * remove cyclocomp from default linter tests --------- Co-authored-by: Michael Chirico <michaelchirico4@gmail.com>
diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
@@ -21,6 +21,7 @@ jobs:
       - uses: r-lib/actions/setup-r-dependencies@v2
         with:
           extra-packages: |
+            any::cyclocomp
             r-lib/lintr
             local::.
           needs: lint
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -25,7 +25,6 @@ Depends:
 Imports:
     backports (>= 1.1.7),
     codetools,
-    cyclocomp,
     digest,
     glue,
     knitr,
@@ -37,6 +36,7 @@ Imports:
 Suggests:
     bookdown,
     cli,
+    cyclocomp,
     jsonlite,
     patrick (>= 0.2.0),
     rlang,
diff --git a/NAMESPACE b/NAMESPACE
@@ -171,7 +171,6 @@ export(with_id)
 export(xml_nodes_to_lints)
 export(xp_call_name)
 export(yoda_test_linter)
-importFrom(cyclocomp,cyclocomp)
 importFrom(glue,glue)
 importFrom(glue,glue_collapse)
 importFrom(rex,character_class)
diff --git a/NEWS.md b/NEWS.md
@@ -15,6 +15,7 @@
 * `extraction_operator_linter()` is deprecated. Although switching from `$` to `[[` has some robustness benefits for package code, it can lead to non-idiomatic code in many contexts (e.g. R6 classes, Shiny applications, etc.) (#2409, @IndrajeetPatil). To enable the detection of the `$` operator for extraction through partial matching, use `options(warnPartialMatchDollar = TRUE)`.
 * `unnecessary_nested_if_linter()` is deprecated and subsumed into the new/more general `unnecessary_nesting_linter()`.
 * Drop support for posting GitHub comments from inside GitHub comment bot, Travis, Wercker, and Jenkins CI tools (spurred by #2148, @MichaelChirico). We rely on GitHub Actions for linting in CI, and don't see any active users relying on these alternatives. We welcome and encourage community contributions to get support for different CI system going again.
+* `cyclocomp_linter()` is no longer part of the default linters (#2555, @IndrajeetPatil) because the tidyverse style guide doesn't contain any guidelines on meeting certain complexity requirements. Note that users with `cyclocomp_linter()` in their configs may now need to install {cyclocomp} intentionally, in particular in CI/CD pipelines.
 
 ## Bug fixes
 
diff --git a/R/cyclocomp_linter.R b/R/cyclocomp_linter.R
@@ -1,11 +1,11 @@
 #' Cyclomatic complexity linter
 #'
-#' Check for overly complicated expressions. See [cyclocomp::cyclocomp()].
+#' Check for overly complicated expressions. See `cyclocomp()` function from `{cyclocomp}`.
 #'
-#' @param complexity_limit Maximum cyclomatic complexity, default 15. Expressions more complex
-#' than this are linted. See [cyclocomp::cyclocomp()].
+#' @param complexity_limit Maximum cyclomatic complexity, default `15`. Expressions more complex
+#' than this are linted.
 #'
-#' @examples
+#' @examplesIf requireNamespace("cyclocomp", quietly = TRUE)
 #' # will produce lints
 #' lint(
 #'   text = "if (TRUE) 1 else 2",
@@ -23,6 +23,15 @@
 #' @export
 cyclocomp_linter <- function(complexity_limit = 15L) {
   Linter(linter_level = "expression", function(source_expression) {
+    # nocov start
+    if (!requireNamespace("cyclocomp", quietly = TRUE)) {
+      cli::cli_abort(c(
+        "Cyclocomp complexity is computed using {.fn cyclocomp::cyclocomp}.",
+        i = "Please install the needed {.pkg cyclocomp} package."
+      ))
+    }
+    # nocov end
+
     complexity <- try_silently(
       cyclocomp::cyclocomp(parse(text = source_expression$content))
     )
diff --git a/R/lintr-package.R b/R/lintr-package.R
@@ -8,7 +8,6 @@
 "_PACKAGE"
 
 ## lintr namespace: start
-#' @importFrom cyclocomp cyclocomp
 #' @importFrom glue glue glue_collapse
 #' @importFrom rex rex regex re_matches re_substitutes character_class
 #' @importFrom stats na.omit
diff --git a/R/zzz.R b/R/zzz.R
@@ -17,7 +17,6 @@ default_linters <- modify_defaults(
   brace_linter(),
   commas_linter(),
   commented_code_linter(),
-  cyclocomp_linter(),
   equals_na_linter(),
   function_left_parentheses_linter(),
   indentation_linter(),
diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv
@@ -17,7 +17,7 @@ conjunct_test_linter,package_development best_practices readability configurable
 consecutive_assertion_linter,style readability consistency
 consecutive_mutate_linter,consistency readability configurable efficiency
 consecutive_stopifnot_linter,style readability consistency deprecated
-cyclocomp_linter,style readability best_practices default configurable
+cyclocomp_linter,style readability best_practices configurable
 duplicate_argument_linter,correctness common_mistakes configurable
 empty_assignment_linter,readability best_practices
 equals_na_linter,robustness correctness common_mistakes default
diff --git a/man/cyclocomp_linter.Rd b/man/cyclocomp_linter.Rd
diff --git a/man/default_linters.Rd b/man/default_linters.Rd
diff --git a/man/linters.Rd b/man/linters.Rd
diff --git a/tests/testthat/default_linter_testcode.R b/tests/testthat/default_linter_testcode.R
@@ -15,7 +15,6 @@ g <- function(x) {
 # commented_code
 # some <- commented("out code")
 
-# cyclocomp
 # equals_na
 # brace_linter
 # indentation
diff --git a/tests/testthat/dummy_projects/project/default_linter_testcode.R b/tests/testthat/dummy_projects/project/default_linter_testcode.R
@@ -10,7 +10,6 @@ f = function (x,y = 1){}
 # commented_code
 # some <- commented("out code")
 
-# cyclocomp
 # equals_na
 # infix_spaces
 # line_length
diff --git a/tests/testthat/test-cache.R b/tests/testthat/test-cache.R
@@ -435,15 +435,16 @@ test_that("it works outside of a package", {
 
 test_that("cache = TRUE workflow works", {
   # Need a test structure with a safe to load .lintr
-  pkg <- "dummy_packages/package"
-  files <- normalizePath(list.files(pkg, recursive = TRUE, full.names = TRUE))
+  withr::local_dir(file.path("dummy_packages", "package"))
+  withr::local_options(lintr.linter_file = "lintr_test_config")
+  files <- normalizePath(list.files(recursive = TRUE, full.names = TRUE))
 
   # Manually clear cache (that function is exported)
   for (f in files) {
     clear_cache(file = f)
   }
-  l1 <- lint_package(pkg, cache = TRUE)
-  l2 <- lint_package(pkg, cache = TRUE)
+  l1 <- lint_package(cache = TRUE)
+  l2 <- lint_package(cache = TRUE)
   expect_identical(l1, l2)
 })
 
diff --git a/tests/testthat/test-cyclocomp_linter.R b/tests/testthat/test-cyclocomp_linter.R
@@ -1,4 +1,6 @@
 test_that("returns the correct linting", {
+  skip_if_not_installed("cyclocomp")
+
   cc_linter_1 <- cyclocomp_linter(1L)
   cc_linter_2 <- cyclocomp_linter(2L)
   lint_msg <- rex::rex("Reduce the cyclomatic complexity of this function")
diff --git a/tests/testthat/test-get_source_expressions.R b/tests/testthat/test-get_source_expressions.R
@@ -418,6 +418,9 @@ patrick::with_parameters_test_that(
       # otherwise we test the trivial linter (#2339)
       linter <- backport_linter(r_version = "3.6.0")
     } else {
+      if (linter == "cyclocomp_linter") {
+        skip_if_not_installed("cyclocomp")
+      }
       linter <- eval(call(linter))
     }
     expression <- expressions[[expression_idx]]
