Lint message cites which argument is missing

R/missing_argument_linter.R

@@ -43,19 +43,28 @@ missing_argument_linter <- function(except = c("alist", "quote", "switch"), allo
   }
 
   # require >3 children to exclude foo(), which is <expr><OP-LEFT-PAREN><OP-RIGHT-PAREN>
-  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)"
+  xpath <- glue("
+    //SYMBOL_FUNCTION_CALL[not({ xp_text_in_table(except) })]
+      /parent::expr
+      /parent::expr[count(*) > 3]
+      /*[{xp_or(conds)}]
+  ")
 
   Linter(linter_level = "file", function(source_expression) {
     xml <- source_expression$full_xml_parsed_content
 
     missing_args <- xml_find_all(xml, xpath)
-    function_call_name <- get_r_string(xml_find_chr(missing_args, to_function_xpath))
+
+    named_idx <- xml_name(missing_args) == "EQ_SUB"
+    arg_id <- character(length(missing_args))
+    arg_id[named_idx] <- sQuote(xml_find_chr(missing_args[named_idx], "string(preceding-sibling::SYMBOL_SUB[1])"), "'")
+    # TODO(r-lib/xml2#412-->CRAN): use xml_find_int() instead
+    arg_id[!named_idx] <- xml_find_num(missing_args[!named_idx], "count(preceding-sibling::OP-COMMA)") + 1.0
 
     xml_nodes_to_lints(
-      missing_args[!function_call_name %in% except],
+      missing_args,
       source_expression = source_expression,
-      lint_message = "Missing argument in function call."
+      lint_message = sprintf("Missing argument %s in function call.", arg_id)
     )
   })
 }

tests/testthat/test-missing_argument_linter.R

@@ -18,14 +18,21 @@ test_that("missing_argument_linter skips allowed usages", {
 
 test_that("missing_argument_linter blocks disallowed usages", {
   linter <- missing_argument_linter()
-  lint_msg <- rex::rex("Missing argument in function call.")
+  lint_msg1 <- rex::rex("Missing argument 1 in function call.")
+  lint_msg2 <- rex::rex("Missing argument 2 in function call.")
+  lint_msg3 <- rex::rex("Missing argument 3 in function call.")
+  lint_msga <- rex::rex("Missing argument 'a' in function call.")
 
-  expect_lint("fun(, a = 1)", list(message = lint_msg), linter)
-  expect_lint("f <- function(x, y) x\nf(, y = 1)\n", list(line = "f(, y = 1)"), linter)
-  expect_lint("fun(a = 1,, b = 2)", list(message = lint_msg), linter)
-  expect_lint("fun(a = 1, b =)", list(message = lint_msg), linter)
-  expect_lint("fun(a = 1,)", list(message = lint_msg), linter)
-  expect_lint("fun(a = )", list(message = lint_msg), linter)
+  expect_lint("fun(, a = 1)", lint_msg1, linter)
+  expect_lint(
+    "f <- function(x, y) x\nf(, y = 1)\n",
+    list(lint_msg1, line = "f(, y = 1)"),
+    linter
+  )
+  expect_lint("fun(a = 1,, b = 2)", lint_msg2, linter)
+  expect_lint("fun(b = 1, a =)", lint_msga, linter)
+  expect_lint("fun(a = 1,)", lint_msg2, linter)
+  expect_lint("fun(a = )", lint_msga, linter)
 
   expect_lint(
     trim_some("
@@ -34,35 +41,12 @@ test_that("missing_argument_linter blocks disallowed usages", {
         b = 2,
       )
     "),
-    list(message = lint_msg),
+    lint_msg3,
     linter
   )
 
-  expect_lint("stats::median(1:10, na.rm =)", list(message = lint_msg), linter)
-  expect_lint("env$get(1:10, default =)", list(message = lint_msg), linter)
-
-  # except list can be empty
-  expect_lint("switch(a =, b = 1, 0)", list(message = lint_msg), missing_argument_linter(character()))
-  expect_lint("alist(a =)", list(message = lint_msg), missing_argument_linter(character()))
-
-  # allow_trailing can allow trailing empty args also for non-excepted functions
-  expect_lint("fun(a = 1,)", NULL, missing_argument_linter(allow_trailing = TRUE))
-  expect_lint(
-    trim_some("
-      fun(
-        a = 1,
-        # comment
-      )
-    "),
-    NULL,
-    missing_argument_linter(allow_trailing = TRUE)
-  )
-  # ... but not if the final argument is named
-  expect_lint(
-    "fun(a = 1, b = )",
-    list(message = lint_msg),
-    missing_argument_linter(allow_trailing = TRUE)
-  )
+  expect_lint("stats::median(1:10, a =)", lint_msga, linter)
+  expect_lint("env$get(1:10, a =)", lint_msga, linter)
 
   # Fixes https://github.com/r-lib/lintr/issues/906
   # Comments should be ignored so that missing arguments could be
@@ -75,7 +59,7 @@ test_that("missing_argument_linter blocks disallowed usages", {
         # comment
       )
     "),
-    list(message = lint_msg),
+    lint_msg3,
     linter
   )
 
@@ -87,7 +71,7 @@ test_that("missing_argument_linter blocks disallowed usages", {
         1
       )
     "),
-    list(message = lint_msg),
+    lint_msg1,
     linter
   )
 
@@ -99,7 +83,37 @@ test_that("missing_argument_linter blocks disallowed usages", {
         1
       )
     "),
-    list(message = lint_msg),
+    lint_msga,
+    linter
+  )
+})
+
+test_that("except list can be empty", {
+  linter <- missing_argument_linter(character())
+  lint_msg <- rex::rex("Missing argument 'a' in function call.")
+
+  expect_lint("switch(a =, b = 1, 0)", lint_msg, linter)
+  expect_lint("alist(a =)", lint_msg, linter)
+})
+
+test_that("allow_trailing can allow trailing empty args also for non-excepted functions", {
+  linter <- missing_argument_linter(allow_trailing = TRUE)
+
+  expect_lint("fun(a = 1,)", NULL, linter)
+  expect_lint(
+    trim_some("
+      fun(
+        a = 1,
+        # comment
+      )
+    "),
+    NULL,
+    linter
+  )
+  # ... but not if the final argument is named
+  expect_lint(
+    "fun(a = 1, b = )",
+    rex::rex("Missing argument 'b' in function call."),
     linter
   )
 })
@@ -112,17 +126,35 @@ test_that("lints vectorize", {
   expect_lint(
     "foo(,,)",
     list(
-      list(lint_msg, column_number = 5L),
-      list(lint_msg, column_number = 6L),
-      list(lint_msg, column_number = 7L)
+      list("Missing argument 1", column_number = 5L),
+      list("Missing argument 2", column_number = 6L),
+      list("Missing argument 3", column_number = 7L)
     ),
     linter
   )
   expect_lint(
     "foo(,,)",
     list(
-      list(lint_msg, column_number = 5L),
-      list(lint_msg, column_number = 6L)
+      list("Missing argument 1", column_number = 5L),
+      list("Missing argument 2", column_number = 6L)
+    ),
+    linter_trailing
+  )
+
+  expect_lint(
+    "foo(a =,,)",
+    list(
+      list("Missing argument 'a'", column_number = 7L),
+      list("Missing argument 2", column_number = 9L),
+      list("Missing argument 3", column_number = 10L)
+    ),
+    linter
+  )
+  expect_lint(
+    "foo(a =,,)",
+    list(
+      list("Missing argument 'a'", column_number = 7L),
+      list("Missing argument 2", column_number = 9L)
     ),
     linter_trailing
   )
@@ -133,8 +165,8 @@ test_that("lints vectorize", {
       bar(,2)
     }"),
     list(
-      list(lint_msg, line_number = 2L),
-      list(lint_msg, line_number = 3L)
+      list("Missing argument 2", line_number = 2L),
+      list("Missing argument 1", line_number = 3L)
     ),
     linter
   )
@@ -143,7 +175,7 @@ test_that("lints vectorize", {
       foo(1,)
       bar(,2)
     }"),
-    list(lint_msg, line_number = 3L),
+    list("Missing argument 1", line_number = 3L),
     linter_trailing
   )
 })