further fixes for new false positives in unnecessary_lambda_linter (#…

…2248)
r-lib · Oct 29, 2023 · a6e20bb · a6e20bb
1 parent 4a8b931
commit a6e20bb
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 2 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -99,7 +99,7 @@
   + finds assignments in call arguments besides the first one (#2136, @MichaelChirico).
   + finds assignments in parenthetical expressions like `if (A && (B <- foo(A))) { }` (#2138, @MichaelChirico).
 * `unnecessary_lambda_linter()` checks for cases using explicit returns, e.g. `lapply(x, \(xi) return(sum(xi)))` (#1567, @MichaelChirico).
-  + thanks to @Bisaloo for detecting a regression in the original fix (#2231).
+  + thanks to @Bisaloo and @strengejacke for detecting a regression in the original fix (#2231, #2247).
 
 # lintr 3.1.0
 

diff --git a/R/unnecessary_lambda_linter.R b/R/unnecessary_lambda_linter.R
@@ -77,7 +77,7 @@ unnecessary_lambda_linter <- function() {
           and preceding-sibling::expr/SYMBOL_FUNCTION_CALL
           and not(preceding-sibling::*[1][self::EQ_SUB])
         ]/SYMBOL
-      and not(OP-DOLLAR or OP-AT or OP-LEFT-BRACKET or LBB)
+      and count(OP-LEFT-PAREN) + count(OP-LEFT-BRACE/following-sibling::expr/OP-LEFT-PAREN) = 1
     ]
     /parent::expr
   ")

diff --git a/tests/testthat/test-unnecessary_lambda_linter.R b/tests/testthat/test-unnecessary_lambda_linter.R
@@ -49,6 +49,12 @@ test_that("unnecessary_lambda_linter skips allowed usages", {
   expect_lint('lapply(l, function(x) rle(x)["values"])', NULL, linter)
   expect_lint('lapply(l, function(x) rle(x)[["values"]])', NULL, linter)
   expect_lint("lapply(l, function(x) rle(x)@values)", NULL, linter)
+
+  # Other operators, #2247
+  expect_lint("lapply(l, function(x) foo(x) - 1)", NULL, linter)
+  expect_lint("lapply(l, function(x) foo(x) * 2)", NULL, linter)
+  expect_lint("lapply(l, function(x) foo(x) ^ 3)", NULL, linter)
+  expect_lint("lapply(l, function(x) foo(x) %% 4)", NULL, linter)
 })
 
 test_that("unnecessary_lambda_linter blocks simple disallowed usage", {
@@ -170,6 +176,10 @@ test_that("cases with braces are caught", {
     NULL,
     linter
   )
+
+  # false positives like #2231, #2247 are avoided with braces too
+  expect_lint("lapply(x, function(xi) { foo(xi)$bar })", NULL, linter)
+  expect_lint("lapply(x, function(xi) { foo(xi) - 1 })", NULL, linter)
 })
 
 test_that("function shorthand is handled", {