Skip to content

Commit

Permalink
following expr can be other expr-ish nodes (#2454)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Dec 15, 2023
1 parent 5b99e6e commit fdbb9bf
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 7 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
+ ignores calls on the RHS of operators like `lapply(l, function(x) "a" %in% names(x))` (#2310, @MichaelChirico).
* `vector_logic_linter()` recognizes some cases where bitwise `&`/`|` are used correctly (#1453, @MichaelChirico).
* `expect_comparison_linter()` ignores faulty usage like `expect_true(x, y > z)` (#2083, @MichaelChirico). Note that `y > z` is being passed to the `info=` argument, so this is likely a mistake.
* `consecutive_assertion_linter()` ignores cases where a second asssertion follows assignment with `=` (#2444, @MichaelChirico).

### Lint accuracy fixes: removing false negatives

Expand Down
15 changes: 9 additions & 6 deletions R/consecutive_assertion_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,23 @@
#' @seealso [linters] for a complete list of linters available in lintr.
#' @export
consecutive_assertion_linter <- function() {
stopifnot_xpath <- "
# annoying expr-but-not-really nodes
next_expr <- "following-sibling::*[self::expr or self::expr_or_assign_or_help or self::equal_assign][1]"

stopifnot_xpath <- glue("
parent::expr
/parent::expr[
expr[1]/SYMBOL_FUNCTION_CALL = following-sibling::expr[1]/expr[1]/SYMBOL_FUNCTION_CALL
expr[1]/SYMBOL_FUNCTION_CALL = {next_expr}/expr[1]/SYMBOL_FUNCTION_CALL
]
"
assert_that_xpath <- "
")
assert_that_xpath <- glue("
parent::expr
/parent::expr[
not(SYMBOL_SUB[text() = 'msg'])
and not(following-sibling::expr[1]/SYMBOL_SUB[text() = 'msg'])
and expr[1]/SYMBOL_FUNCTION_CALL = following-sibling::expr[1]/expr[1]/SYMBOL_FUNCTION_CALL
and expr[1]/SYMBOL_FUNCTION_CALL = {next_expr}/expr[1]/SYMBOL_FUNCTION_CALL
]
"
")

Linter(linter_level = "file", function(source_expression) {
# need the full file to also catch usages at the top level
Expand Down
18 changes: 17 additions & 1 deletion tests/testthat/test-consecutive_assertion_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ test_that("Mixing test functions is fine", {
)
})

test_that("lints vectorie", {
test_that("lints vectorize", {
expect_lint(
trim_some("{
stopifnot(A)
Expand All @@ -127,3 +127,19 @@ test_that("old name consecutive_stopifnot_linter() is deprecated", {
expect_lint("stopifnot(x); y; stopifnot(z)", NULL, old_linter)
expect_lint("stopifnot(x); stopifnot(y)", "Unify consecutive calls", old_linter)
})

test_that("interceding = assignments aren't linted", {
expect_lint(
trim_some("{
stopifnot(A)
x = 1
stopifnot(B)
assert_that(C)
z = 3
assert_that(D)
}"),
NULL,
consecutive_assertion_linter()
)
})

0 comments on commit fdbb9bf

Please sign in to comment.