diff --git a/.github/workflows/R-CMD-check-hard.yaml b/.github/workflows/R-CMD-check-hard.yaml new file mode 100644 index 000000000..e1b19825f --- /dev/null +++ b/.github/workflows/R-CMD-check-hard.yaml @@ -0,0 +1,65 @@ +# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples +# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help +# +# NOTE: This workflow only directly installs "hard" dependencies, i.e. Depends, +# Imports, and LinkingTo dependencies. Notably, Suggests dependencies are never +# installed, with the exception of testthat, knitr, and rmarkdown. The cache is +# never used to avoid accidentally restoring a cache containing a suggested +# dependency. +# +# Customization for `{lintr}` +# +# The following soft dependencies are not optional for testing: +# `{patrick}`, `{rex}`, `{withr}` +on: + push: + branches: [main, master] + pull_request: + branches: [main, master] + +name: R-CMD-check-hard + +jobs: + R-CMD-check: + runs-on: ${{ matrix.config.os }} + + name: ${{ matrix.config.os }} (${{ matrix.config.r }}) + + strategy: + fail-fast: false + matrix: + config: + - { os: ubuntu-latest, r: "release" } + + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + R_KEEP_PKG_SOURCE: yes + + steps: + - uses: actions/checkout@v3 + + - uses: r-lib/actions/setup-pandoc@v2 + + - uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.config.r }} + http-user-agent: ${{ matrix.config.http-user-agent }} + use-public-rspm: true + + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + pak-version: devel + dependencies: '"hard"' + cache: false + extra-packages: | + any::rcmdcheck + any::testthat + any::knitr + any::rmarkdown + any::patrick + any::withr + needs: check + + - uses: r-lib/actions/check-r-package@v2 + with: + upload-snapshots: true diff --git a/DESCRIPTION b/DESCRIPTION index a014ef6be..eaffe9ff4 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -41,7 +41,6 @@ Suggests: jsonlite, mockery, patrick, - pkgdown, rlang, rmarkdown, rstudioapi (>= 0.2), @@ -146,6 +145,7 @@ Collate: 'semicolon_linter.R' 'seq_linter.R' 'settings.R' + 'settings_utils.R' 'single_quotes_linter.R' 'spaces_inside_linter.R' 'spaces_left_parentheses_linter.R' diff --git a/NEWS.md b/NEWS.md index f3f67f0b9..61a33fadb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,6 +11,8 @@ * `object_usage_linter()` no longer silently ignores usage warnings that don't contain a quoted name (#1714, @AshesITR) +* `namespace_linter()` correctly recognizes backticked operators to be exported from respectives namespaces (like `` rlang::`%||%` ``) (#1752, @IndrajeetPatil) + ## Changes to defaults * Set the default for the `except` argument in `duplicate_argument_linter()` to `c("mutate", "transmute")`. diff --git a/R/lint.R b/R/lint.R index e6e33a1f9..6c0f0d349 100644 --- a/R/lint.R +++ b/R/lint.R @@ -379,73 +379,6 @@ reorder_lints <- function(lints) { } -has_description <- function(path) { - desc_info <- file.info(file.path(path, "DESCRIPTION")) - !is.na(desc_info$size) && desc_info$size > 0.0 && !desc_info$isdir -} - -find_package <- function(path) { - depth <- 2L - while (!has_description(path)) { - path <- dirname(path) - if (is_root(path) || depth <= 0L) { - return(NULL) - } - depth <- depth - 1L - } - path -} - -find_rproj_or_package <- function(path) { - path <- normalizePath(path, mustWork = FALSE) - - depth <- 2L - while (!(has_description(path) || has_rproj(path))) { - path <- dirname(path) - if (is_root(path) || depth <= 0L) { - return(NULL) - } - depth <- depth - 1L - } - path -} - -has_rproj <- function(path) { - length(head(Sys.glob(file.path(path, "*.Rproj")), n = 1L)) == 1L -} - -find_rproj_at <- function(path) { - head(Sys.glob(file.path(path, "*.Rproj")), n = 1L) -} -is_root <- function(path) { - identical(path, dirname(path)) -} - -has_config <- function(path, config) { - file.exists(file.path(path, config)) -} - -find_config2 <- function(path) { - config <- basename(getOption("lintr.linter_file")) - path <- normalizePath(path, mustWork = FALSE) - - while (!has_config(path, config)) { - path <- dirname(path) - if (is_root(path)) { - return(character()) - } - } - return(file.path(path, config)) -} - -pkg_name <- function(path = find_package()) { - if (is.null(path)) { - return(NULL) - } else { - read.dcf(file.path(path, "DESCRIPTION"), fields = "Package")[1L] - } -} - #' Create a `lint` object #' @param filename path to the source file that was linted. #' @param line_number line number where the lint occurred. diff --git a/R/namespace_linter.R b/R/namespace_linter.R index 49e4ce3e7..2d2d5c4c6 100644 --- a/R/namespace_linter.R +++ b/R/namespace_linter.R @@ -167,6 +167,9 @@ build_ns_get_int_lints <- function(packages, symbols, symbol_nodes, namespaces, build_ns_get_lints <- function(packages, symbols, symbol_nodes, namespaces, source_expression) { lints <- list() + # strip backticked symbols; `%>%` is the same as %>% (#1752). + symbols <- gsub("^`(.*)`$", "\\1", symbols) + ## Case 4: foo is not an export in pkg::foo unexported <- !vapply( diff --git a/R/settings.R b/R/settings.R index 2ca15792c..7e47ffb8d 100644 --- a/R/settings.R +++ b/R/settings.R @@ -64,46 +64,6 @@ clear_settings <- function() { rm(list = ls(settings), envir = settings) } -find_config <- function(filename) { - if (is.null(filename)) { - return(NULL) - } - linter_file <- getOption("lintr.linter_file") - - ## if users changed lintr.linter_file, return immediately. - if (is_absolute_path(linter_file) && file.exists(linter_file)) { - return(linter_file) - } - - path <- if (is_directory(filename)) { - filename - } else { - dirname(filename) - } - - ## check for a file in the current directory - linter_config <- file.path(path, linter_file) - if (isTRUE(file.exists(linter_config))) { - return(linter_config) - } - - ## next check for a file higher directories - linter_config <- find_config2(path) - if (isTRUE(file.exists(linter_config))) { - return(linter_config) - } - - ## next check for a file in the user directory - # cf: rstudio@bc9b6a5 SessionRSConnect.R#L32 - home_dir <- Sys.getenv("HOME", unset = "~") - linter_config <- file.path(home_dir, linter_file) - if (isTRUE(file.exists(linter_config))) { - return(linter_config) - } - - NULL -} - find_default_encoding <- function(filename) { if (is.null(filename)) { return(NULL) @@ -138,9 +98,3 @@ get_encoding_from_dcf <- function(file) { NULL } - -is_directory <- function(filename) { - is_dir <- file.info(filename)$isdir - - !is.na(is_dir) && is_dir -} diff --git a/R/settings_utils.R b/R/settings_utils.R new file mode 100644 index 000000000..20e43e940 --- /dev/null +++ b/R/settings_utils.R @@ -0,0 +1,113 @@ +has_description <- function(path) { + desc_info <- file.info(file.path(path, "DESCRIPTION")) + !is.na(desc_info$size) && desc_info$size > 0.0 && !desc_info$isdir +} + +find_package <- function(path) { + depth <- 2L + while (!has_description(path)) { + path <- dirname(path) + if (is_root(path) || depth <= 0L) { + return(NULL) + } + depth <- depth - 1L + } + path +} + +find_rproj_or_package <- function(path) { + path <- normalizePath(path, mustWork = FALSE) + + depth <- 2L + while (!(has_description(path) || has_rproj(path))) { + path <- dirname(path) + if (is_root(path) || depth <= 0L) { + return(NULL) + } + depth <- depth - 1L + } + path +} + +has_rproj <- function(path) { + length(head(Sys.glob(file.path(path, "*.Rproj")), n = 1L)) == 1L +} + +find_rproj_at <- function(path) { + head(Sys.glob(file.path(path, "*.Rproj")), n = 1L) +} + +is_root <- function(path) { + identical(path, dirname(path)) +} + +is_directory <- function(filename) { + is_dir <- file.info(filename)$isdir + + !is.na(is_dir) && is_dir +} + +has_config <- function(path, config) { + file.exists(file.path(path, config)) +} + +find_config <- function(filename) { + if (is.null(filename)) { + return(NULL) + } + linter_file <- getOption("lintr.linter_file") + + ## if users changed lintr.linter_file, return immediately. + if (is_absolute_path(linter_file) && file.exists(linter_file)) { + return(linter_file) + } + + path <- if (is_directory(filename)) { + filename + } else { + dirname(filename) + } + + ## check for a file in the current directory + linter_config <- file.path(path, linter_file) + if (isTRUE(file.exists(linter_config))) { + return(linter_config) + } + + ## next check for a file higher directories + linter_config <- find_config2(path) + if (isTRUE(file.exists(linter_config))) { + return(linter_config) + } + + ## next check for a file in the user directory + # cf: rstudio@bc9b6a5 SessionRSConnect.R#L32 + home_dir <- Sys.getenv("HOME", unset = "~") + linter_config <- file.path(home_dir, linter_file) + if (isTRUE(file.exists(linter_config))) { + return(linter_config) + } + + NULL +} + +find_config2 <- function(path) { + config <- basename(getOption("lintr.linter_file")) + path <- normalizePath(path, mustWork = FALSE) + + while (!has_config(path, config)) { + path <- dirname(path) + if (is_root(path)) { + return(character()) + } + } + return(file.path(path, config)) +} + +pkg_name <- function(path = find_package()) { + if (is.null(path)) { + return(NULL) + } else { + read.dcf(file.path(path, "DESCRIPTION"), fields = "Package")[1L] + } +} diff --git a/tests/testthat/test-cache.R b/tests/testthat/test-cache.R index f4e1d463e..0e72fd35f 100644 --- a/tests/testthat/test-cache.R +++ b/tests/testthat/test-cache.R @@ -34,6 +34,8 @@ fhash <- function(filename) { # `clear_cache` test_that("clear_cache deletes the file if a file is given", { + skip_if_not_installed("mockery") + mockery::stub(clear_cache, "read_settings", function(...) invisible(...)) mockery::stub(clear_cache, "unlink", function(...) list(...)) @@ -48,6 +50,8 @@ test_that("clear_cache deletes the file if a file is given", { }) test_that("clear_cache deletes the directory if no file is given", { + skip_if_not_installed("mockery") + mockery::stub(clear_cache, "read_settings", function(...) invisible(...)) mockery::stub(clear_cache, "unlink", function(...) list(...)) @@ -408,6 +412,7 @@ test_that("lint with cache uses the provided relative cache directory", { }) test_that("it works outside of a package", { + skip_if_not_installed("mockery") linter <- assignment_linter() mockery::stub(lintr:::find_default_encoding, "find_package", function(...) NULL) diff --git a/tests/testthat/test-ci.R b/tests/testthat/test-ci.R index 7f1a69823..6da47a84f 100644 --- a/tests/testthat/test-ci.R +++ b/tests/testthat/test-ci.R @@ -27,6 +27,8 @@ test_that("GitHub Actions functionality works in a subdirectory", { }) test_that("GitHub Actions - linting on error works", { + skip_if_not_installed("mockery") + # imitate being on GHA whether or not we are withr::local_envvar(list(GITHUB_ACTIONS = "true", LINTR_ERROR_ON_LINT = "true")) withr::local_options(lintr.rstudio_source_markers = FALSE) @@ -39,6 +41,8 @@ test_that("GitHub Actions - linting on error works", { }) test_that("Printing works for Travis", { + skip_if_not_installed("mockery") + 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)") @@ -50,6 +54,8 @@ test_that("Printing works for Travis", { }) test_that("Printing works for Wercker", { + skip_if_not_installed("mockery") + 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)") diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index e4020eb27..a736a7bc6 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -108,6 +108,8 @@ test_that("print.lint works", { }) test_that("print.lint works for inline data, even in RStudio", { + skip_if_not_installed("mockery") + l <- lint("x = 1\n") # Make sure lints print to console. diff --git a/tests/testthat/test-namespace_linter.R b/tests/testthat/test-namespace_linter.R index fba9d7624..fd3686fc8 100644 --- a/tests/testthat/test-namespace_linter.R +++ b/tests/testthat/test-namespace_linter.R @@ -18,6 +18,23 @@ test_that("namespace_linter respects check_exports and check_nonexports argument expect_lint("stats:::ssd(c(1,2,3))", NULL, namespace_linter(check_exports = FALSE, check_nonexports = FALSE)) }) +test_that("namespace_linter can work with backticked symbols", { + skip_if_not_installed("rlang") + linter <- namespace_linter() + + expect_lint("rlang::`%||%`", NULL, linter) + expect_lint("rlang::`%||%`()", NULL, linter) + + expect_lint("rlang::'%||%'", NULL, linter) + expect_lint("rlang::'%||%'()", NULL, linter) + expect_lint('rlang::"%||%"', NULL, linter) + expect_lint('rlang::"%||%"()', NULL, linter) + + expect_lint("rlang::`%>%`", "'%>%' is not exported from {rlang}.", linter) + expect_lint("rlang::'%>%'()", "'%>%' is not exported from {rlang}.", linter) + expect_lint('rlang::"%>%"()', "'%>%' is not exported from {rlang}.", linter) +}) + test_that("namespace_linter blocks disallowed usages", { linter <- namespace_linter() diff --git a/tests/testthat/test-rstudio_markers.R b/tests/testthat/test-rstudio_markers.R index 523e184a9..b6896780e 100644 --- a/tests/testthat/test-rstudio_markers.R +++ b/tests/testthat/test-rstudio_markers.R @@ -1,4 +1,6 @@ test_that("it returns markers which match lints", { + skip_if_not_installed("mockery") + mockery::stub(rstudio_source_markers, "rstudioapi::callFun", function(...) list(...)) mockery::stub(rstudio_source_markers, "rstudioapi::executeCommand", function(...) NULL) @@ -57,6 +59,8 @@ test_that("it returns markers which match lints", { }) test_that("it prepends the package path if it exists", { + skip_if_not_installed("mockery") + mockery::stub(rstudio_source_markers, "rstudioapi::callFun", function(...) list(...)) mockery::stub(rstudio_source_markers, "rstudioapi::executeCommand", function(...) NULL) @@ -86,6 +90,8 @@ test_that("it prepends the package path if it exists", { }) test_that("it returns an empty list of markers if there are no lints", { + skip_if_not_installed("mockery") + mockery::stub(rstudio_source_markers, "rstudioapi::callFun", function(...) list(...)) mockery::stub(rstudio_source_markers, "rstudioapi::executeCommand", function(...) NULL) @@ -99,6 +105,8 @@ test_that("it returns an empty list of markers if there are no lints", { }) test_that("rstudio_source_markers apply to print within rstudio", { + skip_if_not_installed("mockery") + withr::local_options(lintr.rstudio_source_markers = TRUE) tmp <- withr::local_tempfile(lines = "1:ncol(x)") diff --git a/tests/testthat/test-settings.R b/tests/testthat/test-settings.R index 91fc1cc04..ea3ec43c0 100644 --- a/tests/testthat/test-settings.R +++ b/tests/testthat/test-settings.R @@ -110,8 +110,8 @@ test_that("it has a smart default for encodings", { normalizePath(test_path("dummy_packages", "cp1252"), winslash = "/") ) - expect_identical(find_default_encoding(proj_file), "ISO8859-1") - expect_identical(find_default_encoding(pkg_file), "ISO8859-1") + expect_identical(lintr:::find_default_encoding(proj_file), "ISO8859-1") + expect_identical(lintr:::find_default_encoding(pkg_file), "ISO8859-1") lintr:::read_settings(proj_file) expect_identical(settings$encoding, "ISO8859-1") diff --git a/vignettes/editors.Rmd b/vignettes/editors.Rmd index 11165a018..b58890de1 100644 --- a/vignettes/editors.Rmd +++ b/vignettes/editors.Rmd @@ -13,8 +13,10 @@ knitr::opts_chunk$set( comment = "#>" ) +in_pkgdown <- identical(Sys.getenv("IN_PKGDOWN"), "true") + maybe_still <- function(url) { - if (pkgdown::in_pkgdown()) { + if (in_pkgdown) { url } else { gsub("\\.gif$", "-still.gif", url) @@ -23,7 +25,7 @@ maybe_still <- function(url) { ``` ```{r, echo = FALSE, results = 'asis'} -if (!pkgdown::in_pkgdown()) { +if (!in_pkgdown) { cat( "Note: This vignette is best viewed [online](https://lintr.r-lib.org/articles/editors.html),", "where we can render full animations of editor flows.\n"