Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make cyclocomp_linter() optional #2555

Merged
merged 26 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ad71db5
Make `cyclocomp_linter()` optional
IndrajeetPatil Apr 13, 2024
46a11d2
update NAMESPACE
IndrajeetPatil Apr 13, 2024
10945c2
error if cyclocomp is not installed
IndrajeetPatil Apr 14, 2024
5973479
Update cyclocomp_linter.R
IndrajeetPatil Apr 14, 2024
108870d
more conditional skips
IndrajeetPatil Apr 14, 2024
cd54c31
Update error message
IndrajeetPatil Apr 14, 2024
3c2b889
Merge branch 'main' into f2554-cyclocomp-optional
IndrajeetPatil Apr 20, 2024
a41fae5
Update NEWS.md
IndrajeetPatil Apr 20, 2024
a0074ca
Merge branch 'f2554-cyclocomp-optional' of https://github.com/r-lib/l…
IndrajeetPatil Apr 20, 2024
cd835af
Merge branch 'main' into f2554-cyclocomp-optional
MichaelChirico May 4, 2024
6a46496
Update NEWS.md
IndrajeetPatil May 5, 2024
8295c66
Merge branch 'main' into f2554-cyclocomp-optional
IndrajeetPatil May 5, 2024
6c60645
Merge branch 'main' into f2554-cyclocomp-optional
IndrajeetPatil May 5, 2024
b65e0a9
remove unnecessary skip
IndrajeetPatil May 5, 2024
baf9512
ignore rd xref note
IndrajeetPatil May 5, 2024
c625249
fix file ext
IndrajeetPatil May 5, 2024
b834c1c
Revert "fix file ext"
IndrajeetPatil May 5, 2024
34bec97
Revert "ignore rd xref note"
IndrajeetPatil May 5, 2024
075e5b4
fix Rd issue
IndrajeetPatil May 5, 2024
fd6f038
fix rd issue again
IndrajeetPatil May 5, 2024
34f7f21
Update lint.yaml
IndrajeetPatil May 5, 2024
1a07a08
does clearing cache first help?
IndrajeetPatil May 5, 2024
afbe5e6
Revert "does clearing cache first help?"
IndrajeetPatil May 5, 2024
fa2280e
fix broken test
MichaelChirico May 6, 2024
ef660d4
Merge branch 'main' into f2554-cyclocomp-optional
IndrajeetPatil May 6, 2024
2d672da
remove cyclocomp from default linter tests
IndrajeetPatil May 7, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ jobs:
- uses: r-lib/actions/setup-r-dependencies@v2
with:
extra-packages: |
any::cyclocomp
r-lib/lintr
local::.
needs: lint
Expand Down
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ Depends:
Imports:
backports (>= 1.1.7),
codetools,
cyclocomp,
digest,
glue,
knitr,
Expand All @@ -37,6 +36,7 @@ Imports:
Suggests:
bookdown,
cli,
cyclocomp,
jsonlite,
patrick (>= 0.2.0),
rlang,
Expand Down
1 change: 0 additions & 1 deletion NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 13 additions & 4 deletions R/cyclocomp_linter.R
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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))
)
Expand Down
1 change: 0 additions & 1 deletion R/lintr-package.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion R/zzz.R
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion inst/lintr/linters.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions man/cyclocomp_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions man/default_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions man/linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion tests/testthat/default_linter_testcode.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ g <- function(x) {
# commented_code
# some <- commented("out code")

# cyclocomp
# equals_na
# brace_linter
# indentation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ f = function (x,y = 1){}
# commented_code
# some <- commented("out code")

# cyclocomp
# equals_na
# infix_spaces
# line_length
Expand Down
9 changes: 5 additions & 4 deletions tests/testthat/test-cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down
2 changes: 2 additions & 0 deletions tests/testthat/test-cyclocomp_linter.R
Original file line number Diff line number Diff line change
@@ -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")
Expand Down
3 changes: 3 additions & 0 deletions tests/testthat/test-get_source_expressions.R
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
Loading