From fdbb9bfa1ea868ad66c6fe9a30b4ae872c03f56e 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               | 15 +++++++++------
 .../test-consecutive_assertion_linter.R        | 18 +++++++++++++++++-
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index dac147907..db76f264c 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -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
 
diff --git a/R/consecutive_assertion_linter.R b/R/consecutive_assertion_linter.R
index 525edf117..668ed1593 100644
--- a/R/consecutive_assertion_linter.R
+++ b/R/consecutive_assertion_linter.R
@@ -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
diff --git a/tests/testthat/test-consecutive_assertion_linter.R b/tests/testthat/test-consecutive_assertion_linter.R
index f3df750a7..35fe6d3f6 100644
--- a/tests/testthat/test-consecutive_assertion_linter.R
+++ b/tests/testthat/test-consecutive_assertion_linter.R
@@ -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)
@@ -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()
+  )
+})