Fix false positive for null coalescing without else clause (#2938)

* Fix false positive for null coalescing without else clause

Signed-off-by: Emmanuel Ferdman <[email protected]>

* cite in NEWS

---------

Signed-off-by: Emmanuel Ferdman <[email protected]>
Co-authored-by: Michael Chirico <[email protected]>

Author: emmanuel-ferdman

NEWS.md

@@ -56,7 +56,7 @@
 * `download_file_linter()` encourages the use of `mode = "wb"` (or `mode = "ab"`) when using `download.file()`, rather than `mode = "w"` or `mode = "a"`, as the latter can produce broken
 files in Windows (#2882, @Bisaloo).
 * `lint2df_linter()` encourages the use of the `list2DF()` function, or the `data.frame()` function when recycling is required, over the slower and less readable `do.call(cbind.data.frame, )` alternative (#2834, @Bisaloo).
-* `coalesce_linter()` encourages the use of the infix operator `x %||% y`, which is equivalent to `if (is.null(x)) y else x` (#2246, @MichaelChirico). While this has long been used in many tidyverse packages (it was added to {ggplot2} in 2008), it became part of every R installation from R 4.4.0.
+* `coalesce_linter()` encourages the use of the infix operator `x %||% y`, which is equivalent to `if (is.null(x)) y else x` (#2246, @MichaelChirico). While this has long been used in many tidyverse packages (it was added to {ggplot2} in 2008), it became part of every R installation from R 4.4.0. Thanks also to @emmanuel-ferdman for fixing a false positive before release.
 
 ### Lint accuracy fixes: removing false positives
 

R/coalesce_linter.R

@@ -60,6 +60,7 @@ coalesce_linter <- function() {
   parent::expr[
     preceding-sibling::OP-EXCLAMATION
     and parent::expr/preceding-sibling::IF
+    and parent::expr/following-sibling::ELSE
     and (
       expr[2] = parent::expr/following-sibling::expr[1]
       or expr[2] = parent::expr/following-sibling::{braced_expr_cond}

tests/testthat/test-coalesce_linter.R

@@ -3,6 +3,9 @@ test_that("coalesce_linter skips allowed usage", {
 
   expect_no_lint("if (is.null(x)) y", linter)
   expect_no_lint("if (!is.null(x)) y", linter)
+  expect_no_lint("if (!is.null(x)) x", linter)
+  expect_no_lint("if (is.null(x)) x", linter)
+  expect_no_lint("c(if (!is.null(E)) E)", linter)
   expect_no_lint("if (is.null(x)) y else z", linter)
   expect_no_lint("if (!is.null(x)) x[1] else y", linter)
   expect_no_lint("if (is.null(x[1])) y else x[2]", linter)