New paste_sep_linter (#997)

DESCRIPTION

@@ -102,6 +102,7 @@ Collate:
     'package_hooks_linter.R'
     'paren_body_linter.R'
     'paren_brace_linter.R'
+    'paste_sep_linter.R'
     'path_linters.R'
     'pipe_call_linter.R'
     'pipe_continuation_linter.R'

NAMESPACE

@@ -68,6 +68,7 @@ export(outer_negation_linter)
 export(package_hooks_linter)
 export(paren_body_linter)
 export(paren_brace_linter)
+export(paste_sep_linter)
 export(pipe_call_linter)
 export(pipe_continuation_linter)
 export(semicolon_terminator_linter)

NEWS.md

@@ -102,6 +102,7 @@ function calls. (#850, #851, @renkun-ken)
    * `any_is_na_linter()` Require usage of `anyNA(x)` over `any(is.na(x))`
    * `outer_negation_linter()` Require usage of `!any(x)` over `all(!x)` and `!all(x)` over `any(!x)`
    * `numeric_leading_zero_linter()` Require a leading `0` in fractional numeric constants, e.g. `0.1` instead of `.1`
+   * `paste_sep_linter()` Require usage of `paste0()` over `paste(sep = "")`
    * `nested_ifelse_linter()` Prevent nested calls to `ifelse()` like `ifelse(A, x, ifelse(B, y, z))`, and similar
 * `assignment_linter()` now lints right assignment (`->` and `->>`) and gains two arguments. `allow_cascading_assign` (`TRUE` by default) toggles whether to lint `<<-` and `->>`; `allow_right_assign` toggles whether to lint `->` and `->>` (#915, @michaelchirico)
 * `infix_spaces_linter()` gains argument `exclude_operators` to disable lints on selected infix operators. By default, all "low-precedence" operators throw lints; see `?infix_spaces_linter` for an enumeration of these. (#914 @michaelchirico)

R/paste_sep_linter.R

@@ -0,0 +1,33 @@
+#' Block usage of paste() with sep=""
+#'
+#' [paste0()] is a faster, more concise alternative to using `paste(sep = "")`.
+#'
+#' @evalRd rd_tags("paste_sep_linter")
+#' @seealso [linters] for a complete list of linters available in lintr.
+#' @export
+paste_sep_linter <- function() {
+  Linter(function(source_file) {
+    if (length(source_file$xml_parsed_content) == 0L) {
+      return(list())
+    }
+
+    xml <- source_file$xml_parsed_content
+
+    # NB: string-length()=2 means '' or "" (*not* 'ab' or 'cd' which have length 4)
+    # TODO: adapt this for R>4.0 raw strings
+    xpath <- "//expr[
+      expr[SYMBOL_FUNCTION_CALL[text() = 'paste']]
+      and SYMBOL_SUB[text() = 'sep']/following-sibling::expr[1][STR_CONST[string-length(text()) = 2]]
+    ]"
+
+    bad_expr <- xml2::xml_find_all(xml, xpath)
+
+    return(lapply(
+      bad_expr,
+      xml_nodes_to_lint,
+      source_file = source_file,
+      lint_message = 'paste0(...) is better than paste(..., sep = "").',
+      type = "warning"
+    ))
+  })
+}

inst/lintr/linters.csv

@@ -41,6 +41,7 @@ outer_negation_linter,readability efficiency best_practices
 package_hooks_linter,style correctness package_development
 paren_body_linter,style readability default
 paren_brace_linter,style readability default
+paste_sep_linter,best_practices consistency
 pipe_call_linter,style readability
 pipe_continuation_linter,style readability default
 semicolon_terminator_linter,style readability default configurable

tests/testthat/test-paste_sep_linter.R

@@ -0,0 +1,24 @@
+test_that("paste_sep_linter skips allowed usages", {
+  expect_lint("paste('a', 'b', 'c')", NULL, paste_sep_linter())
+  expect_lint("paste('a', 'b', 'c', sep = ',')", NULL, paste_sep_linter())
+  expect_lint("paste('a', 'b', collapse = '')", NULL, paste_sep_linter())
+  expect_lint("cat(paste('a', 'b'), sep = '')", NULL, paste_sep_linter())
+  expect_lint("sep <- ''; paste('a', sep)", NULL, paste_sep_linter())
+  expect_lint("paste(sep = ',', '', 'a')", NULL, paste_sep_linter())
+
+  expect_lint("paste0('a', 'b', 'c')", NULL, paste_sep_linter())
+})
+
+test_that("paste_sep_linter blocks simple disallowed usages", {
+  expect_lint(
+    "paste(sep = '', 'a', 'b')",
+    rex::rex('paste0(...) is better than paste(..., sep = "").'),
+    paste_sep_linter()
+  )
+
+  expect_lint(
+    "paste('a', 'b', sep = '')",
+    rex::rex('paste0(...) is better than paste(..., sep = "").'),
+    paste_sep_linter()
+  )
+})