Allow to opt out of linting filter() in conjunct_test_linter

NEWS.md

@@ -33,6 +33,7 @@
 * `seq_linter()` recommends `rev()` in the lint message for lints like `nrow(x):1` (#1542, @MichaelChirico).
 * New `xp_call_name()` helper to facilitate writing custom linters (#2023, @MichaelChirico). This helper converts a matched XPath to the R function to which it corresponds. This is useful for including the "offending" function in the lint's message.
 * `function_argument_linter()` detects usage of `missing()` for the linted argument (#1546, @MichaelChirico). The simplest fix for `function_argument_linter()` lints is typically to set that argument to `NULL` by default, in which case it's usually preferable to update function logic checking `missing()` to check `is.null()` instead.
+* Allow to opt out of linting `filter()` expressions in `conjunct_test_linter` via new `allow_filter` argument which defaults to `FALSE` (#2110, @salim-b)
 
 ### New linters
 

R/conjunct_test_linter.R

@@ -7,13 +7,14 @@
 #'
 #' Similar reasoning applies to `&&` usage inside [stopifnot()] and `assertthat::assert_that()` calls.
 #'
-#' Relatedly, `dplyr::filter(DF, A & B)` is the same as `dplyr::filter(DF, A, B)`, but the
-#'   latter will be more readable / easier to format for long conditions. Note that this linter
-#'   assumes usages of `filter()` are `dplyr::filter()`; if you're using another function named `filter()`,
-#'   e.g. [stats::filter()], please namespace-qualify it to avoid false positives.
+#' Relatedly, `dplyr::filter(DF, A & B)` is the same as `dplyr::filter(DF, A, B)`, but the latter will be more readable
+#'   / easier to format for long conditions. Note that this linter assumes usages of `filter()` are `dplyr::filter()`;
+#'   if you're using another function named `filter()`, e.g. [stats::filter()], please namespace-qualify it to avoid
+#'   false positives. You can omit linting `filter()` expressions altogether via `allow_filter = TRUE`.
 #'
 #' @param allow_named_stopifnot Logical, `TRUE` by default. If `FALSE`, "named" calls to `stopifnot()`,
 #'   available since R 4.0.0 to provide helpful messages for test failures, are also linted.
+#' @param allow_filter Logical, `FALSE` by default. If `TRUE`, `filter()` expressions are not linted.
 #'
 #' @examples
 #' # will produce lints
@@ -32,6 +33,14 @@
 #'   linters = conjunct_test_linter(allow_named_stopifnot = FALSE)
 #' )
 #'
+#' lint(
+#'   text = "dplyr::filter(
+#'     mtcars,
+#'     mpg > 20 & vs == 0
+#'   )",
+#'   linters = conjunct_test_linter()
+#' )
+#'
 #' # okay
 #' lint(
 #'   text = "expect_true(x || (y && z))",
@@ -43,10 +52,19 @@
 #'   linters = conjunct_test_linter(allow_named_stopifnot = TRUE)
 #' )
 #'
+#' lint(
+#'   text = "dplyr::filter(
+#'     mtcars,
+#'     mpg > 20 & vs == 0
+#'   )",
+#'   linters = conjunct_test_linter(allow_filter = TRUE)
+#' )
+#'
 #' @evalRd rd_tags("conjunct_test_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
-conjunct_test_linter <- function(allow_named_stopifnot = TRUE) {
+conjunct_test_linter <- function(allow_named_stopifnot = TRUE,
+                                 allow_filter = FALSE) {
   expect_true_assert_that_xpath <- "
   //SYMBOL_FUNCTION_CALL[text() = 'expect_true' or text() = 'assert_that']
     /parent::expr
@@ -103,22 +121,26 @@ conjunct_test_linter <- function(allow_named_stopifnot = TRUE) {
       sprintf(as.character(replacement_fmt), matched_fun),
       "The latter will produce better error messages in the case of failure."
     )
-    test_lints <- xml_nodes_to_lints(
+    lints <- xml_nodes_to_lints(
       test_expr,
       source_expression = source_expression,
       lint_message = lint_message,
       type = "warning"
     )
 
-    filter_expr <- xml_find_all(xml, filter_xpath)
+    if (!allow_filter) {
+      filter_expr <- xml_find_all(xml, filter_xpath)
 
-    filter_lints <- xml_nodes_to_lints(
-      filter_expr,
-      source_expression = source_expression,
-      lint_message = "Use dplyr::filter(DF, A, B) instead of dplyr::filter(DF, A & B).",
-      type = "warning"
-    )
+      filter_lints <- xml_nodes_to_lints(
+        filter_expr,
+        source_expression = source_expression,
+        lint_message = "Use dplyr::filter(DF, A, B) instead of dplyr::filter(DF, A & B).",
+        type = "warning"
+      )
+
+      lints <- c(lints, filter_lints)
+    }
 
-    c(test_lints, filter_lints)
+    lints
   })
 }

tests/testthat/test-conjunct_test_linter.R

@@ -131,6 +131,14 @@ test_that("conjunct_test_linter blocks simple disallowed usages", {
   expect_lint("DF %>% dplyr::filter(A & B)", lint_msg, linter)
 })
 
+test_that("conjunct_test_linter respects its allow_filter argument", {
+  linter <- conjunct_test_linter(allow_filter = TRUE)
+
+  expect_lint("dplyr::filter(DF, A & B)", NULL, linter)
+  expect_lint("dplyr::filter(DF, A & B & C)", NULL, linter)
+  expect_lint("DF %>% dplyr::filter(A & B)", NULL, linter)
+})
+
 test_that("filter() is assumed to be dplyr::filter() by default, unless o/w specified", {
   linter <- conjunct_test_linter()