Merge branch 'main' into check-random-order

.Rbuildignore

@@ -29,3 +29,4 @@
 ^vignettes/[^-]+.gif$
 ^CRAN-SUBMISSION$
 ^CODE_OF_CONDUCT\.md$
+^paper$

.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

.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

.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

.github/workflows/pkgdown.yaml

@@ -39,7 +39,7 @@ jobs:
 
  - name: Deploy to GitHub pages 🚀
  if: github.event_name != 'pull_request'
- uses: JamesIves/[email protected].1
+ uses: JamesIves/[email protected].3
  with:
  clean: false
  branch: gh-pages

.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}

.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,

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'

NEWS.md

@@ -17,6 +17,9 @@
 * 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).
+* `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
 
@@ -57,6 +60,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
 
@@ -82,10 +86,12 @@
 ### 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
 
 * 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

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) {

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(

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) {

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, ...))
@@ -92,7 +94,7 @@ print.lints <- function(x, ...) {
  inline_data <- x[[1L]][["filename"]] == "<text>"
  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, ...)
@@ -101,10 +103,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)
 }
 

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]/*
  "

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)

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.

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

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(),

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

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}}.")
  }

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)