Merge branch 'main' into check-random-order

.lintr

@@ -1,45 +1,30 @@
-linters: linters_with_defaults(
-    any_duplicated_linter(),
-    any_is_na_linter(),
-    backport_linter("oldrel-4", except = c("R_user_dir", "str2lang", "str2expression", "deparse1", "...names")),
-    consecutive_assertion_linter(),
-    expect_comparison_linter(),
-    expect_identical_linter(),
-    expect_length_linter(),
-    expect_named_linter(),
-    expect_not_linter(),
-    expect_null_linter(),
-    expect_s3_class_linter(),
-    expect_s4_class_linter(),
-    expect_true_false_linter(),
-    expect_type_linter(),
-    fixed_regex_linter(),
-    for_loop_index_linter(),
-    if_not_else_linter(),
-    implicit_assignment_linter(),
-    implicit_integer_linter(),
-    keyword_quote_linter(),
-    lengths_linter(),
-    line_length_linter(120),
-    missing_argument_linter(),
-    nested_ifelse_linter(),
-    numeric_leading_zero_linter(),
-    outer_negation_linter(),
-    paste_linter(),
-    redundant_equals_linter(),
-    redundant_ifelse_linter(),
-    sort_linter(),
-    sprintf_linter(),
-    strings_as_factors_linter(),
-    undesirable_function_linter(c(Sys.setenv = NA_character_, mapply = NA_character_, structure = NA_character_)),
-    unnecessary_nested_if_linter(),
-    unnecessary_lambda_linter(),
+linters: all_linters(
+    backport_linter("3.6.0", except = c("R_user_dir", "deparse1", "...names")),
+    line_length_linter(120L),
+    object_overwrite_linter(allow_names = c("line", "lines", "pipe", "symbols")),
+    undesirable_function_linter(modify_defaults(
+      defaults = default_undesirable_functions,
+      library = NULL,
+      options = NULL
+    )),
+    undesirable_operator_linter(modify_defaults(
+      defaults = default_undesirable_operators,
+      `:::` = NULL,
+      `<<-` = NULL
+    )),
     unnecessary_concatenation_linter(allow_single_expression = FALSE),
-    yoda_test_linter()
+    absolute_path_linter = NULL,
+    extraction_operator_linter = NULL,
+    library_call_linter = NULL,
+    nonportable_path_linter = NULL,
+    todo_comment_linter = NULL,
+    # TODO(#2327): Enable this.
+    unreachable_code_linter = NULL
   )
 exclusions: list(
     "inst/doc/creating_linters.R" = 1,
     "inst/example/bad.R",
+    "tests/testthat.R" = list(unused_import_linter = Inf),
     "tests/testthat/default_linter_testcode.R",
     "tests/testthat/dummy_packages",
     "tests/testthat/dummy_projects",

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: lintr
 Title: A 'Linter' for R Code
-Version: 3.1.0.9000
+Version: 3.1.1.9000
 Authors@R: c(
     person("Jim", "Hester", , "[email protected]", role = c("aut", "cre")),
     person("Florent", "Angly", role = "aut",
@@ -21,7 +21,7 @@ License: MIT + file LICENSE
 URL: https://github.com/r-lib/lintr, https://lintr.r-lib.org
 BugReports: https://github.com/r-lib/lintr/issues
 Depends:
-    R (>= 3.5)
+    R (>= 3.6)
 Imports:
     backports (>= 1.1.7),
     codetools,
@@ -36,7 +36,7 @@ Imports:
     xmlparsedata (>= 1.0.5)
 Suggests:
     bookdown,
-    crayon,
+    cli,
     httr (>= 1.2.1),
     jsonlite,
     mockery,
@@ -77,9 +77,12 @@ Collate:
     'commas_linter.R'
     'comment_linters.R'
     'comments.R'
+    'comparison_negation_linter.R'
+    'condition_call_linter.R'
     'condition_message_linter.R'
     'conjunct_test_linter.R'
     'consecutive_assertion_linter.R'
+    'consecutive_mutate_linter.R'
     'cyclocomp_linter.R'
     'declared_functions.R'
     'deprecated.R'
@@ -108,6 +111,7 @@ Collate:
     'get_source_expressions.R'
     'ids_with_token.R'
     'if_not_else_linter.R'
+    'if_switch_linter.R'
     'ifelse_censor_linter.R'
     'implicit_assignment_linter.R'
     'implicit_integer_linter.R'
@@ -127,6 +131,7 @@ Collate:
     'linter_tags.R'
     'lintr-deprecated.R'
     'lintr-package.R'
+    'list_comparison_linter.R'
     'literal_coercion_linter.R'
     'make_linter_from_regex.R'
     'matrix_apply_linter.R'
@@ -136,11 +141,16 @@ Collate:
     'namespace.R'
     'namespace_linter.R'
     'nested_ifelse_linter.R'
+    'nested_pipe_linter.R'
     'nonportable_path_linter.R'
+    'nrow_subset_linter.R'
     'numeric_leading_zero_linter.R'
+    'nzchar_linter.R'
     'object_length_linter.R'
     'object_name_linter.R'
+    'object_overwrite_linter.R'
     'object_usage_linter.R'
+    'one_call_pipe_linter.R'
     'outer_negation_linter.R'
     'package_hooks_linter.R'
     'paren_body_linter.R'
@@ -149,12 +159,17 @@ Collate:
     'pipe_call_linter.R'
     'pipe_consistency_linter.R'
     'pipe_continuation_linter.R'
+    'pipe_return_linter.R'
+    'print_linter.R'
     'quotes_linter.R'
     'redundant_equals_linter.R'
     'redundant_ifelse_linter.R'
     'regex_subset_linter.R'
+    'rep_len_linter.R'
     'repeat_linter.R'
+    'return_linter.R'
     'routine_registration_linter.R'
+    'sample_int_linter.R'
     'scalar_in_linter.R'
     'semicolon_linter.R'
     'seq_linter.R'
@@ -165,9 +180,11 @@ Collate:
     'spaces_inside_linter.R'
     'spaces_left_parentheses_linter.R'
     'sprintf_linter.R'
+    'stopifnot_all_linter.R'
     'string_boundary_linter.R'
     'strings_as_factors_linter.R'
     'system_file_linter.R'
+    'terminal_close_linter.R'
     'trailing_blank_lines_linter.R'
     'trailing_whitespace_linter.R'
     'tree_utils.R'
@@ -176,11 +193,13 @@ Collate:
     'unnecessary_concatenation_linter.R'
     'unnecessary_lambda_linter.R'
     'unnecessary_nested_if_linter.R'
+    'unnecessary_nesting_linter.R'
     'unnecessary_placeholder_linter.R'
     'unreachable_code_linter.R'
     'unused_import_linter.R'
     'use_lintr.R'
     'vector_logic_linter.R'
+    'which_grepl_linter.R'
     'whitespace_linter.R'
     'with.R'
     'with_id.R'

NAMESPACE

@@ -36,9 +36,12 @@ export(clear_cache)
 export(closed_curly_linter)
 export(commas_linter)
 export(commented_code_linter)
+export(comparison_negation_linter)
+export(condition_call_linter)
 export(condition_message_linter)
 export(conjunct_test_linter)
 export(consecutive_assertion_linter)
+export(consecutive_mutate_linter)
 export(consecutive_stopifnot_linter)
 export(cyclocomp_linter)
 export(default_linters)
@@ -70,6 +73,7 @@ export(get_r_string)
 export(get_source_expressions)
 export(ids_with_token)
 export(if_not_else_linter)
+export(if_switch_linter)
 export(ifelse_censor_linter)
 export(implicit_assignment_linter)
 export(implicit_integer_linter)
@@ -89,6 +93,7 @@ export(lint_dir)
 export(lint_package)
 export(linters_with_defaults)
 export(linters_with_tags)
+export(list_comparison_linter)
 export(literal_coercion_linter)
 export(make_linter_from_xpath)
 export(matrix_apply_linter)
@@ -97,12 +102,17 @@ export(missing_package_linter)
 export(modify_defaults)
 export(namespace_linter)
 export(nested_ifelse_linter)
+export(nested_pipe_linter)
 export(no_tab_linter)
 export(nonportable_path_linter)
+export(nrow_subset_linter)
 export(numeric_leading_zero_linter)
+export(nzchar_linter)
 export(object_length_linter)
 export(object_name_linter)
+export(object_overwrite_linter)
 export(object_usage_linter)
+export(one_call_pipe_linter)
 export(open_curly_linter)
 export(outer_negation_linter)
 export(package_hooks_linter)
@@ -112,12 +122,17 @@ export(paste_linter)
 export(pipe_call_linter)
 export(pipe_consistency_linter)
 export(pipe_continuation_linter)
+export(pipe_return_linter)
+export(print_linter)
 export(quotes_linter)
 export(redundant_equals_linter)
 export(redundant_ifelse_linter)
 export(regex_subset_linter)
+export(rep_len_linter)
 export(repeat_linter)
+export(return_linter)
 export(routine_registration_linter)
+export(sample_int_linter)
 export(sarif_output)
 export(scalar_in_linter)
 export(semicolon_linter)
@@ -128,9 +143,11 @@ export(sort_linter)
 export(spaces_inside_linter)
 export(spaces_left_parentheses_linter)
 export(sprintf_linter)
+export(stopifnot_all_linter)
 export(string_boundary_linter)
 export(strings_as_factors_linter)
 export(system_file_linter)
+export(terminal_close_linter)
 export(todo_comment_linter)
 export(trailing_blank_lines_linter)
 export(trailing_whitespace_linter)
@@ -139,12 +156,14 @@ export(undesirable_operator_linter)
 export(unnecessary_concatenation_linter)
 export(unnecessary_lambda_linter)
 export(unnecessary_nested_if_linter)
+export(unnecessary_nesting_linter)
 export(unnecessary_placeholder_linter)
 export(unneeded_concatenation_linter)
 export(unreachable_code_linter)
 export(unused_import_linter)
 export(use_lintr)
 export(vector_logic_linter)
+export(which_grepl_linter)
 export(whitespace_linter)
 export(with_defaults)
 export(with_id)

NEWS.md

@@ -1,10 +1,73 @@
 # lintr (development version)
 
+## Deprecations & breaking changes
+
+* Various things marked deprecated since {lintr} 3.0.0 have been fully deprecated. They will be completely removed in the subsequent release.
+   + `source_file=` argument to `ids_with_token()` and `with_id()`.
+   + Passing linters by name or as non-`"linter"`-classed functions.
+   + `linter=` argument of `Lint()`.
+   + Linters `closed_curly_linter()`, `open_curly_linter()`, `paren_brace_linter()`, and `semicolon_terminator_linter()`..
+   + `with_defaults()`.
+   + Linters `closed_curly_linter()`, `open_curly_linter()`, `paren_brace_linter()`, and `semicolon_terminator_linter()`.
+   + Helper `with_defaults()`.
+* `all_linters()` has signature `all_linters(..., packages)` rather than `all_linters(packages, ...)` (#2332, @MichaelChirico). This forces `packages=` to be supplied by name and will break users who rely on supplying `packages=` positionally, of which we found none searching GitHub.
+
+## Bug fixes
+
+* `object_name_linter()` no longer errors when user-supplied `regexes=` have capture groups (#2188, @MichaelChirico).
+* `.lintr` config validation correctly accepts regular exressions which only compile under `perl = TRUE` (#2375, @MichaelChirico). These have always been valid (since `rex::re_matches()`, which powers the lint exclusion logic, also uses this setting), but the new up-front validation in v3.1.1 incorrectly used `perl = FALSE`.
+
+## Changes to default linters
+
+* New default linter `return_linter()` for the style guide rule that terminal returns should be left implicit (#1100, @MEO265).
+
+## New and improved features
+
+* More helpful errors for invalid configs (#2253, @MichaelChirico).
+* `library_call_linter()` is extended
+  + to encourage all packages to be attached with `library(symbol)`, not `library("symbol", character.only = TRUE)` or "vectorized" approaches looping over package names (part of #884, @MichaelChirico).
+  + to discourage many consecutive calls to `suppressMessages()` or `suppressPackageStartupMessages()` (part of #884, @MichaelChirico).
+* `return_linter()` also has an argument `return_style` (`"implicit"` by default) which checks that all functions confirm to the specified return style of `"implicit"` or `"explicit"` (part of #884, @MichaelChirico, @AshesITR and @MEO265).
+* `unnecessary_lambda_linter` is extended to encourage vectorized comparisons where possible, e.g. `sapply(x, sum) > 0` instead of `sapply(x, function(x) sum(x) > 0)` (part of #884, @MichaelChirico). Toggle this behavior with argument `allow_comparison`.
+* `backport_linter()` is slightly faster by moving expensive computations outside the linting function (#2339, #2348, @AshesITR and @MichaelChirico).
+* `Linter()` has a new argument `linter_level` (default `NA`). This is used by `lint()` to more efficiently check for expression levels than the idiom `if (!is_lint_level(...)) { return(list()) }` (#2351, @AshesITR).
+* `format()` and `print()` methods for `lint` and `lints` classes get a new option `width` to control the printing width of lint messages (#1884, @MichaelChirico). The default is controlled by a new option `lintr.format_width`; if unset, no wrapping occurs (matching earlier behavior).
+
+### New linters
+
+* `condition_call_linter()` for ensuring consistent use of `call.` in `warning()` and `stop()`. The default `call. = FALSE` follows the tidyverse guidance of not displaying the call (#2226, @Bisaloo)
+* `sample_int_linter()` for encouraging `sample.int(n, ...)` over equivalents like `sample(1:n, ...)` (part of #884, @MichaelChirico).
+* `stopifnot_all_linter()` discourages tests with `all()` like `stopifnot(all(x > 0))`; `stopifnot()` runs `all()` itself, and uses a better error message (part of #884, @MichaelChirico).
+* `comparison_negation_linter()` for discouraging negated comparisons when a direct negation is preferable, e.g. `!(x == y)` could be `x != y` (part of #884, @MichaelChirico).
+* `nzchar_linter()` for encouraging `nzchar()` to test for empty strings, e.g. `nchar(x) > 0` can be `nzchar(x)` (part of #884, @MichaelChirico).
+* `terminal_close_linter()` for discouraging using `close()` to end functions (part of #884, @MichaelChirico). Such usages are not robust to errors, where `close()` will not be run as intended. Put `close()` in an `on.exit()` hook, or use {withr} to manage connections with proper cleanup.
+* `rep_len_linter()` for encouraging use of `rep_len()` directly instead of `rep(x, length.out = n)` (part of #884, @MichaelChirico).
+* `which_grepl_linter()` for discouraging `which(grepl(ptn, x))` in favor of directly using `grep(ptn, x)` (part of #884, @MichaelChirico).
+* `list_comparison_linter()` for discouraging comparisons on the output of `lapply()`, e.g. `lapply(x, sum) > 10` (part of #884, @MichaelChirico).
+* `print_linter()` for discouraging usage of `print()` on string literals like `print("Reached here")` or `print(paste("Found", nrow(DF), "rows."))` (#1894, @MichaelChirico).
+* `unnecessary_nesting_linter()` for discouraging overly-nested code where an early return or eliminated sub-expression (inside '{') is preferable (part of #884, @MichaelChirico).
+* `consecutive_mutate_linter()` for encouraging consecutive calls to `dplyr::mutate()` to be combined (part of #884, @MichaelChirico).
+* `if_switch_linter()` for encouraging `switch()` over repeated `if`/`else` tests (part of #884, @MichaelChirico).
+* `nested_pipe_linter()` for discouraging pipes within pipes, e.g. `df1 %>% inner_join(df2 %>% select(a, b))` (part of #884, @MichaelChirico).
+* `nrow_subset_linter()` for discouraging usage like `nrow(subset(x, conditions))` in favor of something like `with(x, sum(conditions))` which doesn't require a full subset of `x` (part of #884, @MichaelChirico).
+* `pipe_return_linter()` for discouraging usage of `return()` inside a {magrittr} pipeline (part of #884, @MichaelChirico).
+* `one_call_pipe_linter()` for discouraging one-step pipelines like `x |> as.character()` (#2330 and part of #884, @MichaelChirico).
+* `object_overwrite_linter()` for discouraging re-use of upstream package exports as local variables (#2344, #2346 and part of #884, @MichaelChirico and @AshesITR).
+
+### Lint accuracy fixes: removing false positives
+
+* `unreachable_code_linter()` ignores reachable code in inline functions like `function(x) if (x > 2) stop() else x` (#2259, @MEO265).
+* `unnecessary_lambda_linter()`
+  + ignores extractions with explicit returns like `lapply(l, function(x) foo(x)$bar)` (#2258, @MichaelChirico).
+  + ignores calls on the RHS of operators like `lapply(l, function(x) "a" %in% names(x))` (#2310, @MichaelChirico).
+
+# lintr 3.1.1
+
 ## Breaking changes
 
 * `infix_spaces_linter()` distinguishes `<-`, `:=`, `<<-` and `->`, `->>`, i.e. `infix_spaces_linter(exclude_operators = "->")` will no longer exclude `->>` (#2115, @MichaelChirico). This change is breaking for users relying on manually-supplied `exclude_operators` containing `"<-"` to also exclude `:=` and `<<-`. The fix is to manually supply `":="` and `"<<-"` as well. We don't expect this change to affect many users, the fix is simple, and the new behavior is much more transparent, so we are including this breakage in a minor release.
 * Removed `find_line()` and `find_column()` entries from `get_source_expressions()` expression-level objects. These have been marked deprecated since version 3.0.0. No users were found on GitHub.
-* There is experimental support for writing config in plain R scripts (as opposed to DCF files; #1210, @MichaelChirico). The script is run in a new environment and variables matching settings (`?default_settings`) are copied over. In particular, this removes the need to write R code in a DCF-friendly way, and allows normal R syntax highlighting in the saved file. We may eventually deprecate the DCF approach in favor of this one; user feedback is welcome on strong preferences for either approach, or for a different approach like YAML. Generally you should be able to convert your existing `.lintr` file to an equivalent R config by replacing the `:` key-value separators with assignments (`<-`). By default, such a config is searched for in a file named '.lintr.R'. This is a mildly breaking change if you happened to be keeping a file '.lintr.R' around since that file is given precedence over '.lintr'.
+* There is experimental support for writing config in plain R scripts (as opposed to DCF files; #1210, @MichaelChirico). The script is run in a new environment and variables matching settings (`?default_settings`) are copied over. In particular, this removes the need to write R code in a DCF-friendly way, and allows normal R syntax highlighting in the saved file. We may eventually deprecate the DCF approach in favor of this one; user feedback is welcome on strong preferences for either approach, or for a different approach like YAML. Generally you should be able to convert your existing `.lintr` file to an equivalent R config by replacing the `:` key-value separators with assignments (`<-`). By default, such a config is searched for in a file named `.lintr.R`. This is a mildly breaking change if you happened to be keeping a file `.lintr.R` around since that file is given precedence over `.lintr`.
   + We also validate config files up-front make it clearer when invalid configs are present (#2195, @MichaelChirico). There is a warning for "invalid" settings, i.e., settings not part of `?default_settings`. We think this is more likely to affect users declaring settings in R, since any variable defined in the config that's not a setting must be removed to make it clearer which variables are settings vs. ancillary.
 
 ## Bug fixes

R/T_and_F_symbol_linter.R

@@ -43,13 +43,11 @@ T_and_F_symbol_linter <- function() { # nolint: object_name.
 
   replacement_map <- c(T = "TRUE", F = "FALSE")
 
-  Linter(function(source_expression) {
-    if (!is_lint_level(source_expression, "expression")) {
-      return(list())
-    }
-
-    bad_usage <- xml_find_all(source_expression$xml_parsed_content, usage_xpath)
-    bad_assignment <- xml_find_all(source_expression$xml_parsed_content, assignment_xpath)
+  Linter(linter_level = "expression", function(source_expression) {
+    xml <- source_expression$xml_parsed_content
+    if (is.null(xml)) return(list())
+    bad_usage <- xml_find_all(xml, usage_xpath)
+    bad_assignment <- xml_find_all(xml, assignment_xpath)
 
     make_lints <- function(expr, fmt) {
       symbol <- xml_text(expr)

R/addins.R

@@ -1,7 +1,7 @@
 # nocov start
 addin_lint <- function() {
   if (!requireNamespace("rstudioapi", quietly = TRUE)) {
-    stop("'rstudioapi' is required for add-ins.")
+    stop("'rstudioapi' is required for add-ins.", call. = FALSE)
   }
   filename <- rstudioapi::getSourceEditorContext()
   if (filename$path == "") {
@@ -27,7 +27,7 @@ addin_lint <- function() {
 
 addin_lint_package <- function() {
   if (!requireNamespace("rstudioapi", quietly = TRUE)) {
-    stop("'rstudioapi' is required for add-ins.")
+    stop("'rstudioapi' is required for add-ins.", call. = FALSE)
   }
   project <- rstudioapi::getActiveProject()
   project_path <- if (is.null(project)) getwd() else project