lint x %in% NA in equals_na_linter (#2112)

NEWS.md

@@ -34,6 +34,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.
+* `equals_na_linter()` checks for `x %in% NA`, which is a more convoluted form of `is.na(x)` (#2088, @MichaelChirico).
 * `commas_linter()` gains an option `allow_trailing` (default `FALSE`) to allow trailing commas while indexing. (#2104, @MEO265)
 
 ### New linters

R/equals_na_linter.R

@@ -1,6 +1,6 @@
 #' Equality check with NA linter
 #'
-#' Check for `x == NA` and `x != NA`. Such usage is almost surely incorrect --
+#' Check for `x == NA`, `x != NA` and `x %in% NA`. Such usage is almost surely incorrect --
 #' checks for missing values should be done with [is.na()].
 #'
 #' @examples
@@ -15,6 +15,11 @@
 #'   linters = equals_na_linter()
 #' )
 #'
+#' lint(
+#'   text = "x %in% NA",
+#'   linters = equals_na_linter()
+#' )
+#'
 #' # okay
 #' lint(
 #'   text = "is.na(x)",
@@ -36,6 +41,9 @@ equals_na_linter <- function() {
   //NUM_CONST[ {na_table} ]
     /parent::expr
     /parent::expr[EQ or NE]
+  |
+  //SPECIAL[text() = '%in%' and following-sibling::expr/NUM_CONST[ {na_table} ]]
+    /parent::expr
   ")
 
   Linter(function(source_expression) {
@@ -50,7 +58,7 @@ equals_na_linter <- function() {
     xml_nodes_to_lints(
       bad_expr,
       source_expression,
-      lint_message = "Use is.na for comparisons to NA (not == or !=)",
+      lint_message = "Use is.na for comparisons to NA (not == or != or %in%)",
       type = "warning"
     )
   })

tests/testthat/test-equals_na_linter.R

@@ -17,34 +17,42 @@ test_that("equals_na_linter skips allowed usages", {
 
   # nested NAs are okay
   expect_lint("x==f(1, ignore = NA)", NULL, linter)
+
+  # this should be covered by any_is_na_linter()
+  expect_lint("NA %in% x", NULL, linter)
 })
 
 skip_if_not_installed("tibble")
 patrick::with_parameters_test_that(
   "equals_na_linter blocks disallowed usages for all combinations of operators and types of NAs",
   expect_lint(
     paste("x", operation, type_na),
-    rex::rex("Use is.na for comparisons to NA (not == or !=)"),
+    rex::rex("Use is.na for comparisons to NA (not == or != or %in%)"),
     equals_na_linter()
   ),
   .cases = tibble::tribble(
-    ~.test_name,                ~operation, ~type_na,
-    "equality, logical NA",     "==",       "NA",
-    "equality, integer NA",     "==",       "NA_integer_",
-    "equality, real NA",        "==",       "NA_real_",
-    "equality, complex NA",     "==",       "NA_complex_",
-    "equality, character NA",   "==",       "NA_character_",
-    "inequality, logical NA",   "!=",       "NA",
-    "inequality, integer NA",   "!=",       "NA_integer_",
-    "inequality, real NA",      "!=",       "NA_real_",
-    "inequality, complex NA",   "!=",       "NA_complex_",
-    "inequality, character NA", "!=",       "NA_character_"
+    ~.test_name,                 ~operation, ~type_na,
+    "equality, logical NA",      "==",       "NA",
+    "equality, integer NA",      "==",       "NA_integer_",
+    "equality, real NA",         "==",       "NA_real_",
+    "equality, complex NA",      "==",       "NA_complex_",
+    "equality, character NA",    "==",       "NA_character_",
+    "containment, logical NA",   "%in%",     "NA",
+    "containment, integer NA",   "%in%",     "NA_integer_",
+    "containment, real NA",      "%in%",     "NA_real_",
+    "containment, complex NA",   "%in%",     "NA_complex_",
+    "containment, character NA", "%in%",     "NA_character_",
+    "inequality, logical NA",    "!=",       "NA",
+    "inequality, integer NA",    "!=",       "NA_integer_",
+    "inequality, real NA",       "!=",       "NA_real_",
+    "inequality, complex NA",    "!=",       "NA_complex_",
+    "inequality, character NA",  "!=",       "NA_character_"
   )
 )
 
 test_that("equals_na_linter blocks disallowed usages in edge cases", {
   linter <- equals_na_linter()
-  lint_msg <- rex::rex("Use is.na for comparisons to NA (not == or !=)")
+  lint_msg <- rex::rex("Use is.na for comparisons to NA (not == or != or %in%)")
 
   # missing spaces around operators
   expect_lint("x==NA", list(message = lint_msg, line_number = 1L, column_number = 1L), linter)