First draft for in_linter()

DESCRIPTION

@@ -112,6 +112,7 @@ Collate:
     'ifelse_censor_linter.R'
     'implicit_assignment_linter.R'
     'implicit_integer_linter.R'
+    'in_linter.R'
     'indentation_linter.R'
     'infix_spaces_linter.R'
     'inner_combine_linter.R'

NAMESPACE

@@ -78,6 +78,7 @@ export(if_switch_linter)
 export(ifelse_censor_linter)
 export(implicit_assignment_linter)
 export(implicit_integer_linter)
+export(in_linter)
 export(indentation_linter)
 export(infix_spaces_linter)
 export(inner_combine_linter)

R/in_linter.R

@@ -0,0 +1,35 @@
+#' Chained equality check linter
+#'
+#' Report the use of chained equality checks where `%in%` would be more
+#' appropriate
+#'
+#' @examples
+#' # lints
+#' lint(
+#'   text = "x == 'a' | x == 'b'",
+#'   linters = in_linter()
+#' )
+#'
+#' # This only makes sense in the case x if of length 1
+#' lint(
+#'   text = "x == 'a' || x == 'b'",
+#'   linters = in_linter()
+#' )
+#'
+#' # okay
+#' lint(
+#'   text = "x %in% c('a', 'b')",
+#'   linters = in_linter()
+#' )
+#'
+#' @evalRd rd_tags("in_linter")
+#' @seealso [linters] for a complete list of linters available in lintr.
+#' @export
+in_linter <- make_linter_from_xpath(
+  xpath = "(//OR|//OR2)[
+    preceding-sibling::expr[EQ]
+    and following-sibling::expr[EQ]
+    and preceding-sibling::expr/expr/SYMBOL/text() = following-sibling::expr/expr/SYMBOL/text()
+  ]",
+  lint_message = "Use %in% to test equality of a variable against multiple values."
+)

inst/lintr/linters.csv

@@ -42,6 +42,7 @@ if_switch_linter,best_practices readability consistency efficiency configurable
 ifelse_censor_linter,best_practices efficiency
 implicit_assignment_linter,style best_practices readability configurable
 implicit_integer_linter,style consistency best_practices configurable
+in_linter,readability efficiency best_practices
 indentation_linter,style readability default configurable
 infix_spaces_linter,style readability default configurable
 inner_combine_linter,efficiency consistency readability

tests/testthat/test-in_linter.R

@@ -0,0 +1,54 @@
+test_that("in_linter catches unnecessary OR chains", {
+  linter <- in_linter()
+  lint_msg <- rex::rex("%in%")
+
+  expect_lint(
+    "x == 'a' | x == 'b'",
+    lint_msg,
+    linter
+  )
+
+  # Works with short-circuit operator
+  expect_lint(
+    "x == 'a' || x == 'b'",
+    lint_msg,
+    linter
+  )
+
+  # Works with yoda tests
+  expect_lint(
+    "'a' == x || 'b' == x",
+    lint_msg,
+    linter
+  )
+
+  # Works with 'semi-yoda' tests
+  expect_lint(
+    "x == 'a' || 'b' == x",
+    lint_msg,
+    linter
+  )
+
+  # Works with longer chains
+  expect_lint(
+    "x == 'a' | x == 'b' | x == 'c'",
+    lint_msg,
+    linter
+  )
+})
+
+test_that("common in_linter negative cases", {
+  linter <- in_linter()
+
+  expect_lint(
+    "x == 'a' | y == 'b'",
+    NULL,
+    linter
+  )
+
+  expect_lint(
+    "x == 'a' || y == 'b'",
+    NULL,
+    linter
+  )
+})