From 1c2c8a9bd8aa010fc3089bc1936324dc7332ec6b Mon Sep 17 00:00:00 2001 From: olivroy <52606734+olivroy@users.noreply.github.com> Date: Thu, 20 Jun 2024 02:04:20 -0400 Subject: [PATCH 01/17] Normalize output to Forward slash (#2613) * Normalize to forward slash * add news * lint * Add `normalizePath()` to undesirable_functions * Use normalize_path everywhere and remove unneeded `fsep` * fix news typo * Use nolint + add comment * Address comments * less whitespace * even simpler --------- Co-authored-by: Michael Chirico --- .lintr | 3 ++- NEWS.md | 1 + R/exclude.R | 2 +- R/lint.R | 13 +++++++------ R/path_utils.R | 6 ++++++ R/settings_utils.R | 4 ++-- R/use_lintr.R | 2 +- tests/testthat/test-cache.R | 2 +- tests/testthat/test-lint.R | 2 +- tests/testthat/test-lint_dir.R | 2 +- tests/testthat/test-normalize_exclusions.R | 8 ++++---- tests/testthat/test-settings.R | 8 ++++---- tests/testthat/test-use_lintr.R | 8 ++++---- 13 files changed, 35 insertions(+), 26 deletions(-) diff --git a/.lintr b/.lintr index 0d4e1af9f..dd04ff1d4 100644 --- a/.lintr +++ b/.lintr @@ -17,7 +17,8 @@ linters: all_linters( options = NULL, message = "use cli::cli_inform()", warning = "use cli::cli_warn()", - stop = "use cli::cli_abort()" + stop = "use cli::cli_abort()", + normalizePath = "use normalize_path()" )), undesirable_operator_linter(modify_defaults( defaults = default_undesirable_operators, diff --git a/NEWS.md b/NEWS.md index f09fbe60e..140ee54ea 100644 --- a/NEWS.md +++ b/NEWS.md @@ -17,6 +17,7 @@ * 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) +* `lint()` and friends now normalize paths to forward slashes on Windows (@olivroy, #2613). ## Bug fixes diff --git a/R/exclude.R b/R/exclude.R index f3bbc5405..25a924cea 100644 --- a/R/exclude.R +++ b/R/exclude.R @@ -326,7 +326,7 @@ normalize_exclusions <- function(x, normalize_path = TRUE, paths[rel_path] <- file.path(root, paths[rel_path]) names(x) <- paths x <- x[file.exists(paths)] # remove exclusions for non-existing files - names(x) <- normalizePath(names(x)) # get full path for remaining files + names(x) <- normalize_path(names(x)) # get full path for remaining files } remove_line_duplicates( diff --git a/R/lint.R b/R/lint.R index 61dc95254..042fbeaca 100644 --- a/R/lint.R +++ b/R/lint.R @@ -52,7 +52,7 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings = close(con) } - filename <- normalizePath(filename, mustWork = !inline_data) # to ensure a unique file in cache + filename <- normalize_path(filename, mustWork = !inline_data) # to ensure a unique file in cache source_expressions <- get_source_expressions(filename, lines) if (isTRUE(parse_settings)) { @@ -163,9 +163,10 @@ lint_dir <- function(path = ".", ..., pattern = pattern ) - # normalizePath ensures names(exclusions) and files have the same names for the same files. + # normalize_path ensures names(exclusions) and files have the same names for the same files. + # It also ensures all paths have forward slash # Otherwise on windows, files might incorrectly not be excluded in to_exclude - files <- normalizePath(dir( + files <- normalize_path(dir( path, pattern = pattern, recursive = TRUE, @@ -198,7 +199,7 @@ lint_dir <- function(path = ".", ..., lints <- reorder_lints(lints) if (relative_path) { - path <- normalizePath(path, mustWork = FALSE) + path <- normalize_path(path, mustWork = FALSE) lints[] <- lapply( lints, function(x) { @@ -249,7 +250,7 @@ lint_package <- function(path = ".", ..., if (is.null(pkg_path)) { cli_warn(c( - i = "Didn't find any R package searching upwards from {.file {normalizePath(path)}}" + i = "Didn't find any R package searching upwards from {.file {normalize_path(path)}}" )) return(NULL) } @@ -274,7 +275,7 @@ lint_package <- function(path = ".", ..., ) if (isTRUE(relative_path)) { - path <- normalizePath(pkg_path, mustWork = FALSE) + path <- normalize_path(pkg_path, mustWork = FALSE) lints[] <- lapply( lints, function(x) { diff --git a/R/path_utils.R b/R/path_utils.R index dd53d4316..f8ac2ac6f 100644 --- a/R/path_utils.R +++ b/R/path_utils.R @@ -133,6 +133,12 @@ split_path <- function(dirs, prefix) { dirs[nzchar(dirs)] } +#' Simple wrapper around normalizePath to ensure forward slash on Windows +#' https://github.com/r-lib/lintr/pull/2613 +#' @noRd +# nolint next: undesirable_function_linter, object_name_linter. +normalize_path <- function(path, mustWork = NA) normalizePath(path = path, winslash = "/", mustWork = mustWork) + #' @include utils.R path_linter_factory <- function(path_function, message, linter, name = linter_auto_name()) { force(name) diff --git a/R/settings_utils.R b/R/settings_utils.R index 9489ea24e..02564efe5 100644 --- a/R/settings_utils.R +++ b/R/settings_utils.R @@ -8,7 +8,7 @@ has_rproj <- function(path) { } find_package <- function(path, allow_rproj = FALSE, max_depth = 2L) { - path <- normalizePath(path, mustWork = !allow_rproj) + path <- normalize_path(path, mustWork = !allow_rproj) if (allow_rproj) { found <- function(path) has_description(path) || has_rproj(path) } else { @@ -68,7 +68,7 @@ find_config <- function(filename) { dirname(filename) } - path <- normalizePath(path, mustWork = FALSE) + path <- normalize_path(path, mustWork = FALSE) # NB: This vector specifies a priority order for where to find the configs, # i.e. the first location where a config exists is chosen and configs which diff --git a/R/use_lintr.R b/R/use_lintr.R index 16b33f279..73dc72e38 100644 --- a/R/use_lintr.R +++ b/R/use_lintr.R @@ -25,7 +25,7 @@ #' lintr::lint_dir() #' } use_lintr <- function(path = ".", type = c("tidyverse", "full")) { - config_file <- normalizePath(file.path(path, lintr_option("linter_file")), mustWork = FALSE) + config_file <- normalize_path(file.path(path, lintr_option("linter_file")), mustWork = FALSE) if (file.exists(config_file)) { cli_abort("Found an existing configuration file at {.file {config_file}}.") } diff --git a/tests/testthat/test-cache.R b/tests/testthat/test-cache.R index 62a0ed32c..a0024708b 100644 --- a/tests/testthat/test-cache.R +++ b/tests/testthat/test-cache.R @@ -437,7 +437,7 @@ test_that("cache = TRUE workflow works", { # Need a test structure with a safe to load .lintr 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)) + files <- normalize_path(list.files(recursive = TRUE, full.names = TRUE)) # Manually clear cache (that function is exported) for (f in files) { diff --git a/tests/testthat/test-lint.R b/tests/testthat/test-lint.R index 6655f54dc..84ad15bc1 100644 --- a/tests/testthat/test-lint.R +++ b/tests/testthat/test-lint.R @@ -118,7 +118,7 @@ test_that("lint() results from file or text should be consistent", { lines <- c("x<-1", "x+1") file <- withr::local_tempfile(lines = lines) text <- paste(lines, collapse = "\n") - file <- normalizePath(file) + file <- normalize_path(file) lint_from_file <- lint(file, linters = linters) lint_from_lines <- lint(linters = linters, text = lines) diff --git a/tests/testthat/test-lint_dir.R b/tests/testthat/test-lint_dir.R index 24345ecd2..6da4b81bb 100644 --- a/tests/testthat/test-lint_dir.R +++ b/tests/testthat/test-lint_dir.R @@ -71,7 +71,7 @@ test_that("respects directory exclusions", { lints_norm <- lint_dir(the_dir, exclusions = "exclude-me", relative_path = FALSE) linted_files <- unique(names(lints_norm)) expect_length(linted_files, 1L) - expect_identical(linted_files, normalizePath(file.path(the_dir, "default_linter_testcode.R"))) + expect_identical(linted_files, normalize_path(file.path(the_dir, "default_linter_testcode.R"))) }) test_that("respect directory exclusions from settings", { diff --git a/tests/testthat/test-normalize_exclusions.R b/tests/testthat/test-normalize_exclusions.R index f2b505e8b..a3c779988 100644 --- a/tests/testthat/test-normalize_exclusions.R +++ b/tests/testthat/test-normalize_exclusions.R @@ -8,9 +8,9 @@ a <- withr::local_tempfile() b <- withr::local_tempfile() c <- withr::local_tempfile(tmpdir = ".") file.create(a, b, c) -a <- normalizePath(a) -b <- normalizePath(b) -c <- normalizePath(c) +a <- normalize_path(a) +b <- normalize_path(b) +c <- normalize_path(c) test_that("it merges two NULL or empty objects as an empty list", { expect_identical(lintr:::normalize_exclusions(c(NULL, NULL)), list()) @@ -132,7 +132,7 @@ test_that("it normalizes file paths, removing non-existing files", { t3[[c]] <- 5L:15L res <- list() res[[a]] <- list(1L:10L) - res[[normalizePath(c)]] <- list(5L:15L) + res[[c]] <- list(5L:15L) expect_identical(lintr:::normalize_exclusions(c(t1, t2, t3)), res) res <- list() diff --git a/tests/testthat/test-settings.R b/tests/testthat/test-settings.R index 2c48c1629..1653735e2 100644 --- a/tests/testthat/test-settings.R +++ b/tests/testthat/test-settings.R @@ -117,12 +117,12 @@ test_that("it has a smart default for encodings", { pkg_file <- test_path("dummy_packages", "cp1252", "R", "cp1252.R") expect_identical( - normalizePath(find_rproj_at(find_package(proj_file, allow_rproj = TRUE)), winslash = "/"), - normalizePath(test_path("dummy_projects", "project", "project.Rproj"), winslash = "/") + normalize_path(find_rproj_at(find_package(proj_file, allow_rproj = TRUE))), + normalize_path(test_path("dummy_projects", "project", "project.Rproj")) ) expect_identical( - normalizePath(find_package(pkg_file), winslash = "/"), - normalizePath(test_path("dummy_packages", "cp1252"), winslash = "/") + normalize_path(find_package(pkg_file)), + normalize_path(test_path("dummy_packages", "cp1252")) ) expect_identical(lintr:::find_default_encoding(proj_file), "ISO8859-1") diff --git a/tests/testthat/test-use_lintr.R b/tests/testthat/test-use_lintr.R index 68e089322..f56d3b304 100644 --- a/tests/testthat/test-use_lintr.R +++ b/tests/testthat/test-use_lintr.R @@ -6,8 +6,8 @@ test_that("use_lintr works as expected", { # check that newly created file is in the root directory expect_identical( - normalizePath(lintr_file, winslash = "/"), - file.path(normalizePath(tmp, winslash = "/"), ".lintr") + normalize_path(lintr_file), + file.path(normalize_path(tmp), ".lintr") ) # can't generate if a .lintr already exists @@ -28,8 +28,8 @@ test_that("use_lintr with type = full also works", { # check that newly created file is in the root directory expect_identical( - normalizePath(lintr_file, winslash = "/"), - file.path(normalizePath(tmp, winslash = "/"), ".lintr") + normalize_path(lintr_file), + file.path(normalize_path(tmp), ".lintr") ) lints <- lint_dir(tmp) From 6ed78c2373ea435932151a5ae2090a3efc346e25 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 20 Jun 2024 23:06:23 -0700 Subject: [PATCH 02/17] Use deparse1 (#2617) --- R/utils.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index 531ac1d89..d23fc8902 100644 --- a/R/utils.R +++ b/R/utils.R @@ -72,7 +72,7 @@ auto_names <- function(x) { if (is_linter(x)) { attr(x, "name", exact = TRUE) } else { - paste(deparse(x, 500L), collapse = " ") + deparse1(x) } } defaults <- vapply(x[empty], default_name, character(1L), USE.NAMES = FALSE) From 03bd51485618ad66ad7b5a34e2a8c24f0bd59bc1 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sat, 22 Jun 2024 08:46:07 +0200 Subject: [PATCH 03/17] Leave out some vignette metadata (#2620) --- vignettes/creating_linters.Rmd | 2 -- vignettes/lintr.Rmd | 1 - 2 files changed, 3 deletions(-) diff --git a/vignettes/creating_linters.Rmd b/vignettes/creating_linters.Rmd index 7301ff680..b38dd7a5d 100644 --- a/vignettes/creating_linters.Rmd +++ b/vignettes/creating_linters.Rmd @@ -1,7 +1,5 @@ --- title: "Creating new linters" -author: "lintr maintainers" -date: "`r Sys.Date()`" output: rmarkdown::html_vignette vignette: > %\VignetteIndexEntry{Creating new linters} diff --git a/vignettes/lintr.Rmd b/vignettes/lintr.Rmd index b819c9a12..275b864e1 100644 --- a/vignettes/lintr.Rmd +++ b/vignettes/lintr.Rmd @@ -1,6 +1,5 @@ --- title: "Using lintr" -author: "Alexander Rosenstock" output: rmarkdown::html_vignette vignette: > %\VignetteIndexEntry{Using lintr} From 3419159633067d2e6799854ec3cf5ff31bce15d0 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sat, 29 Jun 2024 00:27:53 +0200 Subject: [PATCH 04/17] Inclue partial match warnings in vigilant workflow (#2623) --- .github/workflows/test-package-vigilant.yaml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-package-vigilant.yaml b/.github/workflows/test-package-vigilant.yaml index d7c7c7047..414385293 100644 --- a/.github/workflows/test-package-vigilant.yaml +++ b/.github/workflows/test-package-vigilant.yaml @@ -27,7 +27,13 @@ jobs: - name: Run Tests run: | ## -------------------------------------------------------------------- - options(crayon.enabled = TRUE, warn = 2L) + options( + crayon.enabled = TRUE, + warn = 2L, + warnPartialMatchArgs = TRUE, + warnPartialMatchAttr = TRUE, + warnPartialMatchDollar = TRUE + ) if (Sys.getenv("_R_CHECK_FORCE_SUGGESTS_", "") == "") Sys.setenv("_R_CHECK_FORCE_SUGGESTS_" = "false") testthat::test_dir("tests") shell: Rscript {0} From 48b36ce3a281c12368af0c5a381a4681c2a00f9d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 8 Jul 2024 08:16:31 +0200 Subject: [PATCH 05/17] Bump JamesIves/github-pages-deploy-action from 4.6.1 to 4.6.3 (#2624) --- .github/workflows/pkgdown.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pkgdown.yaml b/.github/workflows/pkgdown.yaml index dad368915..3d75fb082 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.1 + uses: JamesIves/github-pages-deploy-action@v4.6.3 with: clean: false branch: gh-pages From 2fa3004f29c67e1003fb04fe69f431a75bb6da46 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Thu, 11 Jul 2024 19:51:54 +0200 Subject: [PATCH 06/17] Add new section for grouped linters in website reference index (#2629) * Add new section for grouped linters * Update _pkgdown.yaml --- _pkgdown.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/_pkgdown.yaml b/_pkgdown.yaml index 8fd54393f..be7ae356f 100644 --- a/_pkgdown.yaml +++ b/_pkgdown.yaml @@ -26,6 +26,10 @@ reference: contents: - ends_with("linter") + - title: Groups of linters + contents: + - ends_with("linters") + - title: Common default configurations contents: - all_undesirable_functions @@ -46,7 +50,6 @@ reference: - title: Meta-tooling contents: - - ends_with("linters") - Lint - checkstyle_output - sarif_output From 73e55d31f6ead2908fcd9a49d6ccf46524b1cccc Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Thu, 11 Jul 2024 23:12:35 +0200 Subject: [PATCH 07/17] Allow users to switch between dark and light mode (#2630) cf. https://pkgdown.r-lib.org/articles/customise.html?q=dark%20#light-switch --- _pkgdown.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/_pkgdown.yaml b/_pkgdown.yaml index be7ae356f..da3dbc60e 100644 --- a/_pkgdown.yaml +++ b/_pkgdown.yaml @@ -2,6 +2,7 @@ url: https://lintr.r-lib.org template: bootstrap: 5 + light-switch: true includes: in_header: | From f0d9407306d9c6d2f049ac6f698d2806d44a989c Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Thu, 18 Jul 2024 22:40:25 +0200 Subject: [PATCH 08/17] Don't include GHA log output in pkgdown website (#2633) * Don't include GHA log output in pkgdown website * add a test * extra blank line * make future-proof --------- Co-authored-by: Michael Chirico --- R/actions.R | 4 ++++ R/methods.R | 2 +- tests/testthat/test-ci.R | 13 +++++++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/R/actions.R b/R/actions.R index 9a20a1986..606442b8f 100644 --- a/R/actions.R +++ b/R/actions.R @@ -2,6 +2,10 @@ in_github_actions <- function() { identical(Sys.getenv("GITHUB_ACTIONS"), "true") } +in_pkgdown <- function() { + identical(Sys.getenv("IN_PKGDOWN"), "true") +} + # Output logging commands for any lints found github_actions_log_lints <- function(lints, project_dir = "") { for (x in lints) { diff --git a/R/methods.R b/R/methods.R index 46e8ed804..29a974d48 100644 --- a/R/methods.R +++ b/R/methods.R @@ -92,7 +92,7 @@ print.lints <- function(x, ...) { inline_data <- x[[1L]][["filename"]] == "" if (!inline_data && use_rstudio_source_markers) { rstudio_source_markers(x) - } else if (in_github_actions()) { + } else if (in_github_actions() && !in_pkgdown()) { github_actions_log_lints(x, project_dir = github_annotation_project_dir) } else { lapply(x, print, ...) diff --git a/tests/testthat/test-ci.R b/tests/testthat/test-ci.R index 3f4f98762..40401e934 100644 --- a/tests/testthat/test-ci.R +++ b/tests/testthat/test-ci.R @@ -1,5 +1,5 @@ test_that("GitHub Actions functionality works", { - withr::local_envvar(list(GITHUB_ACTIONS = "true")) + withr::local_envvar(list(GITHUB_ACTIONS = "true", IN_PKGDOWN = "false")) withr::local_options(lintr.rstudio_source_markers = FALSE) tmp <- withr::local_tempfile(lines = "x <- 1:nrow(y)") @@ -26,7 +26,7 @@ test_that("GitHub Actions functionality works in a subdirectory", { patrick::with_parameters_test_that( "GitHub Actions - error on lint works", { - withr::local_envvar(list(GITHUB_ACTIONS = "true", LINTR_ERROR_ON_LINT = env_var_value)) + withr::local_envvar(list(GITHUB_ACTIONS = "true", IN_PKGDOWN = "", LINTR_ERROR_ON_LINT = env_var_value)) withr::local_options(lintr.rstudio_source_markers = FALSE) tmp <- withr::local_tempfile(lines = "x <- 1:nrow(y)") @@ -51,3 +51,12 @@ patrick::with_parameters_test_that( }, env_var_value = list("", "F", NA, NULL) ) + +test_that("GitHub Actions log is skipped in pkgdown websites", { + withr::local_envvar(list(GITHUB_ACTIONS = "true", IN_PKGDOWN = "true")) + withr::local_options(lintr.rstudio_source_markers = FALSE) + tmp <- withr::local_tempfile(lines = "x <- 1:nrow(y)") + + l <- lint(tmp, linters = seq_linter()) + expect_output(print(l), "warning: [seq_linter]", fixed = TRUE) +}) From f2da882ce6d8889842b8b46294b367ff734f6513 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Wed, 24 Jul 2024 11:00:30 +0200 Subject: [PATCH 09/17] Fix missing DCF code block on website (#2635) --- DESCRIPTION | 2 +- R/settings.R | 2 +- man/read_settings.Rd | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 4831fe875..ff326321a 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -55,7 +55,7 @@ Config/Needs/development: pkgload, cli, testthat, patrick Config/testthat/edition: 3 Encoding: UTF-8 Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.1 +RoxygenNote: 7.3.2 Collate: 'make_linter_from_xpath.R' 'xp_utils.R' diff --git a/R/settings.R b/R/settings.R index 9bd7da159..475467335 100644 --- a/R/settings.R +++ b/R/settings.R @@ -12,7 +12,7 @@ #' This file is a DCF file, see [base::read.dcf()] for details. #' Here is an example of a `.lintr` file: #' -#' ```dcf +#' ``` #' linters: linters_with_defaults( #' any_duplicated_linter(), #' any_is_na_linter(), diff --git a/man/read_settings.Rd b/man/read_settings.Rd index cbb1dc9ec..84e53666e 100644 --- a/man/read_settings.Rd +++ b/man/read_settings.Rd @@ -27,7 +27,7 @@ or the environment variable \code{R_LINTR_LINTER_FILE} This file is a DCF file, see \code{\link[base:dcf]{base::read.dcf()}} for details. Here is an example of a \code{.lintr} file: -\if{html}{\out{
}}\preformatted{linters: linters_with_defaults( +\if{html}{\out{
}}\preformatted{linters: linters_with_defaults( any_duplicated_linter(), any_is_na_linter(), backport_linter("oldrel-4", except = c("R_user_dir", "str2lang")), From 3358539520a089e8267bdbbe0764537b96c24414 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Mon, 5 Aug 2024 22:38:34 +0200 Subject: [PATCH 10/17] Print a message when no lints found (#2643) * Print a message when no lints found closes #2640 * use info markup * Update NEWS.md * delint * more inlining * use single if-else * Create a separate NEWS entry * don't install quarto to avoid NOTE --- .github/workflows/R-CMD-check-hard.yaml | 1 + .github/workflows/R-CMD-check.yaml | 1 + NEWS.md | 1 + R/methods.R | 10 +++++++--- tests/testthat/test-methods.R | 7 +++++++ 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/.github/workflows/R-CMD-check-hard.yaml b/.github/workflows/R-CMD-check-hard.yaml index 54db1e064..977807d3c 100644 --- a/.github/workflows/R-CMD-check-hard.yaml +++ b/.github/workflows/R-CMD-check-hard.yaml @@ -48,6 +48,7 @@ jobs: - uses: r-lib/actions/setup-r-dependencies@v2 with: + install-quarto: false pak-version: devel dependencies: '"hard"' cache: false diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 110d148cb..f0848b43b 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -70,6 +70,7 @@ jobs: - uses: r-lib/actions/setup-r-dependencies@v2 with: + install-quarto: false extra-packages: | any::rcmdcheck needs: check diff --git a/NEWS.md b/NEWS.md index 140ee54ea..68ef13f8d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -58,6 +58,7 @@ * `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). * `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). +* `lint()` and friends emit a message if no lints are found (#2643, @IndrajeetPatil). ### New linters diff --git a/R/methods.R b/R/methods.R index 29a974d48..869c17ef1 100644 --- a/R/methods.R +++ b/R/methods.R @@ -101,10 +101,14 @@ print.lints <- function(x, ...) { if (isTRUE(settings$error_on_lint)) { quit("no", 31L, FALSE) # nocov } - } else if (use_rstudio_source_markers) { - # Empty lints: clear RStudio source markers - rstudio_source_markers(x) + } else { + # Empty lints + cli_inform(c(i = "No lints found.")) + if (use_rstudio_source_markers) { + rstudio_source_markers(x) # clear RStudio source markers + } } + invisible(x) } diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index 172b9692d..b337169f1 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -96,6 +96,13 @@ test_that("print.lint works", { expect_output(print(l), " 1:length(x)", fixed = TRUE) }) +test_that("print.lint works with empty lints", { + withr::local_options(list(lintr.rstudio_source_markers = FALSE)) + l <- lint(text = "1L") + + expect_message(print(l), "No lints found", fixed = TRUE) +}) + test_that("print.lint works for inline data, even in RStudio", { l <- lint("x = 1\n") From 8d96145aa11d4eabf9b66fa391b57f8cc467a740 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Tue, 6 Aug 2024 21:53:38 +0200 Subject: [PATCH 11/17] Update `seq_linter()` docs (#2644) --- R/seq_linter.R | 3 ++- man/seq_linter.Rd | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/R/seq_linter.R b/R/seq_linter.R index decc02c66..d31b3dd08 100644 --- a/R/seq_linter.R +++ b/R/seq_linter.R @@ -7,7 +7,8 @@ #' Additionally, it checks for `1:n()` (from `{dplyr}`) and `1:.N` (from `{data.table}`). #' #' These often cause bugs when the right-hand side is zero. -#' It is safer to use [base::seq_len()] or [base::seq_along()] instead. +#' Instead, it is safer to use [base::seq_len()] (to create a sequence of a specified *length*) or +#' [base::seq_along()] (to create a sequence *along* an object). #' #' @examples #' # will produce lints diff --git a/man/seq_linter.Rd b/man/seq_linter.Rd index 02c46af72..ece1e853c 100644 --- a/man/seq_linter.Rd +++ b/man/seq_linter.Rd @@ -15,7 +15,8 @@ conjunction with \code{seq()} (e.g., \code{seq(length(...))}, \code{seq(nrow(... Additionally, it checks for \code{1:n()} (from \code{{dplyr}}) and \code{1:.N} (from \code{{data.table}}). These often cause bugs when the right-hand side is zero. -It is safer to use \code{\link[base:seq]{base::seq_len()}} or \code{\link[base:seq]{base::seq_along()}} instead. +Instead, it is safer to use \code{\link[base:seq]{base::seq_len()}} (to create a sequence of a specified \emph{length}) or +\code{\link[base:seq]{base::seq_along()}} (to create a sequence \emph{along} an object). } \examples{ # will produce lints From 5990da6a347de8b87566b6d7516e307497ce0680 Mon Sep 17 00:00:00 2001 From: olivroy <52606734+olivroy@users.noreply.github.com> Date: Fri, 9 Aug 2024 13:02:37 -0400 Subject: [PATCH 12/17] Support clickable hyperlinks (#2646) * lintr now imports cli * Support clickable hyperlinks when they are available * Rename variables + restyle * Remove unneeded space * refactor in `build_line_ref()` * Support path with closing brackets * Update NEWS.md * Update NEWS.md Co-authored-by: AshesITR * Update methods.R --------- Co-authored-by: AshesITR --- NEWS.md | 1 + R/methods.R | 42 ++++++++++++++++++++++-------------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/NEWS.md b/NEWS.md index 68ef13f8d..d2508f02d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -88,6 +88,7 @@ ## Notes * All user-facing messages are now prepared using the `{cli}` package (#2418, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent. +* File locations in lints and error messages contain clickable hyperlinks to improve code navigation (#2645, #2588, @olivroy). * {lintr} now depends on R version 4.0.0. It already does so implicitly due to recursive upstream dependencies requiring this version; we've simply made that dependency explicit and up-front (#2569, @MichaelChirico). # lintr 3.1.2 diff --git a/R/methods.R b/R/methods.R index 869c17ef1..7a8fe3cc1 100644 --- a/R/methods.R +++ b/R/methods.R @@ -1,27 +1,16 @@ #' @export format.lint <- function(x, ..., width = getOption("lintr.format_width")) { - if (requireNamespace("cli", quietly = TRUE)) { - color <- switch(x$type, - warning = cli::col_magenta, - error = cli::col_red, - style = cli::col_blue, - cli::style_bold - ) - emph <- cli::style_bold - } else { - # nocov start - color <- identity - emph <- identity - # nocov end - } + color <- switch(x$type, + warning = cli::col_magenta, + error = cli::col_red, + style = cli::col_blue, + cli::style_bold + ) + emph <- cli::style_bold + line_ref <- build_line_ref(x) annotated_msg <- paste0( - emph( - x$filename, ":", - as.character(x$line_number), ":", - as.character(x$column_number), ": ", - sep = "" - ), + emph(line_ref, ": "), color(x$type, ": ", sep = ""), "[", x$linter, "] ", emph(x$message) @@ -40,6 +29,19 @@ format.lint <- function(x, ..., width = getOption("lintr.format_width")) { ) } +build_line_ref <- function(x) { + line_ref <- paste0( + x$filename, ":", + as.character(x$line_number), ":", + as.character(x$column_number) + ) + + if (!cli::ansi_has_hyperlink_support()) { + return(line_ref) + } + cli::format_inline("{.path {line_ref}}") +} + #' @export print.lint <- function(x, ...) { cat(format(x, ...)) From 2a7bdabfa3979f805c9a3c1dd6cda4ce30f418b2 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Tue, 3 Sep 2024 17:20:33 +0200 Subject: [PATCH 13/17] JOSS publication (#1885) * add boilerplate * say more about need and benefits of lintr * update PDF as well * fix merge issues [skip ci] * add a few examples * try width [skip ci] * discuss specific tags * only show examples that lint * add section on best practices * retain TeX file as well [skip ci] * address Jim's comment [skip ci] * Update paper/paper.Rmd Co-authored-by: Michael Chirico * mention tidyverse style guide up front * we only need md document * Update draft-pdf.yml * preserve YAML; add initial list of authors * initial acknowledgments * change example for best practices * also add example that doesn't lint * Update paper/paper.Rmd [skip ci] Co-authored-by: Michael Chirico * Update paper/paper.Rmd [skip ci] Co-authored-by: Michael Chirico * Update paper/paper.Rmd [skip ci] Co-authored-by: Michael Chirico * update ignore regex; update workflow * add bib entry for style guide * change a couple of examples * add common mistakes section * michael has an orcid somehow :) * Apply Michael's suggestions from code review Co-authored-by: Michael Chirico * reknit * add chunk labels * move customizability segment to later * Just create a new section to highlight extensibility * more on customization * add citation to wiki page * Update paper/paper.Rmd [skip ci] Co-authored-by: Michael Chirico * Update paper/paper.Rmd [skip ci] Co-authored-by: Michael Chirico * consistently use with-without lint pairing [skip ci] * use latest, blazingly fast upload artifact action * authors: Jim first, everyone else alphabetical * Update paper.md Co-authored-by: Jim Hester * update Rmd file for Jim's ORCID --------- Co-authored-by: Michael Chirico Co-authored-by: Michael Chirico Co-authored-by: Jim Hester --- .Rbuildignore | 1 + .github/workflows/draft-pdf.yml | 30 + paper/apa.csl | 1916 +++++++++++++++++++++++++++++++ paper/paper.Rmd | 246 ++++ paper/paper.bib | 41 + paper/paper.md | 342 ++++++ 6 files changed, 2576 insertions(+) create mode 100644 .github/workflows/draft-pdf.yml create mode 100644 paper/apa.csl create mode 100644 paper/paper.Rmd create mode 100644 paper/paper.bib create mode 100644 paper/paper.md diff --git a/.Rbuildignore b/.Rbuildignore index e609141bd..257a72963 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -29,3 +29,4 @@ ^vignettes/[^-]+.gif$ ^CRAN-SUBMISSION$ ^CODE_OF_CONDUCT\.md$ +^paper$ diff --git a/.github/workflows/draft-pdf.yml b/.github/workflows/draft-pdf.yml new file mode 100644 index 000000000..d72255cdc --- /dev/null +++ b/.github/workflows/draft-pdf.yml @@ -0,0 +1,30 @@ +# TODO: delete this file once the paper is published +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + paper: + runs-on: ubuntu-latest + name: Paper Draft + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Build draft PDF + uses: openjournals/openjournals-draft-action@master + with: + journal: joss + # This should be the path to the paper within your repo. + paper-path: paper/paper.md + + - name: Upload + uses: actions/upload-artifact@v4 + with: + name: paper + # This is the output path where Pandoc will write the compiled + # PDF. Note, this should be the same directory as the input + # paper.md + path: paper/paper.pdf diff --git a/paper/apa.csl b/paper/apa.csl new file mode 100644 index 000000000..081857d9d --- /dev/null +++ b/paper/apa.csl @@ -0,0 +1,1916 @@ + + diff --git a/paper/paper.Rmd b/paper/paper.Rmd new file mode 100644 index 000000000..4aa995404 --- /dev/null +++ b/paper/paper.Rmd @@ -0,0 +1,246 @@ +--- +title: "Static Code Analysis for R" +date: "`r Sys.Date()`" +tags: ["R", "linter", "tidyverse"] +authors: + - name: Jim Hester + affiliation: 1 + orcid: 0000-0002-2739-7082 + - name: Florent Angly + affiliation: ~ + orcid: ~ + - name: Michael Chirico + affiliation: 2 + orcid: 0000-0003-0787-087X + - name: Russ Hyde + affiliation: 5 + orcid: ~ + - name: Ren Kun + affiliation: ~ + orcid: ~ + - name: Indrajeet Patil + orcid: 0000-0003-1995-6531 + affiliation: 4 + - name: Alexander Rosenstock + affiliation: 3 + orcid: ~ +affiliations: + - index: 1 + name: Netflix + - index: 2 + name: Google + - index: 3 + name: Mathematisches Institut der Heinrich-Heine-Universität Düsseldorf + - index: 4 + name: Preisenergie GmbH, Munich, Germany + - index: 5 + name: Jumping Rivers +output: + md_document: + variant: "markdown" + preserve_yaml: true + standalone: true +bibliography: paper.bib +csl: apa.csl +link-citations: yes +--- + +```{r setup, warning=FALSE, message=FALSE, echo=FALSE} +knitr::opts_chunk$set( + collapse = TRUE, + out.width = "100%", + comment = "#>" +) + +library(lintr) + +withr::local_options(list( + lintr.format_width = 60L +)) +``` + +# Statement of Need + +R is an interpreted, dynamically-typed programming language [@base2023]. It is a popular choice for statistical analysis and visualization, and is used by a wide range of researchers and data scientists. The `{lintr}` package is an open-source R package that provides static code analysis [@enwiki:1218663830] to check for a variety of common problems related to readability, efficiency, consistency, style, etc. In particular, by default it enforces the tidyverse style guide [@Wickham2023]. It is designed to be easy to use and integrate into existing workflows, and can be used as part of an automated build or continuous integration process. `{lintr}` also integrates with a number of popular IDEs and text editors, such as RStudio and Visual Studio Code, making it convenient for users to run `{lintr}` checks on their code as they work. + +# Features + +As of this writing, `{lintr}` offers `r length(all_linters())` linters. + +```{r all_linters} +library(lintr) + +length(all_linters()) +``` + +Naturally, we can't discuss all of them here. To see details about all available linters, we encourage readers to see . + +We will showcase one linter for each kind of common problem found in R code. + +- **Best practices** + +`{lintr}` offers linters that can detect problematic antipatterns and suggest alternatives that follow best practices. + +For example, expressions like `ifelse(x, TRUE, FALSE)` and `ifelse(x, FALSE, TRUE)` are redundant; just `x` or `!x` suffice in R code where logical vectors are a core data structure. The `redundant_ifelse_linter()` linter detects such discouraged usages. + +```{r redundant_ifelse_linter_with_lint} +lint( + text = "ifelse(x >= 2.5, TRUE, FALSE)", + linters = redundant_ifelse_linter() +) +``` + +```{r redundant_ifelse_linter_without_lint} +lint( + text = "x >= 2.5", + linters = redundant_ifelse_linter() +) +``` + +- **Efficiency** + +Sometimes the users might not be aware of a more efficient way offered by R for carrying out a computation. `{lintr}` offers linters to improve code efficiency by avoiding common inefficient patterns. + +For example, the `any_is_na_linter()` linter detects usages of `any(is.na(x))` and suggests `anyNA(x)` as a more efficient alternative to detect presence of *any* missing values. + +```{r any_is_na_linter_with_lint} +lint( + text = "any(is.na(x), na.rm = TRUE)", + linters = any_is_na_linter() +) +``` + +`anyNA()` in R is more efficient than `any(is.na())` because it stops execution once a missing value is found, while `is.na()` evaluates the entire vector. + +```{r any_is_na_linter_without_lint} +lint( + text = "anyNA(x)", + linters = any_is_na_linter() +) +``` + +- **Readability** + +Coders spend significantly more time reading than writing code [@mcconnell2004code]. Thus, writing readable code makes the code more maintainable and reduces the possibility of introducing bugs stemming from a poor understanding of the code. + +`{lintr}` provides a number of linters that suggest more readable alternatives. For example, `comparison_negation_linter()` blocks usages like `!(x == y)` where a direct relational operator is appropriate. + +```{r comparison_negation_linter_with_lint} +lint( + text = "!x == 2", + linters = comparison_negation_linter() +) +``` + +Note also the complicated operator precedence. The more readable alternative here uses `!=`: + +```{r comparison_negation_linter_without_lint} +lint( + text = "x != 2", + linters = comparison_negation_linter() +) +``` + +- **Tidyverse style** + +`{lintr}` also provides linters to enforce the style used throughout the `{tidyverse}` [@Wickham2019] ecosystem of R packages. This style of coding has been outlined in the tidyverse style guide [@Wickham2023]. + +For example, the style guide recommends using snake_case for identifiers: + +```{r object_name_linter_with_lint} +lint( + text = "MyVar <- 1L", + linters = object_name_linter() +) +``` + +```{r object_name_linter_without_lint} +lint( + text = "my_var <- 1L", + linters = object_name_linter() +) +``` + +- **Common mistakes** + +One category of linters helps you detect some common mistakes statically and provide early feedback. + +For example, duplicate arguments in function calls can sometimes cause run-time errors: + +```{r duplicate_args_error_example, error=TRUE} +mean(x = 1:5, x = 2:3) +``` + +But `duplicate_argument_linter()` can check for this statically: + +```{r duplicate_argument_linter_with_lint} +lint( + text = "mean(x = 1:5, x = 2:3)", + linters = duplicate_argument_linter() +) +``` + +Even for cases where duplicate arguments are not an error, this linter explicitly discourages duplicate arguments. + +```{r duplicate_argument_linter_without_lint} +lint( + text = "list(x = TRUE, x = FALSE)", + linters = duplicate_argument_linter() +) +``` + +This is because objects with duplicated names objects can be hard to work with programmatically and should typically be avoided. + +```{r duplicate_arguments_example} +l <- list(x = TRUE, x = FALSE) +l["x"] +l[names(l) == "x"] +``` + +# Extensibility + +`{lintr}` is designed for extensibility by allowing users to easily create custom linting rules. +There are two main ways to customize it: + + - Use additional arguments in existing linters. For example, although tidyverse style guide prefers snake_case for identifiers, if a project's conventions require it, the relevant linter can be customized to support it: + + ```{r object_name_linter_with_custom_style} + lint( + text = "my.var <- 1L", + linters = object_name_linter(styles = "dotted.case") + ) + ``` + + - Create new linters (by leveraging functions like `lintr::make_linter_from_xpath()`) tailored to match project- or organization-specific coding standards. + +# Benefits of using `{lintr}` + +There are several benefits to using `{lintr}` to analyze and improve R code. One of the most obvious is that it can help users identify and fix problems in their code, which can save time and effort during the development process. By catching issues early on, `{lintr}` can help prevent bugs and other issues from creeping into code, which can save time and effort when it comes to debugging and testing. + +Another benefit of `{lintr}` is that it can help users write more readable and maintainable code. By enforcing a consistent style and highlighting potential issues, `{lintr}` can help users write code that is easier to understand and work with. This is especially important for larger projects or teams, where multiple contributors may be working on the same codebase and it is important to ensure that code is easy to follow and understand, particularly when frequently switching context among code primarily authored by different people. + +It can also be a useful tool for teaching and learning R. By providing feedback on code style and potential issues, it can help users learn good coding practices and improve their skills over time. This can be especially useful for beginners, who may not yet be familiar with all of the best practices for writing R code. + +Finally, `{lintr}` has had a large and active user community since its birth in 2014 which has contributed to its rapid development, maintenance, and adoption. At the time of writing, `{lintr}` is in a mature and stable state and therefore provides a reliable API that is unlikely to feature fundamental breaking changes. + +# Conclusion + +`{lintr}` is a valuable tool for R users to help improve the quality and reliability of their code. Its static code analysis capabilities, combined with its flexibility and ease of use, make it relevant and valuable for a wide range of applications. + +# Licensing and Availability + +`{lintr}` is licensed under the MIT License, with all source code openly developed and stored on GitHub (), along with a corresponding issue tracker for bug reporting and feature enhancements. + +# Conflicts of interest + +The authors declare no conflict of interest. + +# Funding + +This work was not financially supported by any of the affiliated institutions of the authors. + +# Acknowledgments + +`{lintr}` would not be possible without the immense work of the [R-core team](https://www.r-project.org/contributors.html) who maintain the R language and we are deeply indebted to them. We are also grateful to all contributors to `{lintr}`. + +# References diff --git a/paper/paper.bib b/paper/paper.bib new file mode 100644 index 000000000..f9578f0c9 --- /dev/null +++ b/paper/paper.bib @@ -0,0 +1,41 @@ +@Article{Wickham2019, + title = {Welcome to the {tidyverse}}, + author = {Hadley Wickham and Mara Averick and Jennifer Bryan and Winston Chang and Lucy D'Agostino McGowan and Romain François and Garrett Grolemund and Alex Hayes and Lionel Henry and Jim Hester and Max Kuhn and Thomas Lin Pedersen and Evan Miller and Stephan Milton Bache and Kirill Müller and Jeroen Ooms and David Robinson and Dana Paige Seidel and Vitalie Spinu and Kohske Takahashi and Davis Vaughan and Claus Wilke and Kara Woo and Hiroaki Yutani}, + year = {2019}, + journal = {Journal of Open Source Software}, + volume = {4}, + number = {43}, + pages = {1686}, + doi = {10.21105/joss.01686}, + } + +@Manual{Wickham2023, + title = {The Tidyverse Style Guide}, + author = {Hadley Wickham}, + year = {2023}, + url = {https://style.tidyverse.org/index.html}, + } + +@Manual{base2023, + title = {{R}: A Language and Environment for Statistical Computing}, + author = {{R Core Team}}, + organization = {R Foundation for Statistical Computing}, + address = {Vienna, Austria}, + year = {2023}, + url = {https://www.R-project.org/}, + } + +@book{mcconnell2004code, + title={Code Complete}, + author={McConnell, Steve}, + year={2004}, + publisher={Pearson Education} + } + + @misc{ enwiki:1218663830, + author = "{Wikipedia contributors}", + title = "Static program analysis --- {Wikipedia}{,} The Free Encyclopedia", + year = "2024", + url = "https://en.wikipedia.org/w/index.php?title=Static_program_analysis&oldid=1218663830", + note = "[Online; accessed 7-May-2024]" + } diff --git a/paper/paper.md b/paper/paper.md new file mode 100644 index 000000000..079b54841 --- /dev/null +++ b/paper/paper.md @@ -0,0 +1,342 @@ +--- +title: "Static Code Analysis for R" +date: "2024-06-21" +tags: ["R", "linter", "tidyverse"] +authors: + - name: Jim Hester + affiliation: 1 + orcid: 0000-0002-2739-7082 + - name: Florent Angly + affiliation: ~ + orcid: ~ + - name: Michael Chirico + affiliation: 2 + orcid: 0000-0003-0787-087X + - name: Russ Hyde + affiliation: 5 + orcid: ~ + - name: Ren Kun + affiliation: ~ + orcid: ~ + - name: Indrajeet Patil + orcid: 0000-0003-1995-6531 + affiliation: 4 + - name: Alexander Rosenstock + affiliation: 3 + orcid: ~ +affiliations: + - index: 1 + name: Netflix + - index: 2 + name: Google + - index: 3 + name: Mathematisches Institut der Heinrich-Heine-Universität Düsseldorf + - index: 4 + name: Preisenergie GmbH, Munich, Germany + - index: 5 + name: Jumping Rivers +output: + md_document: + variant: "markdown" + preserve_yaml: true + standalone: true +bibliography: paper.bib +csl: apa.csl +link-citations: yes +--- + +# Statement of Need + +R is an interpreted, dynamically-typed programming language [@base2023]. +It is a popular choice for statistical analysis and visualization, and +is used by a wide range of researchers and data scientists. The +`{lintr}` package is an open-source R package that provides static code +analysis [@enwiki:1218663830] to check for a variety of common problems +related to readability, efficiency, consistency, style, etc. In +particular, by default it enforces the tidyverse style guide +[@Wickham2023]. It is designed to be easy to use and integrate into +existing workflows, and can be used as part of an automated build or +continuous integration process. `{lintr}` also integrates with a number +of popular IDEs and text editors, such as RStudio and Visual Studio +Code, making it convenient for users to run `{lintr}` checks on their +code as they work. + +# Features + +As of this writing, `{lintr}` offers 113 linters. + +``` r +library(lintr) + +length(all_linters()) +#> [1] 113 +``` + +Naturally, we can't discuss all of them here. To see details about all +available linters, we encourage readers to see +. + +We will showcase one linter for each kind of common problem found in R +code. + +- **Best practices** + +`{lintr}` offers linters that can detect problematic antipatterns and +suggest alternatives that follow best practices. + +For example, expressions like `ifelse(x, TRUE, FALSE)` and +`ifelse(x, FALSE, TRUE)` are redundant; just `x` or `!x` suffice in R +code where logical vectors are a core data structure. The +`redundant_ifelse_linter()` linter detects such discouraged usages. + +``` r +lint( + text = "ifelse(x >= 2.5, TRUE, FALSE)", + linters = redundant_ifelse_linter() +) +#> :1:1: warning: [redundant_ifelse_linter] Just use the +#> logical condition (or its negation) directly instead of +#> calling ifelse(x, TRUE, FALSE) +#> ifelse(x >= 2.5, TRUE, FALSE) +#> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +``` r +lint( + text = "x >= 2.5", + linters = redundant_ifelse_linter() +) +``` + +- **Efficiency** + +Sometimes the users might not be aware of a more efficient way offered +by R for carrying out a computation. `{lintr}` offers linters to improve +code efficiency by avoiding common inefficient patterns. + +For example, the `any_is_na_linter()` linter detects usages of +`any(is.na(x))` and suggests `anyNA(x)` as a more efficient alternative +to detect presence of *any* missing values. + +``` r +lint( + text = "any(is.na(x), na.rm = TRUE)", + linters = any_is_na_linter() +) +#> :1:1: warning: [any_is_na_linter] anyNA(x) is better +#> than any(is.na(x)). +#> any(is.na(x), na.rm = TRUE) +#> ^~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +`anyNA()` in R is more efficient than `any(is.na())` because it stops +execution once a missing value is found, while `is.na()` evaluates the +entire vector. + +``` r +lint( + text = "anyNA(x)", + linters = any_is_na_linter() +) +``` + +- **Readability** + +Coders spend significantly more time reading than writing code +[@mcconnell2004code]. Thus, writing readable code makes the code more +maintainable and reduces the possibility of introducing bugs stemming +from a poor understanding of the code. + +`{lintr}` provides a number of linters that suggest more readable +alternatives. For example, `comparison_negation_linter()` blocks usages +like `!(x == y)` where a direct relational operator is appropriate. + +``` r +lint( + text = "!x == 2", + linters = comparison_negation_linter() +) +#> :1:1: warning: [comparison_negation_linter] Use x != +#> y, not !(x == y). +#> !x == 2 +#> ^~~~~~~ +``` + +Note also the complicated operator precedence. The more readable +alternative here uses `!=`: + +``` r +lint( + text = "x != 2", + linters = comparison_negation_linter() +) +``` + +- **Tidyverse style** + +`{lintr}` also provides linters to enforce the style used throughout the +`{tidyverse}` [@Wickham2019] ecosystem of R packages. This style of +coding has been outlined in the tidyverse style guide [@Wickham2023]. + +For example, the style guide recommends using snake_case for +identifiers: + +``` r +lint( + text = "MyVar <- 1L", + linters = object_name_linter() +) +#> :1:1: style: [object_name_linter] Variable and +#> function name style should match snake_case or symbols. +#> MyVar <- 1L +#> ^~~~~ +``` + +``` r +lint( + text = "my_var <- 1L", + linters = object_name_linter() +) +``` + +- **Common mistakes** + +One category of linters helps you detect some common mistakes statically +and provide early feedback. + +For example, duplicate arguments in function calls can sometimes cause +run-time errors: + +``` r +mean(x = 1:5, x = 2:3) +#> Error in mean(x = 1:5, x = 2:3): formal argument "x" matched by multiple actual arguments +``` + +But `duplicate_argument_linter()` can check for this statically: + +``` r +lint( + text = "mean(x = 1:5, x = 2:3)", + linters = duplicate_argument_linter() +) +#> :1:15: warning: [duplicate_argument_linter] Avoid +#> duplicate arguments in function calls. +#> mean(x = 1:5, x = 2:3) +#> ^ +``` + +Even for cases where duplicate arguments are not an error, this linter +explicitly discourages duplicate arguments. + +``` r +lint( + text = "list(x = TRUE, x = FALSE)", + linters = duplicate_argument_linter() +) +#> :1:16: warning: [duplicate_argument_linter] Avoid +#> duplicate arguments in function calls. +#> list(x = TRUE, x = FALSE) +#> ^ +``` + +This is because objects with duplicated names objects can be hard to +work with programmatically and should typically be avoided. + +``` r +l <- list(x = TRUE, x = FALSE) +l["x"] +#> $x +#> [1] TRUE +``` + +``` r +l[names(l) == "x"] +#> $x +#> [1] TRUE +#> +#> $x +#> [1] FALSE +``` + +# Extensibility + +`{lintr}` is designed for extensibility by allowing users to easily +create custom linting rules. There are two main ways to customize it: + +- Use additional arguments in existing linters. For example, although + tidyverse style guide prefers snake_case for identifiers, if a + project's conventions require it, the relevant linter can be + customized to support it: + +``` r +lint( + text = "my.var <- 1L", + linters = object_name_linter(styles = "dotted.case") +) +``` + +- Create new linters (by leveraging functions like + `lintr::make_linter_from_xpath()`) tailored to match project- or + organization-specific coding standards. + +# Benefits of using `{lintr}` + +There are several benefits to using `{lintr}` to analyze and improve R +code. One of the most obvious is that it can help users identify and fix +problems in their code, which can save time and effort during the +development process. By catching issues early on, `{lintr}` can help +prevent bugs and other issues from creeping into code, which can save +time and effort when it comes to debugging and testing. + +Another benefit of `{lintr}` is that it can help users write more +readable and maintainable code. By enforcing a consistent style and +highlighting potential issues, `{lintr}` can help users write code that +is easier to understand and work with. This is especially important for +larger projects or teams, where multiple contributors may be working on +the same codebase and it is important to ensure that code is easy to +follow and understand, particularly when frequently switching context +among code primarily authored by different people. + +It can also be a useful tool for teaching and learning R. By providing +feedback on code style and potential issues, it can help users learn +good coding practices and improve their skills over time. This can be +especially useful for beginners, who may not yet be familiar with all of +the best practices for writing R code. + +Finally, `{lintr}` has had a large and active user community since its +birth in 2014 which has contributed to its rapid development, +maintenance, and adoption. At the time of writing, `{lintr}` is in a +mature and stable state and therefore provides a reliable API that is +unlikely to feature fundamental breaking changes. + +# Conclusion + +`{lintr}` is a valuable tool for R users to help improve the quality and +reliability of their code. Its static code analysis capabilities, +combined with its flexibility and ease of use, make it relevant and +valuable for a wide range of applications. + +# Licensing and Availability + +`{lintr}` is licensed under the MIT License, with all source code openly +developed and stored on GitHub (), along +with a corresponding issue tracker for bug reporting and feature +enhancements. + +# Conflicts of interest + +The authors declare no conflict of interest. + +# Funding + +This work was not financially supported by any of the affiliated +institutions of the authors. + +# Acknowledgments + +`{lintr}` would not be possible without the immense work of the [R-core +team](https://www.r-project.org/contributors.html) who maintain the R +language and we are deeply indebted to them. We are also grateful to all +contributors to `{lintr}`. + +# References {#references .unnumbered} From 64c8baab28f97cd676118c69f2db6d4c5262a54c Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Thu, 5 Sep 2024 16:30:19 +0200 Subject: [PATCH 14/17] Add missing affiliations for authors (#2651) --- paper/paper.Rmd | 10 +++++++--- paper/paper.md | 20 +++++++++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/paper/paper.Rmd b/paper/paper.Rmd index 4aa995404..05e3e3f61 100644 --- a/paper/paper.Rmd +++ b/paper/paper.Rmd @@ -7,8 +7,8 @@ authors: affiliation: 1 orcid: 0000-0002-2739-7082 - name: Florent Angly - affiliation: ~ - orcid: ~ + affiliation: 6 + orcid: 0000-0002-8999-0738 - name: Michael Chirico affiliation: 2 orcid: 0000-0003-0787-087X @@ -16,7 +16,7 @@ authors: affiliation: 5 orcid: ~ - name: Ren Kun - affiliation: ~ + affiliation: 7 orcid: ~ - name: Indrajeet Patil orcid: 0000-0003-1995-6531 @@ -35,6 +35,10 @@ affiliations: name: Preisenergie GmbH, Munich, Germany - index: 5 name: Jumping Rivers + - index: 6 + name: The University of Queensland + - index: 7 + name: Unknown output: md_document: variant: "markdown" diff --git a/paper/paper.md b/paper/paper.md index 079b54841..ebdc1677d 100644 --- a/paper/paper.md +++ b/paper/paper.md @@ -1,14 +1,14 @@ --- title: "Static Code Analysis for R" -date: "2024-06-21" +date: "2024-09-05" tags: ["R", "linter", "tidyverse"] authors: - name: Jim Hester affiliation: 1 orcid: 0000-0002-2739-7082 - name: Florent Angly - affiliation: ~ - orcid: ~ + affiliation: 6 + orcid: 0000-0002-8999-0738 - name: Michael Chirico affiliation: 2 orcid: 0000-0003-0787-087X @@ -16,7 +16,7 @@ authors: affiliation: 5 orcid: ~ - name: Ren Kun - affiliation: ~ + affiliation: 7 orcid: ~ - name: Indrajeet Patil orcid: 0000-0003-1995-6531 @@ -35,6 +35,10 @@ affiliations: name: Preisenergie GmbH, Munich, Germany - index: 5 name: Jumping Rivers + - index: 6 + name: The University of Queensland + - index: 7 + name: Unknown output: md_document: variant: "markdown" @@ -106,6 +110,7 @@ lint( text = "x >= 2.5", linters = redundant_ifelse_linter() ) +#> ℹ No lints found. ``` - **Efficiency** @@ -138,6 +143,7 @@ lint( text = "anyNA(x)", linters = any_is_na_linter() ) +#> ℹ No lints found. ``` - **Readability** @@ -170,6 +176,7 @@ lint( text = "x != 2", linters = comparison_negation_linter() ) +#> ℹ No lints found. ``` - **Tidyverse style** @@ -197,6 +204,7 @@ lint( text = "my_var <- 1L", linters = object_name_linter() ) +#> ℹ No lints found. ``` - **Common mistakes** @@ -247,9 +255,6 @@ l <- list(x = TRUE, x = FALSE) l["x"] #> $x #> [1] TRUE -``` - -``` r l[names(l) == "x"] #> $x #> [1] TRUE @@ -273,6 +278,7 @@ lint( text = "my.var <- 1L", linters = object_name_linter(styles = "dotted.case") ) +#> ℹ No lints found. ``` - Create new linters (by leveraging functions like From 7fa4876a8757fe7498d328cc2bede11153e745b5 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Thu, 5 Sep 2024 23:55:10 +0200 Subject: [PATCH 15/17] Remove a few linters from efficiency tag (#2655) * Remove a few linters from efficiency tag closes #2653 * Update NEWS.md * Update NEWS.md Co-authored-by: Michael Chirico --------- Co-authored-by: Michael Chirico --- NEWS.md | 2 ++ inst/lintr/linters.csv | 6 +++--- man/efficiency_linters.Rd | 3 --- man/linters.Rd | 8 ++++---- man/list_comparison_linter.Rd | 2 +- man/undesirable_function_linter.Rd | 2 +- man/undesirable_operator_linter.Rd | 2 +- 7 files changed, 12 insertions(+), 13 deletions(-) diff --git a/NEWS.md b/NEWS.md index d2508f02d..abc0f5c2b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -18,6 +18,8 @@ * `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) * `lint()` and friends now normalize paths to forward slashes on Windows (@olivroy, #2613). +* `undesirable_function_linter()`, `undesirable_operator_linter()`, and `list_comparison_linter()` were removed from the tag `efficiency` (@IndrajeetPatil, #2655). If you use `linters_with_tags("efficiency")` to include these linters, you'll need to adjust your config to keep linting your code against them. We did not find any such users on GitHub. + ## Bug fixes diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index 95af98b61..1f070f45f 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -52,7 +52,7 @@ length_test_linter,common_mistakes efficiency lengths_linter,efficiency readability best_practices library_call_linter,style best_practices readability configurable line_length_linter,style readability default configurable -list_comparison_linter,best_practices common_mistakes efficiency +list_comparison_linter,best_practices common_mistakes literal_coercion_linter,best_practices consistency efficiency matrix_apply_linter,readability efficiency missing_argument_linter,correctness common_mistakes configurable @@ -108,8 +108,8 @@ terminal_close_linter,best_practices robustness todo_comment_linter,style configurable trailing_blank_lines_linter,style default trailing_whitespace_linter,style default configurable -undesirable_function_linter,style efficiency configurable robustness best_practices -undesirable_operator_linter,style efficiency configurable robustness best_practices +undesirable_function_linter,style configurable robustness best_practices +undesirable_operator_linter,style configurable robustness best_practices unnecessary_concatenation_linter,style readability efficiency configurable unnecessary_lambda_linter,best_practices efficiency readability configurable unnecessary_nested_if_linter,readability best_practices deprecated diff --git a/man/efficiency_linters.Rd b/man/efficiency_linters.Rd index 37cb8b0d7..458375e27 100644 --- a/man/efficiency_linters.Rd +++ b/man/efficiency_linters.Rd @@ -22,7 +22,6 @@ The following linters are tagged with 'efficiency': \item{\code{\link{inner_combine_linter}}} \item{\code{\link{length_test_linter}}} \item{\code{\link{lengths_linter}}} -\item{\code{\link{list_comparison_linter}}} \item{\code{\link{literal_coercion_linter}}} \item{\code{\link{matrix_apply_linter}}} \item{\code{\link{nested_ifelse_linter}}} @@ -38,8 +37,6 @@ The following linters are tagged with 'efficiency': \item{\code{\link{seq_linter}}} \item{\code{\link{sort_linter}}} \item{\code{\link{string_boundary_linter}}} -\item{\code{\link{undesirable_function_linter}}} -\item{\code{\link{undesirable_operator_linter}}} \item{\code{\link{unnecessary_concatenation_linter}}} \item{\code{\link{unnecessary_lambda_linter}}} \item{\code{\link{vector_logic_linter}}} diff --git a/man/linters.Rd b/man/linters.Rd index 394bd6126..50f8baf7d 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -24,7 +24,7 @@ The following tags exist: \item{\link[=correctness_linters]{correctness} (7 linters)} \item{\link[=default_linters]{default} (25 linters)} \item{\link[=deprecated_linters]{deprecated} (6 linters)} -\item{\link[=efficiency_linters]{efficiency} (32 linters)} +\item{\link[=efficiency_linters]{efficiency} (29 linters)} \item{\link[=executing_linters]{executing} (6 linters)} \item{\link[=package_development_linters]{package_development} (14 linters)} \item{\link[=pkg_testthat_linters]{pkg_testthat} (12 linters)} @@ -88,7 +88,7 @@ The following linters exist: \item{\code{\link{lengths_linter}} (tags: best_practices, efficiency, readability)} \item{\code{\link{library_call_linter}} (tags: best_practices, configurable, readability, style)} \item{\code{\link{line_length_linter}} (tags: configurable, default, readability, style)} -\item{\code{\link{list_comparison_linter}} (tags: best_practices, common_mistakes, efficiency)} +\item{\code{\link{list_comparison_linter}} (tags: best_practices, common_mistakes)} \item{\code{\link{literal_coercion_linter}} (tags: best_practices, consistency, efficiency)} \item{\code{\link{matrix_apply_linter}} (tags: efficiency, readability)} \item{\code{\link{missing_argument_linter}} (tags: common_mistakes, configurable, correctness)} @@ -139,8 +139,8 @@ The following linters exist: \item{\code{\link{todo_comment_linter}} (tags: configurable, style)} \item{\code{\link{trailing_blank_lines_linter}} (tags: default, style)} \item{\code{\link{trailing_whitespace_linter}} (tags: configurable, default, style)} -\item{\code{\link{undesirable_function_linter}} (tags: best_practices, configurable, efficiency, robustness, style)} -\item{\code{\link{undesirable_operator_linter}} (tags: best_practices, configurable, efficiency, robustness, style)} +\item{\code{\link{undesirable_function_linter}} (tags: best_practices, configurable, robustness, style)} +\item{\code{\link{undesirable_operator_linter}} (tags: best_practices, configurable, robustness, style)} \item{\code{\link{unnecessary_concatenation_linter}} (tags: configurable, efficiency, readability, style)} \item{\code{\link{unnecessary_lambda_linter}} (tags: best_practices, configurable, efficiency, readability)} \item{\code{\link{unnecessary_nesting_linter}} (tags: best_practices, configurable, consistency, readability)} diff --git a/man/list_comparison_linter.Rd b/man/list_comparison_linter.Rd index 9a939a686..736e78256 100644 --- a/man/list_comparison_linter.Rd +++ b/man/list_comparison_linter.Rd @@ -29,5 +29,5 @@ lint( \link{linters} for a complete list of linters available in lintr. } \section{Tags}{ -\link[=best_practices_linters]{best_practices}, \link[=common_mistakes_linters]{common_mistakes}, \link[=efficiency_linters]{efficiency} +\link[=best_practices_linters]{best_practices}, \link[=common_mistakes_linters]{common_mistakes} } diff --git a/man/undesirable_function_linter.Rd b/man/undesirable_function_linter.Rd index 1a73f29ce..bfcbd947d 100644 --- a/man/undesirable_function_linter.Rd +++ b/man/undesirable_function_linter.Rd @@ -68,5 +68,5 @@ lint( \link{linters} for a complete list of linters available in lintr. } \section{Tags}{ -\link[=best_practices_linters]{best_practices}, \link[=configurable_linters]{configurable}, \link[=efficiency_linters]{efficiency}, \link[=robustness_linters]{robustness}, \link[=style_linters]{style} +\link[=best_practices_linters]{best_practices}, \link[=configurable_linters]{configurable}, \link[=robustness_linters]{robustness}, \link[=style_linters]{style} } diff --git a/man/undesirable_operator_linter.Rd b/man/undesirable_operator_linter.Rd index 18bc5722b..2f1188116 100644 --- a/man/undesirable_operator_linter.Rd +++ b/man/undesirable_operator_linter.Rd @@ -52,5 +52,5 @@ lint( \link{linters} for a complete list of linters available in lintr. } \section{Tags}{ -\link[=best_practices_linters]{best_practices}, \link[=configurable_linters]{configurable}, \link[=efficiency_linters]{efficiency}, \link[=robustness_linters]{robustness}, \link[=style_linters]{style} +\link[=best_practices_linters]{best_practices}, \link[=configurable_linters]{configurable}, \link[=robustness_linters]{robustness}, \link[=style_linters]{style} } From 427fab3f2a82b824f3cdbfaf936228ed544f70a4 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 5 Sep 2024 22:06:26 -0700 Subject: [PATCH 16/17] Find some = or -> assignments when discovering script globals (#2656) Co-authored-by: AshesITR --- NEWS.md | 1 + R/object_usage_linter.R | 4 +- tests/testthat/test-object_usage_linter.R | 108 ++++++++++------------ 3 files changed, 53 insertions(+), 60 deletions(-) diff --git a/NEWS.md b/NEWS.md index abc0f5c2b..d017c606a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -86,6 +86,7 @@ ### Lint accuracy fixes: removing false positives * `object_name_linter()` and `object_length_linter()` ignore {rlang} name injection like `x |> mutate("{new_name}" := foo(col))` (#1926, @MichaelChirico). No checking is applied in such cases. {data.table} in-place assignments like `DT[, "sPoNGeBob" := "friend"]` are still eligible for lints. +* `object_usage_linter()` finds global variables assigned with `=` or `->`, which avoids some issues around "undefined global variables" in scripts (#2654, @MichaelChirico). ## Notes diff --git a/R/object_usage_linter.R b/R/object_usage_linter.R index 6b9466eff..1b7abce0b 100644 --- a/R/object_usage_linter.R +++ b/R/object_usage_linter.R @@ -148,8 +148,10 @@ get_assignment_symbols <- function(xml) { get_r_string(xml_find_all( xml, " - expr[LEFT_ASSIGN]/expr[1]/SYMBOL[1] | + expr[LEFT_ASSIGN or EQ_ASSIGN]/expr[1]/SYMBOL[1] | + expr[RIGHT_ASSIGN]/expr[2]/SYMBOL[1] | equal_assign/expr[1]/SYMBOL[1] | + expr_or_assign_or_help/expr[1]/SYMBOL[1] | expr[expr[1][SYMBOL_FUNCTION_CALL/text()='assign']]/expr[2]/* | expr[expr[1][SYMBOL_FUNCTION_CALL/text()='setMethod']]/expr[2]/* " diff --git a/tests/testthat/test-object_usage_linter.R b/tests/testthat/test-object_usage_linter.R index 299bd6e0e..355ea6328 100644 --- a/tests/testthat/test-object_usage_linter.R +++ b/tests/testthat/test-object_usage_linter.R @@ -748,6 +748,9 @@ test_that("symbols in formulas aren't treated as 'undefined global'", { }) test_that("NSE-ish symbols after $/@ are ignored as sources for lints", { + linter <- object_usage_linter() + lint_msg <- "no visible binding for global variable 'column'" + expect_lint( trim_some(" foo <- function(x) { @@ -757,12 +760,8 @@ test_that("NSE-ish symbols after $/@ are ignored as sources for lints", { ) } "), - list( - message = "no visible binding for global variable 'column'", - line_number = 4L, - column_number = 22L - ), - object_usage_linter() + list(lint_msg, line_number = 4L, column_number = 22L), + linter ) expect_lint( @@ -774,12 +773,8 @@ test_that("NSE-ish symbols after $/@ are ignored as sources for lints", { ) } "), - list( - message = "no visible binding for global variable 'column'", - line_number = 4L, - column_number = 22L - ), - object_usage_linter() + list(lint_msg, line_number = 4L, column_number = 22L), + linter ) }) @@ -798,53 +793,34 @@ test_that("functional lambda definitions are also caught", { }) test_that("messages without location info are repaired", { + linter <- object_usage_linter() + global_function_msg <- rex::rex("no visible global function definition for", anything) + global_variable_msg <- rex::rex("no visible binding for global variable", anything) + local_variable_msg <- rex::rex("local variable", anything, "assigned but may not be used") + # regression test for #1986 expect_lint( - trim_some(" - foo <- function() no_fun() - "), - list( - message = rex::rex("no visible global function definition for", anything), - line_number = 1L, - column_number = 19L - ), - object_usage_linter() + "foo <- function() no_fun()", + list(global_function_msg, line_number = 1L, column_number = 19L), + linter ) expect_lint( - trim_some(" - foo <- function(a = no_fun()) a - "), - list( - message = rex::rex("no visible global function definition for", anything), - line_number = 1L, - column_number = 21L - ), - object_usage_linter() + "foo <- function(a = no_fun()) a", + list(global_function_msg, line_number = 1L, column_number = 21L), + linter ) expect_lint( - trim_some(" - foo <- function() no_global - "), - list( - message = rex::rex("no visible binding for global variable", anything), - line_number = 1L, - column_number = 19L - ), - object_usage_linter() + "foo <- function() no_global", + list(global_variable_msg, line_number = 1L, column_number = 19L), + linter ) expect_lint( - trim_some(" - foo <- function() unused_local <- 42L - "), - list( - message = rex::rex("local variable", anything, "assigned but may not be used"), - line_number = 1L, - column_number = 19L - ), - object_usage_linter() + "foo <- function() unused_local <- 42L", + list(local_variable_msg, line_number = 1L, column_number = 19L), + linter ) # More complex case with two lints and missing location info @@ -854,17 +830,31 @@ test_that("messages without location info are repaired", { bar() "), list( - list( - message = rex::rex("local variable", anything, "assigned but may not be used"), - line_number = 1L, - column_number = 19L - ), - list( - message = rex::rex("no visible global function definition for", anything), - line_number = 2L, - column_number = 3L - ) + list(local_variable_msg, line_number = 1L, column_number = 19L), + list(global_function_msg, line_number = 2L, column_number = 3L) ), - object_usage_linter() + linter + ) +}) + +test_that("globals in scripts are found regardless of assignment operator", { + linter <- object_usage_linter() + + expect_lint( + trim_some(" + library(dplyr) + + global_const_eq = 5 + global_const_la <- 6 + 7 -> global_const_ra + + examplefunction <- function(df) { + df %>% + select(dist) %>% + mutate(power = global_const_eq + global_const_ra + global_const_la) + } + "), + NULL, + linter ) }) From 927157a5a9b6665c94d886a24ef3177582d6b5c6 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sat, 7 Sep 2024 07:18:08 +0200 Subject: [PATCH 17/17] Include `{data.table}` example in `redundant_equals_linter()` docs (#2659) --- R/redundant_equals_linter.R | 11 +++++++++++ man/redundant_equals_linter.Rd | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/R/redundant_equals_linter.R b/R/redundant_equals_linter.R index 48d524c5b..d986dc184 100644 --- a/R/redundant_equals_linter.R +++ b/R/redundant_equals_linter.R @@ -19,6 +19,11 @@ #' linters = redundant_equals_linter() #' ) #' +#' lint( +#' text = "dt[is_tall == FALSE, y]", +#' linters = redundant_equals_linter() +#' ) +#' #' # okay #' lint( #' text = "if (any(x)) 1", @@ -30,6 +35,12 @@ #' linters = redundant_equals_linter() #' ) #' +#' # in `{data.table}` semantics, `dt[x]` is a join, `dt[(x)]` is a subset +#' lint( +#' text = "dt[(!is_tall), y]", +#' linters = redundant_equals_linter() +#' ) +#' #' @evalRd rd_tags("redundant_equals_linter") #' @seealso #' - [linters] for a complete list of linters available in lintr. diff --git a/man/redundant_equals_linter.Rd b/man/redundant_equals_linter.Rd index e901db9c8..3045cbc7b 100644 --- a/man/redundant_equals_linter.Rd +++ b/man/redundant_equals_linter.Rd @@ -26,6 +26,11 @@ lint( linters = redundant_equals_linter() ) +lint( + text = "dt[is_tall == FALSE, y]", + linters = redundant_equals_linter() +) + # okay lint( text = "if (any(x)) 1", @@ -37,6 +42,12 @@ lint( linters = redundant_equals_linter() ) +# in `{data.table}` semantics, `dt[x]` is a join, `dt[(x)]` is a subset +lint( + text = "dt[(!is_tall), y]", + linters = redundant_equals_linter() +) + } \seealso{ \itemize{