diff --git a/NEWS.md b/NEWS.md index 1424a10bb..731a8a9d9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -69,6 +69,10 @@ + ignores calls on the RHS of operators like `lapply(l, function(x) "a" %in% names(x))` (#2310, @MichaelChirico). * `vector_logic_linter()` recognizes some cases where bitwise `&`/`|` are used correctly (#1453, @MichaelChirico). +### Lint accuracy fixes: removing false negatives + +* `missing_argument_linter()` catches all missing arguments in calls with several, e.g. `foo(,,)` gives 3 lints instead of 2 (#2399, @MichaelChirico). + # lintr 3.1.1 ## Breaking changes diff --git a/R/missing_argument_linter.R b/R/missing_argument_linter.R index d549169dd..9c23ffcb0 100644 --- a/R/missing_argument_linter.R +++ b/R/missing_argument_linter.R @@ -37,10 +37,13 @@ missing_argument_linter <- function(except = c("alist", "quote", "switch"), allo "self::EQ_SUB[following-sibling::*[not(self::COMMENT)][1][self::OP-RIGHT-PAREN or self::OP-COMMA]]" ) if (!allow_trailing) { - conds <- c(conds, "self::OP-COMMA[following-sibling::*[not(self::COMMENT)][1][self::OP-RIGHT-PAREN]]") + conds <- c(conds, + "self::OP-RIGHT-PAREN[preceding-sibling::*[not(self::COMMENT)][1][self::OP-LEFT-PAREN or self::OP-COMMA]]" + ) } - xpath <- glue("//SYMBOL_FUNCTION_CALL/parent::expr/parent::expr/*[{xp_or(conds)}]") + # require >3 children to exclude foo(), which is + xpath <- glue("//SYMBOL_FUNCTION_CALL/parent::expr/parent::expr[count(*) > 3]/*[{xp_or(conds)}]") to_function_xpath <- "string(./preceding-sibling::expr[last()]/SYMBOL_FUNCTION_CALL)" Linter(linter_level = "file", function(source_expression) { diff --git a/tests/testthat/test-missing_argument_linter.R b/tests/testthat/test-missing_argument_linter.R index 48dba5093..06e9e1f91 100644 --- a/tests/testthat/test-missing_argument_linter.R +++ b/tests/testthat/test-missing_argument_linter.R @@ -10,6 +10,9 @@ test_that("missing_argument_linter skips allowed usages", { expect_lint("alist(a =, b =, c = 1, 0)", NULL, linter) expect_lint("pairlist(path = quote(expr = ))", NULL, linter) #1889 + # always allow this missing usage + expect_lint("foo()", NULL, linter) + expect_lint("test(a =, b =, c = 1, 0)", NULL, missing_argument_linter("test")) }) @@ -100,3 +103,47 @@ test_that("missing_argument_linter blocks disallowed usages", { linter ) }) + +test_that("lints vectorize", { + linter <- missing_argument_linter() + linter_trailing <- missing_argument_linter(allow_trailing = TRUE) + lint_msg <- rex::rex("Missing argument in function call.") + + expect_lint( + "foo(,,)", + list( + list(lint_msg, column_number = 5L), + list(lint_msg, column_number = 6L), + list(lint_msg, column_number = 7L) + ), + linter + ) + expect_lint( + "foo(,,)", + list( + list(lint_msg, column_number = 5L), + list(lint_msg, column_number = 6L) + ), + linter_trailing + ) + + expect_lint( + trim_some("{ + foo(1,) + bar(,2) + }"), + list( + list(lint_msg, line_number = 2L), + list(lint_msg, line_number = 3L) + ), + linter + ) + expect_lint( + trim_some("{ + foo(1,) + bar(,2) + }"), + list(lint_msg, line_number = 3L), + linter_trailing + ) +})