diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 6ab847c84..32b3f16ba 100644 --- 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/.github/workflows/pkgdown.yaml b/.github/workflows/pkgdown.yaml index 9e29ea59f..dad368915 100644 --- a/.github/workflows/pkgdown.yaml +++ b/.github/workflows/pkgdown.yaml @@ -39,7 +39,7 @@ jobs: - name: Deploy to GitHub pages 🚀 if: github.event_name != 'pull_request' - uses: JamesIves/github-pages-deploy-action@v4.6.0 + uses: JamesIves/github-pages-deploy-action@v4.6.1 with: clean: false branch: gh-pages diff --git a/.github/workflows/test-coverage-examples.yaml b/.github/workflows/test-coverage-examples.yaml new file mode 100644 index 000000000..b0c58c2f0 --- /dev/null +++ b/.github/workflows/test-coverage-examples.yaml @@ -0,0 +1,71 @@ +on: + schedule: + # * is a special character in YAML so you have to quote this string + # Trigger once a month at 10:00 on the first day of every month + - cron: "00 10 1 * *" + +name: test-coverage-examples + +jobs: + test-coverage-examples: + runs-on: ubuntu-latest + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + + steps: + - uses: actions/checkout@v4 + + - uses: r-lib/actions/setup-r@v2 + with: + use-public-rspm: true + + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: | + any::covr + local::. + + - name: Test example coverage + run: | + options(crayon.enabled = TRUE) + library(covr) + files_to_exclude <- c( + # examples present but not run + "R/lint.R", + "R/use_lintr.R", + # mostly internal utilities + "R/actions.R", + "R/cache.R", + "R/deprecated.R", + "R/exclude.R", + "R/extract.R", + "R/ids_with_token.R", + "R/lintr-deprecated.R", + "R/make_linter_from_regex.R", + "R/make_linter_from_xpath.R", + "R/namespace.R", + "R/methods.R", + "R/settings.R", + "R/shared_constants.R", + "R/with.R", + "R/with_id.R", + "R/zzz.R" + ) + coverage <- covr::package_coverage( + type = "examples", + quiet = TRUE, + commentDonttest = FALSE, + commentDontrun = FALSE, + line_exclusions = files_to_exclude + ) + print(coverage) + percent_coverage <- as.integer(covr::percent_coverage(coverage)) + threshold <- 90 + cli::cli_rule() + if (percent_coverage < threshold) { + cli::cli_abort("Code coverage using examples ({percent_coverage}%) is below the required threshold ({threshold}%).") + } else { + cli::cli_alert_success("Code coverage using examples ({percent_coverage}%) is above the required threshold ({threshold}%).") + } + cli::cli_rule() + shell: Rscript {0} diff --git a/DESCRIPTION b/DESCRIPTION index ae418c729..f16a46112 100644 --- 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, @@ -202,6 +202,7 @@ Collate: 'with.R' 'with_id.R' 'xml_nodes_to_lints.R' + 'xml_utils.R' 'yoda_test_linter.R' 'zzz.R' Language: en-US diff --git a/NAMESPACE b/NAMESPACE index 00d9ad2e0..1b2702104 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -57,6 +57,7 @@ export(expect_length_linter) export(expect_lint) export(expect_lint_free) export(expect_named_linter) +export(expect_no_lint) export(expect_not_linter) export(expect_null_linter) export(expect_s3_class_linter) @@ -171,7 +172,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 index e85efaeb1..77ace633f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -15,6 +15,8 @@ * `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. +* `scalar_in_linter` is now configurable to allow other `%in%` like operators to be linted. The data.table operator `%chin%` is no longer linted by default; use `in_operators = "%chin%"` to continue linting it. (@F-Noelle) ## Bug fixes @@ -52,7 +54,8 @@ * `vector_logic_linter()` is extended to recognize incorrect usage of scalar operators `&&` and `||` inside subsetting expressions like `dplyr::filter(x, A && B)` (#2166, @MichaelChirico). * `any_is_na_linter()` is extended to catch the unusual usage `NA %in% x` (#2113, @MichaelChirico). * `make_linter_from_xpath()` errors up front when `lint_message` is missing (instead of delaying this error until the linter is used, #2541, @MichaelChirico). -* `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico). +* `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico). +* `expect_no_lint()` was added as new function to cover the typical use case of expecting no lint message, akin to the recent {testthat} functions like `expect_no_warning()` (#2580, @F-Noelle). ### New linters diff --git a/R/backport_linter.R b/R/backport_linter.R index 3c1eaeaeb..d93841c0d 100644 --- a/R/backport_linter.R +++ b/R/backport_linter.R @@ -30,6 +30,11 @@ #' linters = backport_linter("4.0.0") #' ) #' +#' lint( +#' text = "str2lang(x)", +#' linters = backport_linter("3.2.0", except = "str2lang") +#' ) +#' #' @evalRd rd_tags("backport_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export diff --git a/R/cyclocomp_linter.R b/R/cyclocomp_linter.R index c5563646f..5a64156bd 100644 --- 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/exclude.R b/R/exclude.R index a5462c290..40c79d834 100644 --- a/R/exclude.R +++ b/R/exclude.R @@ -30,6 +30,8 @@ #' " a character vector of files to exclude or a vector of lines to exclude.", #' NULL #' ) +#' +#' @keywords internal exclude <- function(lints, exclusions = settings$exclusions, linter_names = NULL, ...) { if (length(lints) <= 0L) { return(lints) @@ -100,6 +102,7 @@ line_info <- function(line_numbers, type = c("start", "end")) { #' @param linter_names Names of active linters. #' #' @return A possibly named list of excluded lines, possibly for specific linters. +#' @keywords internal parse_exclusions <- function(file, exclude = settings$exclude, exclude_next = settings$exclude_next, diff --git a/R/expect_lint.R b/R/expect_lint.R index feb2164ef..c0c9915ae 100644 --- a/R/expect_lint.R +++ b/R/expect_lint.R @@ -1,6 +1,8 @@ #' Lint expectation #' -#' This is an expectation function to test that the lints produced by `lint` satisfy a number of checks. +#' These are expectation functions to test specified linters on sample code in the `testthat` testing framework. +#' * `expect_lint` asserts that specified lints are generated. +#' * `expect_no_lint` asserts that no lints are generated. #' #' @param content a character vector for the file content to be linted, each vector element representing a line of #' text. @@ -22,7 +24,7 @@ #' @return `NULL`, invisibly. #' @examples #' # no expected lint -#' expect_lint("a", NULL, trailing_blank_lines_linter()) +#' expect_no_lint("a", trailing_blank_lines_linter()) #' #' # one expected lint #' expect_lint("a\n", "trailing blank", trailing_blank_lines_linter()) @@ -42,7 +44,8 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { if (!requireNamespace("testthat", quietly = TRUE)) { stop( # nocov start - "'expect_lint' is designed to work within the 'testthat' testing framework, but 'testthat' is not installed.", + "'expect_lint' and 'expect_no_lint' are designed to work within the 'testthat' testing framework, ", + "but 'testthat' is not installed.", call. = FALSE ) # nocov end } @@ -123,6 +126,11 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { invisible(NULL) } +#' @rdname expect_lint +#' @export +expect_no_lint <- function(content, ..., file = NULL, language = "en") { + expect_lint(content, NULL, ..., file = file, language = language) +} #' Test that the package is lint free #' diff --git a/R/inner_combine_linter.R b/R/inner_combine_linter.R index b83a120a3..bac85955e 100644 --- a/R/inner_combine_linter.R +++ b/R/inner_combine_linter.R @@ -6,6 +6,13 @@ #' preferred so that the most expensive part of the operation ([as.Date()]) #' is applied only once. #' +#' Note that [strptime()] has one idiosyncrasy to be aware of, namely that +#' auto-detected `format=` is set by the first matching input, which means +#' that a case like `c(as.POSIXct("2024-01-01"), as.POSIXct("2024-01-01 01:02:03"))` +#' gives different results to `as.POSIXct(c("2024-01-01", "2024-01-01 01:02:03"))`. +#' This false positive is rare; a workaround where possible is to use +#' consistent formatting, i.e., `"2024-01-01 00:00:00"` in the example. +#' #' @examples #' # will produce lints #' lint( diff --git a/R/lintr-package.R b/R/lintr-package.R index 898db1445..1195e6137 100644 --- 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/scalar_in_linter.R b/R/scalar_in_linter.R index 77ca70285..e6c9faace 100644 --- a/R/scalar_in_linter.R +++ b/R/scalar_in_linter.R @@ -1,12 +1,14 @@ #' Block usage like x %in% "a" #' #' `vector %in% set` is appropriate for matching a vector to a set, but if -#' that set has size 1, `==` is more appropriate. `%chin%` from `{data.table}` -#' is matched as well. +#' that set has size 1, `==` is more appropriate. #' #' `scalar %in% vector` is OK, because the alternative (`any(vector == scalar)`) #' is more circuitous & potentially less clear. #' +#' @param in_operators Character vector of additional infix operators that behave like the `%in%` operator, +#' e.g. `{data.table}`'s `%chin%` operator. +#' #' @examples #' # will produce lints #' lint( @@ -16,7 +18,7 @@ #' #' lint( #' text = "x %chin% 'a'", -#' linters = scalar_in_linter() +#' linters = scalar_in_linter(in_operators = "%chin%") #' ) #' #' # okay @@ -28,22 +30,24 @@ #' @evalRd rd_tags("scalar_in_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -scalar_in_linter <- function() { +scalar_in_linter <- function(in_operators = NULL) { # TODO(#2085): Extend to include other cases where the RHS is clearly a scalar # NB: all of logical, integer, double, hex, complex are parsed as NUM_CONST - xpath <- " - //SPECIAL[text() = '%in%' or text() = '%chin%'] + xpath <- glue(" + //SPECIAL[{xp_text_in_table(c('%in%', {in_operators}))}] /following-sibling::expr[NUM_CONST[not(starts-with(text(), 'NA'))] or STR_CONST] /parent::expr - " + ") Linter(linter_level = "expression", function(source_expression) { xml <- source_expression$xml_parsed_content bad_expr <- xml_find_all(xml, xpath) in_op <- xml_find_chr(bad_expr, "string(SPECIAL)") - lint_msg <- - paste0("Use == to match length-1 scalars, not ", in_op, ". Note that == preserves NA where ", in_op, " does not.") + lint_msg <- glue( + "Use comparison operators (e.g. ==, !=, etc.) to match length-1 scalars instead of {in_op}. ", + "Note that comparison operators preserve NA where {in_op} does not." + ) xml_nodes_to_lints( bad_expr, diff --git a/R/utils.R b/R/utils.R index 5a9e22b84..079431c75 100644 --- a/R/utils.R +++ b/R/utils.R @@ -86,17 +86,6 @@ names2 <- function(x) { names(x) %||% rep("", length(x)) } -safe_parse_to_xml <- function(parsed_content) { - if (is.null(parsed_content)) { - return(xml2::xml_missing()) - } - tryCatch( - xml2::read_xml(xmlparsedata::xml_parse_data(parsed_content)), - # use xml_missing so that code doesn't always need to condition on XML existing - error = function(e) xml2::xml_missing() - ) -} - get_content <- function(lines, info) { lines[is.na(lines)] <- "" @@ -226,8 +215,8 @@ platform_independent_sort <- function(x) x[platform_independent_order(x)] #' writeLines("c('a', 'b')", tmp) #' expr_as_xml <- get_source_expressions(tmp)$expressions[[1L]]$xml_parsed_content #' writeLines(as.character(expr_as_xml)) -#' get_r_string(expr_as_xml, "expr[2]") # "a" -#' get_r_string(expr_as_xml, "expr[3]") # "b" +#' get_r_string(expr_as_xml, "expr[2]") +#' get_r_string(expr_as_xml, "expr[3]") #' unlink(tmp) #' #' # more importantly, extract strings under R>=4 raw strings @@ -236,8 +225,8 @@ platform_independent_sort <- function(x) x[platform_independent_order(x)] #' writeLines("c(R'(a\\b)', R'--[a\\\"\'\"\\b]--')", tmp4.0) #' expr_as_xml4.0 <- get_source_expressions(tmp4.0)$expressions[[1L]]$xml_parsed_content #' writeLines(as.character(expr_as_xml4.0)) -#' get_r_string(expr_as_xml4.0, "expr[2]") # "a\\b" -#' get_r_string(expr_as_xml4.0, "expr[3]") # "a\\\"'\"\\b" +#' get_r_string(expr_as_xml4.0, "expr[2]") +#' get_r_string(expr_as_xml4.0, "expr[3]") #' unlink(tmp4.0) #' #' @export @@ -257,18 +246,6 @@ get_r_string <- function(s, xpath = NULL) { out } -#' str2lang, but for xml children. -#' -#' [xml2::xml_text()] is deceptively close to obviating this helper, but it collapses -#' text across lines. R is _mostly_ whitespace-agnostic, so this only matters in some edge cases, -#' in particular when there are comments within an expression (`` node). See #1919. -#' -#' @noRd -xml2lang <- function(x) { - x_strip_comments <- xml_find_all(x, ".//*[not(self::COMMENT or self::expr)]") - str2lang(paste(xml_text(x_strip_comments), collapse = " ")) -} - is_linter <- function(x) inherits(x, "linter") is_tainted <- function(lines) { diff --git a/R/xml_nodes_to_lints.R b/R/xml_nodes_to_lints.R index 323a0f5be..32b49c905 100644 --- a/R/xml_nodes_to_lints.R +++ b/R/xml_nodes_to_lints.R @@ -96,10 +96,3 @@ xml_nodes_to_lints <- function(xml, source_expression, lint_message, ranges = list(c(col1, col2)) ) } - -is_node <- function(xml) inherits(xml, "xml_node") -is_nodeset <- function(xml) inherits(xml, "xml_nodeset") -is_nodeset_like <- function(xml) { - is_nodeset(xml) || - (is.list(xml) && all(vapply(xml, is_node, logical(1L)))) -} diff --git a/R/xml_utils.R b/R/xml_utils.R new file mode 100644 index 000000000..3b0546da6 --- /dev/null +++ b/R/xml_utils.R @@ -0,0 +1,32 @@ +# utils for working with XML + +#' str2lang, but for xml children. +#' +#' [xml2::xml_text()] is deceptively close to obviating this helper, but it collapses +#' text across lines. R is _mostly_ whitespace-agnostic, so this only matters in some edge cases, +#' in particular when there are comments within an expression (`` node). See #1919. +#' +#' @noRd +xml2lang <- function(x) { + x_strip_comments <- xml_find_all(x, ".//*[not(self::COMMENT or self::expr)]") + str2lang(paste(xml_text(x_strip_comments), collapse = " ")) +} + + +safe_parse_to_xml <- function(parsed_content) { + if (is.null(parsed_content)) { + return(xml2::xml_missing()) + } + tryCatch( + xml2::read_xml(xmlparsedata::xml_parse_data(parsed_content)), + # use xml_missing so that code doesn't always need to condition on XML existing + error = function(e) xml2::xml_missing() + ) +} + +is_node <- function(xml) inherits(xml, "xml_node") +is_nodeset <- function(xml) inherits(xml, "xml_nodeset") +is_nodeset_like <- function(xml) { + is_nodeset(xml) || + (is.list(xml) && all(vapply(xml, is_node, logical(1L)))) +} diff --git a/R/zzz.R b/R/zzz.R index a135938a8..291539297 100644 --- 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 index f6d614775..95af98b61 100644 --- 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 @@ -90,7 +90,7 @@ repeat_linter,style readability return_linter,style configurable default routine_registration_linter,best_practices efficiency robustness sample_int_linter,efficiency readability robustness -scalar_in_linter,readability consistency best_practices efficiency +scalar_in_linter,readability consistency best_practices efficiency configurable semicolon_linter,style readability default configurable semicolon_terminator_linter,defunct seq_linter,robustness efficiency consistency best_practices default diff --git a/man/backport_linter.Rd b/man/backport_linter.Rd index 4d72027dd..17bb27317 100644 --- a/man/backport_linter.Rd +++ b/man/backport_linter.Rd @@ -39,6 +39,11 @@ lint( linters = backport_linter("4.0.0") ) +lint( + text = "str2lang(x)", + linters = backport_linter("3.2.0", except = "str2lang") +) + } \seealso{ \link{linters} for a complete list of linters available in lintr. diff --git a/man/configurable_linters.Rd b/man/configurable_linters.Rd index cb1c17a54..1c72fffab 100644 --- a/man/configurable_linters.Rd +++ b/man/configurable_linters.Rd @@ -44,6 +44,7 @@ The following linters are tagged with 'configurable': \item{\code{\link{quotes_linter}}} \item{\code{\link{redundant_ifelse_linter}}} \item{\code{\link{return_linter}}} +\item{\code{\link{scalar_in_linter}}} \item{\code{\link{semicolon_linter}}} \item{\code{\link{string_boundary_linter}}} \item{\code{\link{todo_comment_linter}}} diff --git a/man/cyclocomp_linter.Rd b/man/cyclocomp_linter.Rd index 59a376cdf..93d42f905 100644 --- a/man/cyclocomp_linter.Rd +++ b/man/cyclocomp_linter.Rd @@ -7,13 +7,14 @@ cyclocomp_linter(complexity_limit = 15L) } \arguments{ -\item{complexity_limit}{Maximum cyclomatic complexity, default 15. Expressions more complex -than this are linted. See \code{\link[cyclocomp:cyclocomp]{cyclocomp::cyclocomp()}}.} +\item{complexity_limit}{Maximum cyclomatic complexity, default \code{15}. Expressions more complex +than this are linted.} } \description{ -Check for overly complicated expressions. See \code{\link[cyclocomp:cyclocomp]{cyclocomp::cyclocomp()}}. +Check for overly complicated expressions. See \code{cyclocomp()} function from \code{{cyclocomp}}. } \examples{ +\dontshow{if (requireNamespace("cyclocomp", quietly = TRUE)) (if (getRversion() >= "3.4") withAutoprint else force)(\{ # examplesIf} # will produce lints lint( text = "if (TRUE) 1 else 2", @@ -25,11 +26,11 @@ lint( text = "if (TRUE) 1 else 2", linters = cyclocomp_linter(complexity_limit = 2L) ) - +\dontshow{\}) # examplesIf} } \seealso{ \link{linters} for a complete list of linters available in lintr. } \section{Tags}{ -\link[=best_practices_linters]{best_practices}, \link[=configurable_linters]{configurable}, \link[=default_linters]{default}, \link[=readability_linters]{readability}, \link[=style_linters]{style} +\link[=best_practices_linters]{best_practices}, \link[=configurable_linters]{configurable}, \link[=readability_linters]{readability}, \link[=style_linters]{style} } diff --git a/man/default_linters.Rd b/man/default_linters.Rd index f14177a9a..cdaa763c7 100644 --- a/man/default_linters.Rd +++ b/man/default_linters.Rd @@ -5,7 +5,7 @@ \alias{default_linters} \title{Default linters} \format{ -An object of class \code{list} of length 26. +An object of class \code{list} of length 25. } \usage{ default_linters @@ -29,7 +29,6 @@ The following linters are tagged with 'default': \item{\code{\link{brace_linter}}} \item{\code{\link{commas_linter}}} \item{\code{\link{commented_code_linter}}} -\item{\code{\link{cyclocomp_linter}}} \item{\code{\link{equals_na_linter}}} \item{\code{\link{function_left_parentheses_linter}}} \item{\code{\link{indentation_linter}}} diff --git a/man/exclude.Rd b/man/exclude.Rd index 3a97087e1..b454dea15 100644 --- a/man/exclude.Rd +++ b/man/exclude.Rd @@ -42,3 +42,4 @@ a character vector of files to exclude or a vector of lines to exclude. } } } +\keyword{internal} diff --git a/man/expect_lint.Rd b/man/expect_lint.Rd index 8b7a22fc1..943cfd1a2 100644 --- a/man/expect_lint.Rd +++ b/man/expect_lint.Rd @@ -2,9 +2,12 @@ % Please edit documentation in R/expect_lint.R \name{expect_lint} \alias{expect_lint} +\alias{expect_no_lint} \title{Lint expectation} \usage{ expect_lint(content, checks, ..., file = NULL, language = "en") + +expect_no_lint(content, ..., file = NULL, language = "en") } \arguments{ \item{content}{a character vector for the file content to be linted, each vector element representing a line of @@ -33,11 +36,15 @@ This makes testing them reproducible on all systems irrespective of their native \code{NULL}, invisibly. } \description{ -This is an expectation function to test that the lints produced by \code{lint} satisfy a number of checks. +These are expectation functions to test specified linters on sample code in the \code{testthat} testing framework. +\itemize{ +\item \code{expect_lint} asserts that specified lints are generated. +\item \code{expect_no_lint} asserts that no lints are generated. +} } \examples{ # no expected lint -expect_lint("a", NULL, trailing_blank_lines_linter()) +expect_no_lint("a", trailing_blank_lines_linter()) # one expected lint expect_lint("a\n", "trailing blank", trailing_blank_lines_linter()) diff --git a/man/get_r_string.Rd b/man/get_r_string.Rd index 05b8c6062..b1564c601 100644 --- a/man/get_r_string.Rd +++ b/man/get_r_string.Rd @@ -27,8 +27,8 @@ tmp <- tempfile() writeLines("c('a', 'b')", tmp) expr_as_xml <- get_source_expressions(tmp)$expressions[[1L]]$xml_parsed_content writeLines(as.character(expr_as_xml)) -get_r_string(expr_as_xml, "expr[2]") # "a" -get_r_string(expr_as_xml, "expr[3]") # "b" +get_r_string(expr_as_xml, "expr[2]") +get_r_string(expr_as_xml, "expr[3]") unlink(tmp) # more importantly, extract strings under R>=4 raw strings @@ -37,8 +37,8 @@ tmp4.0 <- tempfile() writeLines("c(R'(a\\\\b)', R'--[a\\\\\"\'\"\\\\b]--')", tmp4.0) expr_as_xml4.0 <- get_source_expressions(tmp4.0)$expressions[[1L]]$xml_parsed_content writeLines(as.character(expr_as_xml4.0)) -get_r_string(expr_as_xml4.0, "expr[2]") # "a\\b" -get_r_string(expr_as_xml4.0, "expr[3]") # "a\\\"'\"\\b" +get_r_string(expr_as_xml4.0, "expr[2]") +get_r_string(expr_as_xml4.0, "expr[3]") unlink(tmp4.0) \dontshow{\}) # examplesIf} } diff --git a/man/inner_combine_linter.Rd b/man/inner_combine_linter.Rd index 73ac594e2..6455f5d68 100644 --- a/man/inner_combine_linter.Rd +++ b/man/inner_combine_linter.Rd @@ -13,6 +13,14 @@ The same equivalence holds for several other vectorized functions like preferred so that the most expensive part of the operation (\code{\link[=as.Date]{as.Date()}}) is applied only once. } +\details{ +Note that \code{\link[=strptime]{strptime()}} has one idiosyncrasy to be aware of, namely that +auto-detected \verb{format=} is set by the first matching input, which means +that a case like \code{c(as.POSIXct("2024-01-01"), as.POSIXct("2024-01-01 01:02:03"))} +gives different results to \code{as.POSIXct(c("2024-01-01", "2024-01-01 01:02:03"))}. +This false positive is rare; a workaround where possible is to use +consistent formatting, i.e., \code{"2024-01-01 00:00:00"} in the example. +} \examples{ # will produce lints lint( diff --git a/man/linters.Rd b/man/linters.Rd index 10c45d374..394bd6126 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -19,10 +19,10 @@ The following tags exist: \itemize{ \item{\link[=best_practices_linters]{best_practices} (63 linters)} \item{\link[=common_mistakes_linters]{common_mistakes} (11 linters)} -\item{\link[=configurable_linters]{configurable} (43 linters)} +\item{\link[=configurable_linters]{configurable} (44 linters)} \item{\link[=consistency_linters]{consistency} (32 linters)} \item{\link[=correctness_linters]{correctness} (7 linters)} -\item{\link[=default_linters]{default} (26 linters)} +\item{\link[=default_linters]{default} (25 linters)} \item{\link[=deprecated_linters]{deprecated} (6 linters)} \item{\link[=efficiency_linters]{efficiency} (32 linters)} \item{\link[=executing_linters]{executing} (6 linters)} @@ -54,7 +54,7 @@ The following linters exist: \item{\code{\link{conjunct_test_linter}} (tags: best_practices, configurable, package_development, pkg_testthat, readability)} \item{\code{\link{consecutive_assertion_linter}} (tags: consistency, readability, style)} \item{\code{\link{consecutive_mutate_linter}} (tags: configurable, consistency, efficiency, readability)} -\item{\code{\link{cyclocomp_linter}} (tags: best_practices, configurable, default, readability, style)} +\item{\code{\link{cyclocomp_linter}} (tags: best_practices, configurable, readability, style)} \item{\code{\link{duplicate_argument_linter}} (tags: common_mistakes, configurable, correctness)} \item{\code{\link{empty_assignment_linter}} (tags: best_practices, readability)} \item{\code{\link{equals_na_linter}} (tags: common_mistakes, correctness, default, robustness)} @@ -123,7 +123,7 @@ The following linters exist: \item{\code{\link{return_linter}} (tags: configurable, default, style)} \item{\code{\link{routine_registration_linter}} (tags: best_practices, efficiency, robustness)} \item{\code{\link{sample_int_linter}} (tags: efficiency, readability, robustness)} -\item{\code{\link{scalar_in_linter}} (tags: best_practices, consistency, efficiency, readability)} +\item{\code{\link{scalar_in_linter}} (tags: best_practices, configurable, consistency, efficiency, readability)} \item{\code{\link{semicolon_linter}} (tags: configurable, default, readability, style)} \item{\code{\link{seq_linter}} (tags: best_practices, consistency, default, efficiency, robustness)} \item{\code{\link{sort_linter}} (tags: best_practices, efficiency, readability)} diff --git a/man/parse_exclusions.Rd b/man/parse_exclusions.Rd index d496c6b17..881642e44 100644 --- a/man/parse_exclusions.Rd +++ b/man/parse_exclusions.Rd @@ -42,3 +42,4 @@ A possibly named list of excluded lines, possibly for specific linters. \description{ read a source file and parse all the excluded lines from it } +\keyword{internal} diff --git a/man/scalar_in_linter.Rd b/man/scalar_in_linter.Rd index be94fd1a1..1773c699f 100644 --- a/man/scalar_in_linter.Rd +++ b/man/scalar_in_linter.Rd @@ -4,12 +4,15 @@ \alias{scalar_in_linter} \title{Block usage like x \%in\% "a"} \usage{ -scalar_in_linter() +scalar_in_linter(in_operators = NULL) +} +\arguments{ +\item{in_operators}{Character vector of additional infix operators that behave like the \code{\%in\%} operator, +e.g. \code{{data.table}}'s \verb{\%chin\%} operator.} } \description{ \code{vector \%in\% set} is appropriate for matching a vector to a set, but if -that set has size 1, \code{==} is more appropriate. \verb{\%chin\%} from \code{{data.table}} -is matched as well. +that set has size 1, \code{==} is more appropriate. } \details{ \code{scalar \%in\% vector} is OK, because the alternative (\code{any(vector == scalar)}) @@ -24,7 +27,7 @@ lint( lint( text = "x \%chin\% 'a'", - linters = scalar_in_linter() + linters = scalar_in_linter(in_operators = "\%chin\%") ) # okay @@ -38,5 +41,5 @@ lint( \link{linters} for a complete list of linters available in lintr. } \section{Tags}{ -\link[=best_practices_linters]{best_practices}, \link[=consistency_linters]{consistency}, \link[=efficiency_linters]{efficiency}, \link[=readability_linters]{readability} +\link[=best_practices_linters]{best_practices}, \link[=configurable_linters]{configurable}, \link[=consistency_linters]{consistency}, \link[=efficiency_linters]{efficiency}, \link[=readability_linters]{readability} } diff --git a/tests/testthat/default_linter_testcode.R b/tests/testthat/default_linter_testcode.R index 1f9f060d6..1cadc18cc 100644 --- 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 index ba87313b3..3098b0b02 100644 --- 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 index 6b0c47aa1..62a0ed32c 100644 --- 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 index 4f2b463f7..5e14b5680 100644 --- 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-expect_lint.R b/tests/testthat/test-expect_lint.R index 622882884..00bf44bc6 100644 --- a/tests/testthat/test-expect_lint.R +++ b/tests/testthat/test-expect_lint.R @@ -6,9 +6,9 @@ linter <- assignment_linter() lint_msg <- "Use <-, not =" test_that("no checks", { - expect_success(expect_lint("a", NULL, linter)) - expect_success(expect_lint("a=1", NULL, list())) - expect_failure(expect_lint("a=1", NULL, linter)) + expect_success(expect_no_lint("a", linter)) + expect_success(expect_no_lint("a=1", list())) + expect_failure(expect_no_lint("a=1", linter)) }) test_that("single check", { diff --git a/tests/testthat/test-get_source_expressions.R b/tests/testthat/test-get_source_expressions.R index dbacecacc..b12fb1b66 100644 --- 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]] diff --git a/tests/testthat/test-scalar_in_linter.R b/tests/testthat/test-scalar_in_linter.R index 2bfd66f83..fb3663087 100644 --- a/tests/testthat/test-scalar_in_linter.R +++ b/tests/testthat/test-scalar_in_linter.R @@ -3,11 +3,10 @@ test_that("scalar_in_linter skips allowed usages", { expect_lint("x %in% y", NULL, linter) expect_lint("y %in% c('a', 'b')", NULL, linter) - expect_lint("c('a', 'b') %chin% x", NULL, linter) + expect_lint("c('a', 'b') %in% x", NULL, linter) expect_lint("z %in% 1:3", NULL, linter) # scalars on LHS are fine (often used as `"col" %in% names(DF)`) expect_lint("3L %in% x", NULL, linter) - # this should be is.na(x), but it more directly uses the "always TRUE/FALSE, _not_ NA" # aspect of %in%, so we delegate linting here to equals_na_linter() expect_lint("x %in% NA", NULL, linter) @@ -15,16 +14,33 @@ test_that("scalar_in_linter skips allowed usages", { }) test_that("scalar_in_linter blocks simple disallowed usages", { - linter <- scalar_in_linter() - lint_in_msg <- rex::rex("Use == to match length-1 scalars, not %in%.") - lint_chin_msg <- rex::rex("Use == to match length-1 scalars, not %chin%.") + linter <- scalar_in_linter(in_operators = c("%chin%", "%notin%")) + lint_msg <- rex::rex("Use comparison operators (e.g. ==, !=, etc.) to match length-1 scalars instead of") + + expect_lint("x %in% 1", lint_msg, linter) + expect_lint("x %chin% 'a'", lint_msg, linter) + expect_lint("x %notin% 1", lint_msg, linter) +}) - expect_lint("x %in% 1", lint_in_msg, linter) - expect_lint("x %chin% 'a'", lint_chin_msg, linter) +test_that("scalar_in_linter blocks or skips based on configuration", { + linter_default <- scalar_in_linter() + linter_config <- scalar_in_linter(in_operators = "%notin%") + + lint_msg <- rex::rex("Use comparison operators (e.g. ==, !=, etc.) to match length-1 scalars instead of") + + # default + expect_lint("x %in% 1", lint_msg, linter_default) + expect_lint("x %notin% 1", NULL, linter_default) + expect_lint("x %notin% y", NULL, linter_default) + + # configured + expect_lint("x %in% 1", lint_msg, linter_config) + expect_lint("x %notin% 1", lint_msg, linter_config) + expect_lint("x %notin% y", NULL, linter_config) }) test_that("multiple lints are generated correctly", { - linter <- scalar_in_linter() + linter <- scalar_in_linter(in_operators = "%chin%") expect_lint( trim_some('{ diff --git a/vignettes/creating_linters.Rmd b/vignettes/creating_linters.Rmd index d4025848e..7301ff680 100644 --- a/vignettes/creating_linters.Rmd +++ b/vignettes/creating_linters.Rmd @@ -217,7 +217,7 @@ The main three aspects to test are: 1. Linter returns no lints when there is nothing to lint, e.g. ```r -expect_lint("blah", NULL, assignment_linter()) +expect_no_lint("blah", assignment_linter()) ``` 2. Linter returns a lint when there is something to lint, e.g.