From a6e20bbb3e43e3551cd6324d194b518c35e19dde Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 29 Oct 2023 11:57:59 -0700 Subject: [PATCH] further fixes for new false positives in unnecessary_lambda_linter (#2248) --- NEWS.md | 2 +- R/unnecessary_lambda_linter.R | 2 +- tests/testthat/test-unnecessary_lambda_linter.R | 10 ++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 07073e65b..a4d533834 100644 --- 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 index 003caee0d..647ed4224 100644 --- 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 index 078f8f937..2c7e6cd59 100644 --- 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", {