From 7f72cf014f53481f669059e4be5b548697b6b72e Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 28 Mar 2022 03:40:04 +0000 Subject: [PATCH] New consecutive_stopifnot_linter --- DESCRIPTION | 1 + NAMESPACE | 1 + NEWS.md | 1 + R/consecutive_stopifnot_linter.R | 36 ++++++++++++ inst/lintr/linters.csv | 1 + man/consecutive_stopifnot_linter.Rd | 18 ++++++ man/consistency_linters.Rd | 1 + man/linters.Rd | 7 ++- man/readability_linters.Rd | 1 + man/style_linters.Rd | 1 + .../test-consecutive_stopifnot_linter.R | 57 +++++++++++++++++++ 11 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 R/consecutive_stopifnot_linter.R create mode 100644 man/consecutive_stopifnot_linter.Rd create mode 100644 tests/testthat/test-consecutive_stopifnot_linter.R diff --git a/DESCRIPTION b/DESCRIPTION index 15ce73d105..910c2b7454 100644 --- a/DESCRIPTION +++ b/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' diff --git a/NAMESPACE b/NAMESPACE index da384b4f59..fc0019bded 100644 --- a/NAMESPACE +++ b/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) diff --git a/NEWS.md b/NEWS.md index ac9e707c06..43801e1649 100644 --- a/NEWS.md +++ b/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) diff --git a/R/consecutive_stopifnot_linter.R b/R/consecutive_stopifnot_linter.R new file mode 100644 index 0000000000..73af76a975 --- /dev/null +++ b/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 + )) + }) +} diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index f94652e94a..08940568eb 100644 --- a/inst/lintr/linters.csv +++ b/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 diff --git a/man/consecutive_stopifnot_linter.Rd b/man/consecutive_stopifnot_linter.Rd new file mode 100644 index 0000000000..470b3e80bd --- /dev/null +++ b/man/consecutive_stopifnot_linter.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/consecutive_stopifnot_linter.R +\name{consecutive_stopifnot_linter} +\alias{consecutive_stopifnot_linter} +\title{Force consecutive calls to stopifnot into just one when possible} +\usage{ +consecutive_stopifnot_linter() +} +\description{ +\code{stopifnot()} accepts any number of tests, so sequences like +\verb{stopifnot(x); stopifnot(y)} are redundant. +} +\seealso{ +\link{linters} for a complete list of linters available in lintr. +} +\section{Tags}{ +\link[=consistency_linters]{consistency}, \link[=readability_linters]{readability}, \link[=style_linters]{style} +} diff --git a/man/consistency_linters.Rd b/man/consistency_linters.Rd index 98fbbbc273..758ec08e73 100644 --- a/man/consistency_linters.Rd +++ b/man/consistency_linters.Rd @@ -15,6 +15,7 @@ The following linters are tagged with 'consistency': \itemize{ \item{\code{\link{assignment_linter}}} \item{\code{\link{class_equals_linter}}} +\item{\code{\link{consecutive_stopifnot_linter}}} \item{\code{\link{implicit_integer_linter}}} \item{\code{\link{literal_coercion_linter}}} \item{\code{\link{no_tab_linter}}} diff --git a/man/linters.Rd b/man/linters.Rd index a09d2ba224..a4631232c2 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -20,14 +20,14 @@ The following tags exist: \item{\link[=best_practices_linters]{best_practices} (28 linters)} \item{\link[=common_mistakes_linters]{common_mistakes} (5 linters)} \item{\link[=configurable_linters]{configurable} (16 linters)} -\item{\link[=consistency_linters]{consistency} (11 linters)} +\item{\link[=consistency_linters]{consistency} (12 linters)} \item{\link[=correctness_linters]{correctness} (7 linters)} \item{\link[=default_linters]{default} (28 linters)} \item{\link[=efficiency_linters]{efficiency} (10 linters)} \item{\link[=package_development_linters]{package_development} (13 linters)} -\item{\link[=readability_linters]{readability} (31 linters)} +\item{\link[=readability_linters]{readability} (32 linters)} \item{\link[=robustness_linters]{robustness} (11 linters)} -\item{\link[=style_linters]{style} (34 linters)} +\item{\link[=style_linters]{style} (35 linters)} } } \section{Linters}{ @@ -43,6 +43,7 @@ The following linters exist: \item{\code{\link{commas_linter}} (tags: default, readability, style)} \item{\code{\link{commented_code_linter}} (tags: best_practices, default, readability, style)} \item{\code{\link{conjunct_expectation_linter}} (tags: best_practices, package_development, readability)} +\item{\code{\link{consecutive_stopifnot_linter}} (tags: consistency, readability, style)} \item{\code{\link{cyclocomp_linter}} (tags: best_practices, configurable, default, readability, style)} \item{\code{\link{duplicate_argument_linter}} (tags: common_mistakes, configurable, correctness)} \item{\code{\link{equals_na_linter}} (tags: common_mistakes, correctness, default, robustness)} diff --git a/man/readability_linters.Rd b/man/readability_linters.Rd index 27e3e2945b..404e969504 100644 --- a/man/readability_linters.Rd +++ b/man/readability_linters.Rd @@ -16,6 +16,7 @@ The following linters are tagged with 'readability': \item{\code{\link{commas_linter}}} \item{\code{\link{commented_code_linter}}} \item{\code{\link{conjunct_expectation_linter}}} +\item{\code{\link{consecutive_stopifnot_linter}}} \item{\code{\link{cyclocomp_linter}}} \item{\code{\link{expect_length_linter}}} \item{\code{\link{expect_named_linter}}} diff --git a/man/style_linters.Rd b/man/style_linters.Rd index ba523553cd..bf79e385c2 100644 --- a/man/style_linters.Rd +++ b/man/style_linters.Rd @@ -16,6 +16,7 @@ The following linters are tagged with 'style': \item{\code{\link{closed_curly_linter}}} \item{\code{\link{commas_linter}}} \item{\code{\link{commented_code_linter}}} +\item{\code{\link{consecutive_stopifnot_linter}}} \item{\code{\link{cyclocomp_linter}}} \item{\code{\link{extraction_operator_linter}}} \item{\code{\link{function_brace_linter}}} diff --git a/tests/testthat/test-consecutive_stopifnot_linter.R b/tests/testthat/test-consecutive_stopifnot_linter.R new file mode 100644 index 0000000000..4d7bdeb014 --- /dev/null +++ b/tests/testthat/test-consecutive_stopifnot_linter.R @@ -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() + ) +})