New consecutive_stopifnot_linter (#1005)

Author: MichaelChirico

DESCRIPTION

@@ -61,6 +61,7 @@ Collate:
     'comment_linters.R'
     'comments.R'
     'conjunct_expectation_linter.R'
+    'consecutive_stopifnot_linter.R'
     'cyclocomp_linter.R'
     'declared_functions.R'
     'deprecated.R'

NAMESPACE

@@ -25,6 +25,7 @@ export(closed_curly_linter)
 export(commas_linter)
 export(commented_code_linter)
 export(conjunct_expectation_linter)
+export(consecutive_stopifnot_linter)
 export(cyclocomp_linter)
 export(default_linters)
 export(default_settings)

NEWS.md

@@ -107,6 +107,7 @@ function calls. (#850, #851, @renkun-ken)
    * `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
    * `unreachable_code_linter()` Prevent code after `return()` and `stop()` statements that will never be reached
+   * `consecutive_stopifnot_linter()` Require consecutive calls to `stopifnot()` to be unified into one
 * `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)
 * `infix_spaces_linter()` now throws a lint on `a~b` and `function(a=1) {}` (#930, @michaelchirico)

R/consecutive_stopifnot_linter.R

@@ -0,0 +1,36 @@
+#' Force consecutive calls to stopifnot into just one when possible
+#'
+#' [stopifnot()] accepts any number of tests, so sequences like
+#'   `stopifnot(x); stopifnot(y)` are redundant.
+#'
+#' @evalRd rd_tags("consecutive_stopifnot_linter")
+#' @seealso [linters] for a complete list of linters available in lintr.
+#' @export
+consecutive_stopifnot_linter <- function() {
+  Linter(function(source_file) {
+    # need the full file to also catch usages at the top level
+    if (length(source_file$full_xml_parsed_content) == 0L) {
+      return(list())
+    }
+
+    xml <- source_file$full_xml_parsed_content
+
+    # match on the expr, not the SYMBOL_FUNCTION_CALL, to ensure
+    #   namespace-qualified calls only match if the namespaces do.
+    xpath <- glue::glue("
+    //expr[
+      expr[SYMBOL_FUNCTION_CALL[text() = 'stopifnot']] = following-sibling::expr[1]/expr
+    ]
+    ")
+    bad_expr <- xml2::xml_find_all(xml, xpath)
+
+    return(lapply(
+      bad_expr,
+      xml_nodes_to_lint,
+      source_file = source_file,
+      lint_message = "Unify consecutive calls to stopifnot().",
+      type = "warning",
+      global = TRUE
+    ))
+  })
+}

inst/lintr/linters.csv

@@ -9,6 +9,7 @@ closed_curly_linter,style readability default configurable
 commas_linter,style readability default
 commented_code_linter,style readability best_practices default
 conjunct_expectation_linter,package_development best_practices readability
+consecutive_stopifnot_linter,style readability consistency
 cyclocomp_linter,style readability best_practices default configurable
 duplicate_argument_linter,correctness common_mistakes configurable
 equals_na_linter,robustness correctness common_mistakes default

man/consecutive_stopifnot_linter.Rd

@@ -0,0 +1,57 @@
+test_that("consecutive_stopifnot_linter skips allowed usages", {
+  expect_lint("stopifnot(x)", NULL, consecutive_stopifnot_linter())
+  expect_lint("stopifnot(x, y, z)", NULL, consecutive_stopifnot_linter())
+
+  # intervening expression
+  expect_lint("stopifnot(x); y; stopifnot(z)", NULL, consecutive_stopifnot_linter())
+
+  # inline or potentially with gaps don't matter
+  lines <- trim_some("
+    stopifnot(x)
+    y
+
+    stopifnot(z)
+  ")
+  expect_lint(lines, NULL, consecutive_stopifnot_linter())
+})
+
+test_that("consecutive_stopifnot_linter blocks simple disallowed usages", {
+  # one test of inline usage
+  expect_lint(
+    "stopifnot(x); stopifnot(y)",
+    rex::rex("Unify consecutive calls to stopifnot()."),
+    consecutive_stopifnot_linter()
+  )
+
+  lines_gap <- trim_some("
+    stopifnot(x)
+
+    stopifnot(y, z)
+  ")
+  expect_lint(
+    lines_gap,
+    rex::rex("Unify consecutive calls to stopifnot()."),
+    consecutive_stopifnot_linter()
+  )
+
+  lines_consecutive <- trim_some("
+    stopifnot(x)
+    stopifnot(y)
+  ")
+  expect_lint(
+    lines_consecutive,
+    rex::rex("Unify consecutive calls to stopifnot()."),
+    consecutive_stopifnot_linter()
+  )
+
+  lines_comment <- trim_some("
+    stopifnot(x)
+    # a comment on y
+    stopifnot(y)
+  ")
+  expect_lint(
+    lines_comment,
+    rex::rex("Unify consecutive calls to stopifnot()."),
+    consecutive_stopifnot_linter()
+  )
+})