Fix false positive in if_switch_linter() for empty strings

Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>

Author: emmanuel-ferdman

NEWS.md

@@ -11,6 +11,7 @@
 * Excluding `cyclocomp_linter()` in `available_linters()` or `linters_with_tags()`, which requires the weak dependency {cyclocomp}, no longer emits a warning (#2909, @MichaelChirico).
 * `repeat_linter()` no longer errors when `while` is in a column to the right of `}` (#2828, @MichaelChirico).
 * `undesirable_operator_linter(call_is_undesirable = FALSE)` now correctly skips prefix notation like `` `:::`(pkg, fun) `` (#2999, @emmanuel-ferdman).
+* `if_switch_linter()` no longer produces a false positive when comparing to empty strings (`""` or `''`), which cannot be used as `switch()` case names (#2835, @emmanuel-ferdman).
 
 ## New and improved features
 

R/if_switch_linter.R

@@ -159,7 +159,7 @@
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 if_switch_linter <- function(max_branch_lines = 0L, max_branch_expressions = 0L) {
-  equal_str_cond <- "expr[1][EQ and expr/STR_CONST]"
+  equal_str_cond <- "expr[1][EQ and expr/STR_CONST[string-length(text()) > 2]]"
 
   if (max_branch_lines > 0L || max_branch_expressions > 0L) {
     complexity_cond <- xp_or(c(
@@ -191,6 +191,10 @@ if_switch_linter <- function(max_branch_lines = 0L, max_branch_expressions = 0L)
   # NB: IF AND {...} AND ELSE/... implies >= 3 equality conditions are present
   # .//expr/IF/...: the expr in `==` that's _not_ the STR_CONST
   # not(preceding::IF): prevent nested matches which might be incorrect globally
+  empty_string_cond <- paste0(
+    ".//ELSE/following-sibling::expr/IF/following-sibling::expr[1][EQ]",
+    "/expr/STR_CONST[string-length(text()) = 2]"
+  )
   if_xpath <- glue("
   //IF
     /parent::expr[
@@ -202,6 +206,7 @@ if_switch_linter <- function(max_branch_lines = 0L, max_branch_expressions = 0L)
         and ELSE/following-sibling::expr[IF and {equal_str_cond}]
       ]
       and not({ max_lines_cond })
+      and not({ empty_string_cond })
     ]
   ")
 

tests/testthat/test-if_switch_linter.R

@@ -19,6 +19,13 @@ test_that("if_switch_linter skips allowed usages", {
   expect_no_lint("if (x == 'a') 1 else if (x == 'b') 2", linter)
   # still no third if() clause
   expect_no_lint("if (x == 'a') 1 else if (x == 'b') 2 else 3", linter)
+
+  # empty string comparisons can't use switch()
+  expect_no_lint("if (x == '') 1 else if (x == 'a') 2 else if (x == 'b') 3", linter)
+  expect_no_lint('if (x == "") 1 else if (x == "a") 2 else if (x == "b") 3', linter)
+  expect_no_lint("if (x == 'a') 1 else if (x == '') 2 else if (x == 'b') 3", linter)
+  expect_no_lint("if (x == 'a') 1 else if (x == 'b') 2 else if (x == '') 3", linter)
+  expect_no_lint("if (x == 'a') 1 else if (x == 'b') 2 else if (x == 'c') 3 else if (x == '') 4", linter)
 })
 
 test_that("if_switch_linter blocks simple disallowed usages", {