From 1d28f7e2edac7334b6835f0e34ff718e36c6efaa Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Fri, 15 Dec 2023 19:37:12 +0800
Subject: [PATCH] following expr can be other expr-ish nodes (#2454)

---
 NEWS.md                                       |  1 +
 R/consecutive_assertion_linter.R              | 20 ++++++------
 .../test-consecutive_assertion_linter.R       | 32 +++++++++++++++++++
 3 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 0f2c12b1a..597785635 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -13,6 +13,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).
 
 # lintr 3.1.1
 
diff --git a/R/consecutive_assertion_linter.R b/R/consecutive_assertion_linter.R
index 22042f754..f16929d04 100644
--- a/R/consecutive_assertion_linter.R
+++ b/R/consecutive_assertion_linter.R
@@ -31,21 +31,23 @@
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 consecutive_assertion_linter <- function() {
-  xpath <- "
-  //SYMBOL_FUNCTION_CALL[text() = 'stopifnot']
-    /parent::expr
+  # 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
     ]
-  |
-  //SYMBOL_FUNCTION_CALL[text() = 'assert_that']
-    /parent::expr
+  ")
+  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(function(source_expression) {
     # need the full file to also catch usages at the top level
diff --git a/tests/testthat/test-consecutive_assertion_linter.R b/tests/testthat/test-consecutive_assertion_linter.R
index 362e3ba18..35fe6d3f6 100644
--- a/tests/testthat/test-consecutive_assertion_linter.R
+++ b/tests/testthat/test-consecutive_assertion_linter.R
@@ -100,6 +100,22 @@ test_that("Mixing test functions is fine", {
   )
 })
 
+test_that("lints vectorize", {
+  expect_lint(
+    trim_some("{
+      stopifnot(A)
+      stopifnot(B)
+      assert_that(C)
+      assert_that(D)
+    }"),
+    list(
+      list("stopifnot", line_number = 2L),
+      list("assert_that", line_number = 4L)
+    ),
+    consecutive_assertion_linter()
+  )
+})
+
 test_that("old name consecutive_stopifnot_linter() is deprecated", {
   expect_warning(
     {
@@ -111,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()
+  )
+})