New redundant_equals_linter

DESCRIPTION

@@ -126,6 +126,7 @@ Collate:
     'path_linters.R'
     'pipe_call_linter.R'
     'pipe_continuation_linter.R'
+    'redundant_equals_linter.R'
     'redundant_ifelse_linter.R'
     'regex_subset_linter.R'
     'semicolon_linter.R'

NAMESPACE

@@ -92,6 +92,7 @@ export(paren_brace_linter)
 export(paste_linter)
 export(pipe_call_linter)
 export(pipe_continuation_linter)
+export(redundant_equals_linter)
 export(redundant_ifelse_linter)
 export(regex_subset_linter)
 export(sarif_output)

NEWS.md

@@ -22,6 +22,10 @@
 
 * `object_usage_linter()` improves lint metadata when detecting undefined infix operators, e.g. `%>%` or `:=` (#1497, @MichaelChirico)
 
+### New linters
+
+* `redundant_equals_linter()` for redundant comparisons to `TRUE` or `FALSE` like `is_treatment == TRUE` (#1500, @MichaelChirico)
+
 # lintr 3.0.1
 
 * Skip multi-byte tests in non UTF-8 locales (#1504)

R/redundant_equals_linter.R

@@ -0,0 +1,38 @@
+#' Block usage of `==`, `!=` on logical vectors
+#'
+#' Testing `x == TRUE` is redundant if `x` is a logical vector. Wherever this is
+#' used to improve readability, the solution should instead be to improve the
+#' naming of the object to better indicate that its contents are logical. This
+#' can be done using prefixes (is, has, can, etc.). For example, `is_child`,
+#' `has_parent_supervision`, `can_watch_horror_movie` clarify their logical
+#' nature, while `child`, `parent_supervision`, `watch_horror_movie` don't.
+#' @export
+redundant_equals_linter <- function() {
+  xpath <- paste0(
+    c("//EQ", "//NE"),
+    "/parent::expr/expr[NUM_CONST[text() = 'TRUE' or text() = 'FALSE']]/parent::expr",
+    collapse = " | "
+  )
+
+  Linter(function(source_expression) {
+    if (!is_lint_level(source_expression, "expression")) {
+      return(list())
+    }
+
+    xml <- source_expression$xml_parsed_content
+
+    bad_expr <- xml2::xml_find_all(xml, xpath)
+    op <- xml2::xml_text(xml2::xml_find_first(bad_expr, "*[2]"))
+
+    xml_nodes_to_lints(
+      bad_expr,
+      source_expression = source_expression,
+      lint_message = paste(
+        "Using", op, "on a logical vector is redundant.",
+        "Well-named logical vectors can be used directly in filtering.",
+        "For data.table's `i` argument, wrap the column name in (), like `DT[(is_treatment)]`."
+      ),
+      type = "warning"
+    )
+  })
+}

inst/lintr/linters.csv

@@ -53,6 +53,7 @@ paren_brace_linter,style readability deprecated
 paste_linter,best_practices consistency
 pipe_call_linter,style readability
 pipe_continuation_linter,style readability default
+redundant_equals_linter,best_practices readability efficiency common_mistakes
 redundant_ifelse_linter,best_practices efficiency consistency
 regex_subset_linter,best_practices efficiency
 semicolon_linter,style readability default configurable

tests/testthat/test-redundant_equals_linter.R

@@ -0,0 +1,39 @@
+test_that("redundant_equals_linter skips allowed usages", {
+  # comparisons to non-logical constants
+  expect_lint("x == 1", NULL, redundant_equals_linter())
+  # comparison to TRUE as a string
+  expect_lint("x != 'TRUE'", NULL, redundant_equals_linter())
+})
+
+test_that("mutliple lints return correct custom messages", {
+  expect_lint(
+    "list(x == TRUE, y != TRUE)",
+    list(
+      "Using == on a logical vector",
+      "Using != on a logical vector"
+    ),
+    redundant_equals_linter()
+  )
+})
+
+test_that("Order doesn't matter", {
+  expect_lint("TRUE == x", rex::rex("Using == on a logical vector is redundant."), redundant_equals_linter())
+})
+
+skip_if_not_installed("patrick")
+
+patrick::with_parameters_test_that(
+  "redundant_equals_linter blocks simple disallowed usages",
+  {
+    msg <- rex::rex(paste("Using", op, "on a logical vector is redundant."))
+    code <- paste("x", op, bool)
+    expect_lint(code, msg, redundant_equals_linter())
+  },
+  .cases = tibble::tribble(
+    ~.test_name, ~op, ~bool,
+    "==, TRUE", "==", "TRUE",
+    "==, FALSE", "==", "FALSE",
+    "!=, TRUE", "!=", "TRUE",
+    "!=, FALSE", "!=", "FALSE"
+  )
+)