Skip to content

New consecutive_stopifnot_linter #1005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 36 additions & 0 deletions R/consecutive_stopifnot_linter.R
Original file line number Diff line number Diff line change
@@ -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
))
})
}
1 change: 1 addition & 0 deletions inst/lintr/linters.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions man/consecutive_stopifnot_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/consistency_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions man/linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/readability_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/style_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 57 additions & 0 deletions tests/testthat/test-consecutive_stopifnot_linter.R
Original file line number Diff line number Diff line change
@@ -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()
)
})