expect_true_false_linter() is pipe-aware

NEWS.md

@@ -47,6 +47,7 @@
 * `return_linter()` works on functions that happen to use braced expressions in their formals (#2616, @MichaelChirico).
 * `object_name_linter()` and `object_length_linter()` account for S3 class correctly when the generic is assigned with `=` (#2507, @MichaelChirico).
 * `assignment_linter()` with `operator = "="` does a better job of skipping implicit assignments, which are intended to be governed by `implicit_assignment_linter()` (#2765, @MichaelChirico).
+* `expect_true_false_linter()` is pipe-aware, so that `42 |> expect_identical(x, ignore_attr = TRUE)` no longer lints (#1520, @MichaelChirico).
 * `T_and_F_symbol_linter()` ignores `T` and `F` used as symbols in formulas (`y ~ T + F`), which can represent variables in data not controlled by the author (#2637, @MichaelChirico).
 
 ### Lint accuracy fixes: removing false negatives

R/expect_true_false_linter.R

@@ -32,10 +32,14 @@
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 expect_true_false_linter <- function() {
-  xpath <- "
-  following-sibling::expr[position() <= 2 and NUM_CONST[text() = 'TRUE' or text() = 'FALSE']]
+  pipe_cond <- glue("parent::expr/parent::expr[PIPE or SPECIAL[{xp_text_in_table(magrittr_pipes)}]]")
+  xpath <- glue("
+  following-sibling::expr[
+    NUM_CONST[text() = 'TRUE' or text() = 'FALSE']
+    and position() <= 2 - count({pipe_cond})
+  ]
     /parent::expr
-  "
+  ")
 
   Linter(linter_level = "expression", function(source_expression) {
     xml_calls <- source_expression$xml_find_function_calls(c("expect_equal", "expect_identical"))

tests/testthat/test-expect_true_false_linter.R

@@ -1,16 +1,22 @@
 test_that("expect_true_false_linter skips allowed usages", {
+  linter <- expect_true_false_linter()
   # expect_true is a scalar test; testing logical vectors with expect_equal is OK
-  expect_lint("expect_equal(x, c(TRUE, FALSE))", NULL, expect_true_false_linter())
+  expect_no_lint("expect_equal(x, c(TRUE, FALSE))", linter)
+
+  expect_no_lint("expect_equal(x, y, ignore_attr = TRUE)")
+
+  expect_no_lint("42 %>% expect_identical(42, ignore_attr = TRUE)", linter)
+  expect_no_lint("42 %>% expect_identical(42, TRUE)", linter)
+
+  skip_if_not_r_version("4.1.0")
+  expect_no_lint("42 |> expect_identical(42, ignore_attr = TRUE)", linter)
 })
 
 test_that("expect_true_false_linter blocks simple disallowed usages", {
   linter <- expect_true_false_linter()
+  lint_msg <- rex::rex("expect_true(x) is better than expect_equal(x, TRUE)")
 
-  expect_lint(
-    "expect_equal(foo(x), TRUE)",
-    rex::rex("expect_true(x) is better than expect_equal(x, TRUE)"),
-    linter
-  )
+  expect_lint("expect_equal(foo(x), TRUE)", lint_msg, linter)
 
   # expect_identical is treated the same as expect_equal
   expect_lint(
@@ -20,11 +26,12 @@ test_that("expect_true_false_linter blocks simple disallowed usages", {
   )
 
   # also caught when TRUE/FALSE is the first argument
-  expect_lint(
-    "expect_equal(TRUE, foo(x))",
-    rex::rex("expect_true(x) is better than expect_equal(x, TRUE)"),
-    linter
-  )
+  expect_lint("expect_equal(TRUE, foo(x))", lint_msg, linter)
+
+  expect_lint("42 %T>% expect_equal(TRUE)", lint_msg, linter)
+
+  skip_if_not_r_version("4.1.0")
+  expect_lint("42 |> expect_equal(TRUE)", lint_msg, linter)
 })
 
 test_that("lints vectorize", {