sprintf_linter works for gettextf

NEWS.md

@@ -39,6 +39,8 @@
 * `paste_linter()` also catches usages like `paste(rep("*", 10L), collapse = "")` that can be written more
   concisely as `strrep("*", 10L)` (#1108, @MichaelChirico)
 
+* `sprintf_linter()` also applies to `gettextf()` (#1677, @MichaelChirico)
+
 ### New linters
 
 * `unnecessary_lambda_linter()`: detect unnecessary lambdas (anonymous functions), e.g.

R/sprintf_linter.R

@@ -1,7 +1,9 @@
 #' Require correct `sprintf()` calls
 #'
-#' Check for an inconsistent number of arguments or arguments with incompatible types
-#' (for literal arguments) in `sprintf()` calls.
+#' Check for an inconsistent number of arguments or arguments with incompatible types (for literal arguments) in
+#' [sprintf()] calls.
+#'
+#' [gettextf()] calls are also included, since `gettextf()` is a thin wrapper around `sprintf()`.
 #'
 #' @examples
 #' # will produce lints
@@ -26,7 +28,7 @@
 #' @export
 sprintf_linter <- function() {
   xpath <- "
-  //SYMBOL_FUNCTION_CALL[text() = 'sprintf']
+  //SYMBOL_FUNCTION_CALL[text() = 'sprintf' or text() = 'gettextf']
     /parent::expr
     /parent::expr[
       (

tests/testthat/test-sprintf_linter.R

@@ -1,67 +1,67 @@
-test_that("sprintf_linter skips allowed usages", {
-  linter <- sprintf_linter()
-
-  expect_lint("sprintf('hello')", NULL, linter)
-  expect_lint("sprintf('hello %d', 1)", NULL, linter)
-  expect_lint("sprintf('hello %d', x)", NULL, linter)
-  expect_lint("sprintf('hello %d', x + 1)", NULL, linter)
-  expect_lint("sprintf('hello %d', f(x))", NULL, linter)
-  expect_lint("sprintf('hello %1$s %1$s', x)", NULL, linter)
-  expect_lint("sprintf('hello %1$s %1$s %2$d', x, y)", NULL, linter)
-  expect_lint("sprintf('hello %1$s %1$s %2$d %3$s', x, y, 1.5)", NULL, linter)
-})
-
-test_that("sprintf_linter blocks disallowed usages", {
-  linter <- sprintf_linter()
-
-  expect_lint(
-    "sprintf('hello', 1)",
-    if (getRversion() >= "4.1.0") "one argument not used by format" else NULL,
-    linter
-  )
-
-  expect_lint(
-    "sprintf('hello %d', 'a')",
-    list(message = rex::rex("invalid format '%d'; use format %s for character objects")),
-    linter
-  )
-
-  expect_lint(
-    "sprintf('hello %d', 1.5)",
-    list(message = rex::rex("invalid format '%d'; use format %f, %e, %g or %a for numeric objects")),
-    linter
-  )
-
-  expect_lint(
-    "sprintf('hello %d',)",
-    list(message = rex::rex("argument is missing, with no default")),
-    linter
-  )
-
-  expect_lint(
-    "sprintf('hello %1$s %s', 'a', 'b')",
-    if (getRversion() >= "4.1.0") "one argument not used by format" else NULL,
-    linter
-  )
-
-  expect_lint(
-    "sprintf('hello %1$s %1$s', x, y)",
-    if (getRversion() >= "4.1.0") "one argument not used by format" else NULL,
-    linter
-  )
-
-  expect_lint(
-    "sprintf('hello %1$s %1$s %3$d', x, y)",
-    list(message = rex::rex("reference to non-existent argument 3")),
-    linter
-  )
-
-  expect_lint(
-    "sprintf('hello %1$s %1$s %2$d %3$d', x, y, 1.5)",
-    list(message = rex::rex("invalid format '%d'; use format %f, %e, %g or %a for numeric objects")),
-    linter
-  )
-})
+skip_if_not_installed("patrick")
+patrick::with_parameters_test_that(
+  "sprintf_linter skips allowed usages",
+  {
+    linter <- sprintf_linter()
+
+    # NB: using paste0, not sprintf, to avoid escaping '%d' in sprint fmt=
+    expect_lint(paste0(call_name, "('hello')"), NULL, linter)
+    expect_lint(paste0(call_name, "('hello %d', 1)"), NULL, linter)
+    expect_lint(paste0(call_name, "('hello %d', x)"), NULL, linter)
+    expect_lint(paste0(call_name, "('hello %d', x + 1)"), NULL, linter)
+    expect_lint(paste0(call_name, "('hello %d', f(x))"), NULL, linter)
+    expect_lint(paste0(call_name, "('hello %1$s %1$s', x)"), NULL, linter)
+    expect_lint(paste0(call_name, "('hello %1$s %1$s %2$d', x, y)"), NULL, linter)
+    expect_lint(paste0(call_name, "('hello %1$s %1$s %2$d %3$s', x, y, 1.5)"), NULL, linter)
+  },
+  .test_name = c("sprintf", "gettextf"),
+  call_name = c("sprintf", "gettextf")
+)
+
+patrick::with_parameters_test_that(
+  "sprintf_linter blocks disallowed usages",
+  {
+    linter <- sprintf_linter()
+    unused_arg_msg <- if (getRversion() >= "4.1.0") "one argument not used by format" else NULL
+
+    expect_lint(paste0(call_name, "('hello', 1)"), unused_arg_msg, linter)
+
+    expect_lint(
+      paste0(call_name, "('hello %d', 'a')"),
+      rex::rex("invalid format '%d'; use format %s for character objects"),
+      linter
+    )
+
+    expect_lint(
+      paste0(call_name, "('hello %d', 1.5)"),
+      rex::rex("invalid format '%d'; use format %f, %e, %g or %a for numeric objects"),
+      linter
+    )
+
+    expect_lint(
+      paste0(call_name, "('hello %d',)"),
+      rex::rex("argument is missing, with no default"),
+      linter
+    )
+
+    expect_lint(paste0(call_name, "('hello %1$s %s', 'a', 'b')"), unused_arg_msg, linter)
+    expect_lint(paste0(call_name, "('hello %1$s %1$s', x, y)"), unused_arg_msg, linter)
+
+    expect_lint(
+      paste0(call_name, "('hello %1$s %1$s %3$d', x, y)"),
+      rex::rex("reference to non-existent argument 3"),
+      linter
+    )
+
+    expect_lint(
+      paste0(call_name, "('hello %1$s %1$s %2$d %3$d', x, y, 1.5)"),
+      rex::rex("invalid format '%d'; use format %f, %e, %g or %a for numeric objects"),
+      linter
+    )
+  },
+  .test_name = c("sprintf", "gettextf"),
+  call_name = c("sprintf", "gettextf")
+)
 
 test_that("edge cases are detected correctly", {
   linter <- sprintf_linter()
@@ -81,7 +81,7 @@ test_that("edge cases are detected correctly", {
   # dots
   expect_lint("sprintf('%d %d, %d', id, ...)", NULL, linter)
 
-  # TODO (@AshesITR) extend ... detection to at least test for too many arguments.
+  # TODO(#1265) extend ... detection to at least test for too many arguments.
 
   # named argument fmt
   expect_lint("sprintf(x, fmt = 'hello %1$s %1$s')", NULL, linter)