diff --git a/.github/CODE_OF_CONDUCT.md b/.github/CODE_OF_CONDUCT.md index cb7ff35fb..286c0219e 100644 --- a/.github/CODE_OF_CONDUCT.md +++ b/.github/CODE_OF_CONDUCT.md @@ -59,7 +59,7 @@ representative at an online or offline event. ## Enforcement Instances of abusive, harassing, or otherwise unacceptable behavior may be -reported to the community leaders responsible for enforcement at james.f.hester@gmail.com. +reported to the community leaders responsible for enforcement at michaelchirico4@gmail.com. All complaints will be reviewed and investigated promptly and fairly. All community leaders are obligated to respect the privacy and security of the diff --git a/DESCRIPTION b/DESCRIPTION index c07339b68..39bd42daf 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -2,11 +2,11 @@ Package: lintr Title: A 'Linter' for R Code Version: 3.1.1.9000 Authors@R: c( - person("Jim", "Hester", , "james.f.hester@gmail.com", role = c("aut", "cre")), + person("Jim", "Hester", , role = "aut"), person("Florent", "Angly", role = "aut", comment = "fangly"), person("Russ", "Hyde", role = "aut"), - person("Michael", "Chirico", role = "aut"), + person("Michael", "Chirico", email = "michaelchirico4@gmail.com", role = c("aut", "cre")), person("Kun", "Ren", role = "aut"), person("Alexander", "Rosenstock", role = "aut", comment = "AshesITR"), @@ -37,7 +37,11 @@ Imports: xmlparsedata (>= 1.0.5) Suggests: bookdown, +<<<<<<< HEAD httr (>= 1.2.1), +======= + cli, +>>>>>>> main jsonlite, patrick (>= 0.2.0), rlang, @@ -76,7 +80,6 @@ Collate: 'class_equals_linter.R' 'commas_linter.R' 'commented_code_linter.R' - 'comments.R' 'comparison_negation_linter.R' 'condition_call_linter.R' 'condition_message_linter.R' diff --git a/NEWS.md b/NEWS.md index 55d7d5e80..87cfd9ff2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -14,11 +14,13 @@ * Adjusted various lint messages for consistency in readability (#1330, @MichaelChirico). In general, we favor lint messages to be phrased like "Action, reason" to but the "what" piece of the message front-and-center. This may be a breaking change for code that tests the specific phrasing of lints. * `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 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. ## Bug fixes * `object_name_linter()` no longer errors when user-supplied `regexes=` have capture groups (#2188, @MichaelChirico). * `.lintr` config validation correctly accepts regular expressions which only compile under `perl = TRUE` (#2375, @MichaelChirico). These have always been valid (since `rex::re_matches()`, which powers the lint exclusion logic, also uses this setting), but the new up-front validation in v3.1.1 incorrectly used `perl = FALSE`. +* `.lintr` configs set by option `lintr.linter_file` or environment variable `R_LINTR_LINTER_FILE` can point to subdirectories (#2512, @MichaelChirico). ## Changes to default linters @@ -60,7 +62,7 @@ * `which_grepl_linter()` for discouraging `which(grepl(ptn, x))` in favor of directly using `grep(ptn, x)` (part of #884, @MichaelChirico). * `list_comparison_linter()` for discouraging comparisons on the output of `lapply()`, e.g. `lapply(x, sum) > 10` (part of #884, @MichaelChirico). * `print_linter()` for discouraging usage of `print()` on string literals like `print("Reached here")` or `print(paste("Found", nrow(DF), "rows."))` (#1894, @MichaelChirico). -* `unnecessary_nesting_linter()` for discouraging overly-nested code where an early return or eliminated sub-expression (inside '{') is preferable (#2317 and part of #884, @MichaelChirico). +* `unnecessary_nesting_linter()` for discouraging overly-nested code where an early return or eliminated sub-expression (inside '{') is preferable (#2317, #2334 and part of #884, @MichaelChirico). * `consecutive_mutate_linter()` for encouraging consecutive calls to `dplyr::mutate()` to be combined (part of #884, @MichaelChirico). * `if_switch_linter()` for encouraging `switch()` over repeated `if`/`else` tests (#2322 and part of #884, @MichaelChirico). * `nested_pipe_linter()` for discouraging pipes within pipes, e.g. `df1 %>% inner_join(df2 %>% select(a, b))` (part of #884, @MichaelChirico). diff --git a/R/comments.R b/R/comments.R deleted file mode 100644 index 8d1040e17..000000000 --- a/R/comments.R +++ /dev/null @@ -1,119 +0,0 @@ -in_ci <- function() { - in_travis() || in_wercker() || in_jenkins() -} - -ci_type <- function() { - if (in_travis()) { - return("travis") - } - if (in_wercker()) { - return("wercker") - } - if (in_jenkins()) { - return("jenkins") - } - "" -} - -in_jenkins <- function() { - nzchar(Sys.getenv("JENKINS_URL")) && !is.null(jenkins_build_info()) -} - -jenkins_build_info <- function() { - git_url <- Sys.getenv("GIT_URL", Sys.getenv("GIT_URL_1", NA)) - if (is.na(git_url)) { - return(NULL) - } - - pattern <- "(https?:\\/\\/|git@)github\\.com[:\\/](.+\\/.+)\\.git" - if (!length(grep(pattern, git_url))) { - return(NULL) - } - slug <- gsub(pattern, "\\2", git_url) - - slug_info <- strsplit(slug, "/", fixed = TRUE)[[1L]] - - list( - user = slug_info[1L], - repo = slug_info[2L], - pull = Sys.getenv("CHANGE_ID", NA) %||% NULL, - commit = Sys.getenv("GIT_COMMIT", NA) %||% NULL - ) -} - -in_travis <- function() { - nzchar(Sys.getenv("TRAVIS_REPO_SLUG")) -} - -travis_build_info <- function() { - slug <- Sys.getenv("TRAVIS_REPO_SLUG") - slug_info <- strsplit(slug, "/", fixed = TRUE)[[1L]] - - list( - user = slug_info[1L] %||% "", - repo = slug_info[2L] %||% "", - pull = Sys.getenv("TRAVIS_PULL_REQUEST"), - branch = Sys.getenv("TRAVIS_BRANCH"), - commit = Sys.getenv("TRAVIS_COMMIT") - ) -} - -in_wercker <- function() { - nzchar(Sys.getenv("WERCKER_GIT_BRANCH")) -} - -ci_build_info <- function() { - type <- ci_type() - switch( - type, - travis = travis_build_info(), - wercker = wercker_build_info(), - jenkins = jenkins_build_info() - ) -} - -wercker_build_info <- function() { - list( - user = Sys.getenv("WERCKER_GIT_OWNER"), - repo = Sys.getenv("WERCKER_GIT_REPOSITORY"), - branch = Sys.getenv("WERCKER_GIT_BRANCH"), - commit = Sys.getenv("WERCKER_GIT_COMMIT") - ) -} - -# nocov start -github_comment <- function(text, info = NULL, token = settings$comment_token) { - if (!requireNamespace("httr", quietly = TRUE)) { - cli_abort("Package {.pkg httr} is required to post comments with {.fn github_comment}.") - } - if (!requireNamespace("jsonlite", quietly = TRUE)) { - cli_abort("Package {.pkg jsonlite} is required to post comments with {.fn github_comment}.") - } - - if (is.null(info)) { - info <- ci_build_info() - } - - if (!is.null(info$pull) && info$pull != "false") { - api_subdir <- file.path("issues", info$pull) - } else if (!is.null(info$commit)) { - api_subdir <- file.path("commits", info$commit) - } else { - cli_abort(c( - "Expected a pull or a commit", - x = "Instead received {.fn ci_build_info} = {format(info)}" - )) - } - response <- httr::POST( - "https://api.github.com", - path = file.path("repos", info$user, info$repo, api_subdir, "comments"), - body = list(body = jsonlite::unbox(text)), - query = list(access_token = token), - encode = "json" - ) - - if (httr::status_code(response) >= 300L) { - message(httr::http_condition(response, "error", task = httr::content(response, as = "text"))) - } -} -# nocov end diff --git a/R/methods.R b/R/methods.R index 123d1e312..4ff71c0a1 100644 --- a/R/methods.R +++ b/R/methods.R @@ -95,18 +95,6 @@ print.lints <- function(x, ...) { } else if (in_github_actions()) { github_actions_log_lints(x, project_dir = github_annotation_project_dir) } else { - if (in_ci() && settings$comment_bot) { - info <- ci_build_info() - - lint_output <- trim_output( - paste0( - collapse = "\n", - capture.output(invisible(lapply(x, markdown, info, ...))) - ) - ) - - github_comment(lint_output, info, ...) - } lapply(x, print, ...) } diff --git a/R/settings_utils.R b/R/settings_utils.R index a04ef5b55..9489ea24e 100644 --- a/R/settings_utils.R +++ b/R/settings_utils.R @@ -75,7 +75,7 @@ find_config <- function(filename) { # may exist in subsequent directories are ignored file_locations <- c( # Local (incl. parent) directories - find_local_config(path, basename(linter_file)), + find_local_config(path, linter_file), # User directory # cf: rstudio@bc9b6a5 SessionRSConnect.R#L32 file.path(Sys.getenv("HOME", unset = "~"), linter_file), diff --git a/R/unnecessary_nesting_linter.R b/R/unnecessary_nesting_linter.R index 54df45d04..0fdbe61fa 100644 --- a/R/unnecessary_nesting_linter.R +++ b/R/unnecessary_nesting_linter.R @@ -9,6 +9,12 @@ #' The `TRUE` case facilitates interaction with [implicit_assignment_linter()] #' for certain cases where an implicit assignment is necessary, so a braced #' assignment is used to further distinguish the assignment. See examples. +#' @param allow_functions Character vector of functions which always allow +#' one-child braced expressions. `testthat::test_that()` is always allowed because +#' testthat requires a braced expression in its `code` argument. The other defaults +#' similarly compute on expressions in a way which is worth highlighting by +#' em-bracing them, even if there is only one expression, while [switch()] is allowed +#' for its use as a control flow analogous to `if`/`else`. #' #' @examples #' # will produce lints @@ -39,6 +45,11 @@ #' linters = unnecessary_nesting_linter() #' ) #' +#' lint( +#' text = "my_quote({x})", +#' linters = unnecessary_nesting_linter() +#' ) +#' #' # okay #' code <- "if (A) {\n stop('A is bad because a.')\n} else {\n stop('!A is bad too.')\n}" #' writeLines(code) @@ -73,12 +84,27 @@ #' linters = unnecessary_nesting_linter() #' ) #' +#' lint( +#' text = "my_quote({x})", +#' linters = unnecessary_nesting_linter(allow_functions = "my_quote") +#' ) +#' #' @evalRd rd_tags("unnecessary_nesting_linter") #' @seealso #' - [cyclocomp_linter()] for another linter that penalizes overly complexcode. #' - [linters] for a complete list of linters available in lintr. #' @export -unnecessary_nesting_linter <- function(allow_assignment = TRUE) { +unnecessary_nesting_linter <- function( + allow_assignment = TRUE, + allow_functions = c( + "switch", + "try", "tryCatch", "withCallingHandlers", + "quote", "expression", "bquote", "substitute", + "with_parameters_test_that", + "reactive", "observe", "observeEvent", + "renderCachedPlot", "renderDataTable", "renderImage", "renderPlot", + "renderPrint", "renderTable", "renderText", "renderUI" + )) { exit_calls <- c("stop", "return", "abort", "quit", "q") exit_call_expr <- glue(" expr[SYMBOL_FUNCTION_CALL[{xp_text_in_table(exit_calls)}]] @@ -145,6 +171,7 @@ unnecessary_nesting_linter <- function(allow_assignment = TRUE) { or self::IF or self::WHILE or self::REPEAT + or self::expr/SYMBOL_FUNCTION_CALL[{ xp_text_in_table(c('test_that', allow_functions)) }] or self::expr/expr/SYMBOL_FUNCTION_CALL[text() = 'foreach'] or self::OP-TILDE or self::LEFT_ASSIGN[text() = ':='] diff --git a/inst/syntastic/lintr.vim b/inst/syntastic/lintr.vim index 80e5cbe37..e042ce366 100644 --- a/inst/syntastic/lintr.vim +++ b/inst/syntastic/lintr.vim @@ -1,7 +1,7 @@ "============================================================================ "File: lintr.vim "Description: Syntax checking plugin for syntastic.vim -"Maintainer: Jim Hester +"Maintainer: Michael Chirico "License: This program is free software. It comes without any warranty, " to the extent permitted by applicable law. You can redistribute " it and/or modify it under the terms of the Do What The Fuck You diff --git a/man/lintr-package.Rd b/man/lintr-package.Rd index a88a8b4dc..7e36edb2e 100644 --- a/man/lintr-package.Rd +++ b/man/lintr-package.Rd @@ -13,13 +13,13 @@ Supports on the fly checking of R code edited with Emacs, Vim, and Sublime Text. \code{\link[=lint]{lint()}}, \code{\link[=lint_package]{lint_package()}}, \code{\link[=lint_dir]{lint_dir()}}, \link{linters} } \author{ -\strong{Maintainer}: Jim Hester \email{james.f.hester@gmail.com} +\strong{Maintainer}: Michael Chirico \email{michaelchirico4@gmail.com} Authors: \itemize{ + \item Jim Hester \item Florent Angly (fangly) \item Russ Hyde - \item Michael Chirico \item Kun Ren \item Alexander Rosenstock (AshesITR) \item Indrajeet Patil \email{patilindrajeet.science@gmail.com} (\href{https://orcid.org/0000-0003-1995-6531}{ORCID}) (@patilindrajeets) diff --git a/man/unnecessary_nesting_linter.Rd b/man/unnecessary_nesting_linter.Rd index 6119e3e13..0e185446d 100644 --- a/man/unnecessary_nesting_linter.Rd +++ b/man/unnecessary_nesting_linter.Rd @@ -4,7 +4,13 @@ \alias{unnecessary_nesting_linter} \title{Block instances of unnecessary nesting} \usage{ -unnecessary_nesting_linter(allow_assignment = TRUE) +unnecessary_nesting_linter( + allow_assignment = TRUE, + allow_functions = c("switch", "try", "tryCatch", "withCallingHandlers", "quote", + "expression", "bquote", "substitute", "with_parameters_test_that", "reactive", + "observe", "observeEvent", "renderCachedPlot", "renderDataTable", "renderImage", + "renderPlot", "renderPrint", "renderTable", "renderText", "renderUI") +) } \arguments{ \item{allow_assignment}{Logical, default \code{TRUE}, in which case @@ -13,6 +19,13 @@ if \code{FALSE}, all braced expressions with only one child expression are linte The \code{TRUE} case facilitates interaction with \code{\link[=implicit_assignment_linter]{implicit_assignment_linter()}} for certain cases where an implicit assignment is necessary, so a braced assignment is used to further distinguish the assignment. See examples.} + +\item{allow_functions}{Character vector of functions which always allow +one-child braced expressions. \code{testthat::test_that()} is always allowed because +testthat requires a braced expression in its \code{code} argument. The other defaults +similarly compute on expressions in a way which is worth highlighting by +em-bracing them, even if there is only one expression, while \code{\link[=switch]{switch()}} is allowed +for its use as a control flow analogous to \code{if}/\verb{else}.} } \description{ Excessive nesting harms readability. Use helper functions or early returns @@ -47,6 +60,11 @@ lint( linters = unnecessary_nesting_linter() ) +lint( + text = "my_quote({x})", + linters = unnecessary_nesting_linter() +) + # okay code <- "if (A) {\n stop('A is bad because a.')\n} else {\n stop('!A is bad too.')\n}" writeLines(code) @@ -81,6 +99,11 @@ lint( linters = unnecessary_nesting_linter() ) +lint( + text = "my_quote({x})", + linters = unnecessary_nesting_linter(allow_functions = "my_quote") +) + } \seealso{ \itemize{ diff --git a/tests/testthat/test-ci.R b/tests/testthat/test-ci.R index 8bab65609..df42f8cd6 100644 --- a/tests/testthat/test-ci.R +++ b/tests/testthat/test-ci.R @@ -37,25 +37,3 @@ test_that("GitHub Actions - linting on error works", { local_mocked_bindings(quit = function(...) cat("Tried to quit.\n")) expect_output(print(l), "::warning file", fixed = TRUE) }) - -test_that("Printing works for Travis", { - withr::local_envvar(list(GITHUB_ACTIONS = "false", TRAVIS_REPO_SLUG = "test/repo", LINTR_COMMENT_BOT = "true")) - withr::local_options(lintr.rstudio_source_markers = FALSE) - tmp <- withr::local_tempfile(lines = "x <- 1:nrow(y)") - - l <- lint(tmp) - - local_mocked_bindings(github_comment = function(x, ...) cat(x, "\n")) - expect_output(print(l), "*warning:*", fixed = TRUE) -}) - -test_that("Printing works for Wercker", { - withr::local_envvar(list(GITHUB_ACTIONS = "false", WERCKER_GIT_BRANCH = "test/repo", LINTR_COMMENT_BOT = "true")) - withr::local_options(lintr.rstudio_source_markers = FALSE) - tmp <- withr::local_tempfile(lines = "x <- 1:nrow(y)") - - l <- lint(tmp) - - local_mocked_bindings(github_comment = function(x, ...) cat(x, "\n")) - expect_output(print(l), "*warning:*", fixed = TRUE) -}) diff --git a/tests/testthat/test-comments.R b/tests/testthat/test-comments.R deleted file mode 100644 index 50356dd98..000000000 --- a/tests/testthat/test-comments.R +++ /dev/null @@ -1,81 +0,0 @@ -clear_ci_info <- function() { - withr::local_envvar( - c( - JENKINS_URL = NA_character_, - GIT_URL = NA_character_, - GIT_URL_1 = NA_character_, - CHANGE_ID = NA_character_, - GIT_COMMIT = NA_character_ - ), - .local_envir = parent.frame() - ) -} - -test_that("it detects CI environments", { - clear_ci_info() - withr::with_envvar( - c(TRAVIS_REPO_SLUG = "foo/bar"), - expect_true(lintr:::in_ci()) - ) - withr::with_envvar( - c(TRAVIS_REPO_SLUG = ""), - expect_false(lintr:::in_ci()) - ) -}) - -test_that("it returns NULL if GIT_URL is not on github", { - clear_ci_info() - withr::local_envvar(c( - JENKINS_URL = "https://jenkins.example.org/", - GIT_URL = "https://example.com/user/repo.git", - CHANGE_ID = "123" - )) - expect_false(lintr:::in_ci()) -}) - - -test_that("it returns NULL for Jenkins PR build info when git URL is missing", { - clear_ci_info() - expect_null(lintr:::jenkins_build_info()) -}) - -test_that("it determines Jenkins PR build info", { - clear_ci_info() - withr::with_envvar( - c( - JENKINS_URL = "https://jenkins.example.org/", - GIT_URL = "https://github.com/user/repo.git", - CHANGE_ID = "123" - ), - { - expect_true(lintr:::in_ci()) - - expect_identical( - lintr:::ci_build_info(), - list(user = "user", repo = "repo", pull = "123", commit = NULL) - ) - } - ) - - withr::with_envvar( - list(JENKINS_URL = NULL, GIT_URL = NULL, CHANGE_ID = NULL), - expect_false(lintr:::in_ci()) - ) -}) - -test_that("it determines Jenkins commit build info", { - clear_ci_info() - withr::local_envvar(c( - JENKINS_URL = "https://jenkins.example.org/", - GIT_URL_1 = "https://github.com/user/repo.git", - GIT_COMMIT = "abcde" - )) - - expect_true(lintr:::in_ci()) - expect_identical(lintr:::ci_build_info(), list( - user = "user", - repo = "repo", - pull = NULL, - commit = "abcde" - )) -}) diff --git a/tests/testthat/test-fixed_regex_linter.R b/tests/testthat/test-fixed_regex_linter.R index 95ac7b09c..371848200 100644 --- a/tests/testthat/test-fixed_regex_linter.R +++ b/tests/testthat/test-fixed_regex_linter.R @@ -287,51 +287,67 @@ robust_non_printable_unicode <- function() { } # styler: off -patrick::with_parameters_test_that("fixed replacements are correct", { - skip_if( - regex_expr %in% c("abc\\U{A0DEF}ghi", "[\\U1d4d7]", "[\\U{1D4D7}]", "\\u{A0}\\U{0001d4d7}") && - .Platform$OS.type == "windows" && - !hasName(R.Version(), "crt"), - message = "UTF-8 support is required" +local({ + .cases <- tibble::tribble( + ~.test_name, ~regex_expr, ~fixed_expr, + "[.]", "[.]", ".", + '[\\\"]', '[\\\"]', '\\"', + "[]]", "[]]", "]", + "\\\\.", "\\\\.", ".", + "\\\\:", "\\\\:", ":", + "\\\\<", "\\\\<", "<", + "\\\\$", "\\\\$", "$", + "[\\1]", "[\\1]", "\\001", + "\\1", "\\1", "\\001", + "[\\12]", "[\\12]", "\\n", + "[\\123]", "[\\123]", "S", + "a[*]b", "a[*]b", "a*b", + "abcdefg", "abcdefg", "abcdefg", + "abc\\U{A0DEF}ghi", "abc\\U{A0DEF}ghi", robust_non_printable_unicode(), + "a-z", "a-z", "a-z", + "[\\n]", "[\\n]", "\\n", + "\\n", "\n", "\\n", + "[\\u01]", "[\\u01]", "\\001", + "[\\u012]", "[\\u012]", "\\022", + "[\\u0123]", "[\\u0123]", "\u0123", + "[\\u{1}]", "[\\u{1}]", "\\001", + "[\\U1d4d7]", "[\\U1d4d7]", "\U1D4D7", + "[\\U{1D4D7}]", "[\\U{1D4D7}]", "\U1D4D7", + "[\\U8]", "[\\U8]", "\\b", + "\\u{A0}", "\\u{A0}", "\uA0", + "\\u{A0}\\U{0001d4d7}", "\\u{A0}\\U{0001d4d7}", "\uA0\U1D4D7", + "[\\uF]", "[\\uF]", "\\017", + "[\\U{F7D5}]", "[\\U{F7D5}]", "\UF7D5", + "[\\x32]", "[\\x32]", "2", + "[\\xa]", "[\\xa]", "\\n" ) - expect_lint( - sprintf("grepl('%s', x)", regex_expr), - rex::rex(sprintf('Use "%s" with fixed = TRUE', fixed_expr)), - fixed_regex_linter() + if (.Platform$OS.type == "windows" && !hasName(R.Version(), "crt")) { + skip_cases <- c( + # These require UTF-8 support + "abc\\U{A0DEF}ghi", "[\\U1d4d7]", "[\\U{1D4D7}]", "\\u{A0}\\U{0001d4d7}", + # R version-specific difference in output message on Windows (probably r80051) + if (getRversion() == "4.0.4") "[\\U{F7D5}]" + ) + } else { + skip_cases <- character() + } + patrick::with_parameters_test_that( + "fixed replacements are correct", + { + # TODO(google/patrick#19): handle this more cleanly by skipping up-front + skip_if( + regex_expr %in% skip_cases, + sprintf("regex '%s' is not supported on this system", regex_expr) + ) + expect_lint( + sprintf("grepl('%s', x)", regex_expr), + rex::rex(sprintf('Use "%s" with fixed = TRUE', fixed_expr)), + fixed_regex_linter() + ) + }, + .cases = .cases ) -}, .cases = tibble::tribble( - ~.test_name, ~regex_expr, ~fixed_expr, - "[.]", "[.]", ".", - '[\\\"]', '[\\\"]', '\\"', - "[]]", "[]]", "]", - "\\\\.", "\\\\.", ".", - "\\\\:", "\\\\:", ":", - "\\\\<", "\\\\<", "<", - "\\\\$", "\\\\$", "$", - "[\\1]", "[\\1]", "\\001", - "\\1", "\\1", "\\001", - "[\\12]", "[\\12]", "\\n", - "[\\123]", "[\\123]", "S", - "a[*]b", "a[*]b", "a*b", - "abcdefg", "abcdefg", "abcdefg", - "abc\\U{A0DEF}ghi", "abc\\U{A0DEF}ghi", robust_non_printable_unicode(), - "a-z", "a-z", "a-z", - "[\\n]", "[\\n]", "\\n", - "\\n", "\n", "\\n", - "[\\u01]", "[\\u01]", "\\001", - "[\\u012]", "[\\u012]", "\\022", - "[\\u0123]", "[\\u0123]", "\u0123", - "[\\u{1}]", "[\\u{1}]", "\\001", - "[\\U1d4d7]", "[\\U1d4d7]", "\U1D4D7", - "[\\U{1D4D7}]", "[\\U{1D4D7}]", "\U1D4D7", - "[\\U8]", "[\\U8]", "\\b", - "\\u{A0}", "\\u{A0}", "\uA0", - "\\u{A0}\\U{0001d4d7}", "\\u{A0}\\U{0001d4d7}", "\uA0\U1D4D7", - "[\\uF]", "[\\uF]", "\\017", - "[\\U{F7D5}]", "[\\U{F7D5}]", "\UF7D5", - "[\\x32]", "[\\x32]", "2", - "[\\xa]", "[\\xa]", "\\n" -)) +}) # styler: on test_that("'unescaped' regex can optionally be skipped", { diff --git a/tests/testthat/test-get_source_expressions.R b/tests/testthat/test-get_source_expressions.R index 917a414ed..dbacecacc 100644 --- a/tests/testthat/test-get_source_expressions.R +++ b/tests/testthat/test-get_source_expressions.R @@ -93,8 +93,12 @@ test_that("Multi-byte character truncated by parser is ignored", { skip_if_not_utf8_locale() # \U2013 is the Unicode character 'en dash', which is # almost identical to a minus sign in monospaced fonts. - with_content_to_parse("y <- x \U2013 42", { - expect_identical(error$message, "unexpected input") + content <- "y <- x \U2013 42" + # message is like ':1:8: unexpected invalid token\n1: ...' + with_content_to_parse(content, { + base_msg <- conditionMessage(tryCatch(str2lang(content), error = identity)) + # Just ensure that the captured message is a substring of the parser error, #2527 + expect_true(grepl(error$message, base_msg, fixed = TRUE, useBytes = TRUE)) expect_identical(error$column_number, 8L) }) }) diff --git a/tests/testthat/test-lint_package.R b/tests/testthat/test-lint_package.R index 957b8af9c..f0d2c0c48 100644 --- a/tests/testthat/test-lint_package.R +++ b/tests/testthat/test-lint_package.R @@ -231,6 +231,9 @@ test_that("package using .lintr.R config lints correctly", { }) test_that("lintr need not be attached for .lintr.R configs to use lintr functions", { + # For some obscure reason, running in the subprocess on this specific version of R + # on Windows stopped working after PR #2446 with 'Package lintr not found'. + if (getRversion() == "3.6.3") skip_on_os("windows") exprs <- paste( 'options(lintr.linter_file = "lintr_test_config")', sprintf('lints <- lintr::lint_package("%s")', test_path("dummy_packages", "RConfig")), diff --git a/tests/testthat/test-settings.R b/tests/testthat/test-settings.R index a09012e22..bb3b71461 100644 --- a/tests/testthat/test-settings.R +++ b/tests/testthat/test-settings.R @@ -285,3 +285,19 @@ test_that("perl-only regular expressions are accepted in config", { writeLines("a <- 1", "aaa.R") expect_silent(lint("aaa.R")) }) + +test_that("settings can be put in a sub-directory", { + withr::local_dir(withr::local_tempdir()) + + dir.create(".settings") + .lintr <- ".settings/.lintr.R" + writeLines("linters <- list(line_length_linter(10))", .lintr) + + dir.create("R") + writeLines("abcdefghijklmnopqrstuvwxyz=1", "R/a.R") + + writeLines(c("Package: foo", "Version: 0.1"), "DESCRIPTION") + + withr::local_options(lintr.linter_file = .lintr) + expect_length(lint_package(), 1L) +}) diff --git a/tests/testthat/test-unnecessary_nesting_linter.R b/tests/testthat/test-unnecessary_nesting_linter.R index b2c47b1b6..a1114e965 100644 --- a/tests/testthat/test-unnecessary_nesting_linter.R +++ b/tests/testthat/test-unnecessary_nesting_linter.R @@ -325,7 +325,7 @@ test_that("rlang's double-brace operator is skipped", { test_that("unnecessary_nesting_linter blocks one-expression braced expressions", { expect_lint( trim_some(" - tryCatch( + tryToCatch( { foo(x) }, @@ -340,7 +340,7 @@ test_that("unnecessary_nesting_linter blocks one-expression braced expressions", test_that("unnecessary_nesting_linter allow_assignment= argument works", { expect_lint( trim_some(" - tryCatch( + tryToCatch( { idx <- foo(x) }, @@ -710,3 +710,26 @@ test_that("else that can drop braces is found", { linter ) }) + +patrick::with_parameters_test_that( + "default allowed functions are skipped", + expect_lint(sprintf("%s(x, {y}, z)", call), NULL, unnecessary_nesting_linter()), + call = c( + "test_that", "with_parameters_test_that", + "switch", + "try", "tryCatch", "withCallingHandlers", + "quote", "bquote", "expression", "substitute", + "observe", "observeEvent", "reactive", + "renderCachedPlot", "renderDataTable", "renderImage", "renderPlot", + "renderPrint", "renderTable", "renderText", "renderUI" + ) +) + +test_that("allow_functions= works", { + linter_default <- unnecessary_nesting_linter() + linter_foo <- unnecessary_nesting_linter(allow_functions = "foo") + expect_lint("foo(x, {y}, z)", "Reduce the nesting of this statement", linter_default) + expect_lint("foo(x, {y}, z)", NULL, linter_foo) + expect_lint("test_that('a', {y})", NULL, linter_default) + expect_lint("that_that('b', {y})", NULL, linter_foo) +}) diff --git a/vignettes/continuous-integration.Rmd b/vignettes/continuous-integration.Rmd index f5763eaaa..a07478923 100644 --- a/vignettes/continuous-integration.Rmd +++ b/vignettes/continuous-integration.Rmd @@ -51,27 +51,6 @@ Note that this will kill the R process in case of a lint. If your project is in a subdirectory and you would like to use GitHub Actions annotations, you can set `options(lintr.github_annotation_project_dir = "path/to/project")` which will make sure that the annotations point to the correct paths. -### Travis CI - -If you want to run `lintr` on [Travis-CI](https://www.travis-ci.com/), you will need to have Travis install the package first. -This can be done by adding the following line to your `.travis.yml` - -``` yaml -r_github_packages: - - r-lib/lintr -``` - -We recommend running `lintr::lint_package()` as an after_success step in your build process: - -``` yaml -after_success: - - R CMD INSTALL $PKG_TARBALL - - Rscript -e 'lintr::lint_package()' -``` - -If lints are found in the commit or pull request they will be printed on Travis-CI. -The environment variable `LINTR_ERROR_ON_LINT` mentioned for GitHub actions also works with Travis CI builds. - ## For projects You are not limited to using `lintr` for packages -- you can use it in combination with continuous integration for any other project.