From 28dfdec7f3731b0cdc4d6aa62ac300fc006c138b Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 11 Oct 2022 20:28:22 +0000
Subject: [PATCH 1/4] initial progress

---
 R/sprintf_linter.R | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/R/sprintf_linter.R b/R/sprintf_linter.R
index 9645c4bb6..ad8d2348e 100644
--- a/R/sprintf_linter.R
+++ b/R/sprintf_linter.R
@@ -1,14 +1,16 @@
 #' Require correct `sprintf()` calls
 #'
 #' Check for an inconsistent number of arguments or arguments with incompatible types (for literal arguments) in
-#' `sprintf()` calls.
+#' [sprintf()] calls.
+#'
+#' [gettextf()] calls are also included, since `gettextf()` is a thin wrapper around `sprintf()`.
 #'
 #' @evalRd rd_tags("sprintf_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 sprintf_linter <- function() {
   xpath <- "
-  //SYMBOL_FUNCTION_CALL[text() = 'sprintf']
+  //SYMBOL_FUNCTION_CALL[text() = 'sprintf' or text() = 'gettextf']
     /parent::expr
     /parent::expr[
       (

From 991f6121a3e14b411928ed601e20b9e0e816e9ab Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 11 Oct 2022 20:47:04 +0000
Subject: [PATCH 2/4] update tests

---
 tests/testthat/test-sprintf_linter.R | 129 +++++++++++++--------------
 1 file changed, 64 insertions(+), 65 deletions(-)

diff --git a/tests/testthat/test-sprintf_linter.R b/tests/testthat/test-sprintf_linter.R
index 8663345ab..db68a865a 100644
--- a/tests/testthat/test-sprintf_linter.R
+++ b/tests/testthat/test-sprintf_linter.R
@@ -1,67 +1,66 @@
-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()
+
+    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 +80,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)

From b38ee5932fdb29c4a7ce3e7d226aaaf430ba8615 Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 11 Oct 2022 20:47:49 +0000
Subject: [PATCH 3/4] news

---
 NEWS.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/NEWS.md b/NEWS.md
index 7ad4dd334..9b984a2e8 100644
--- a/NEWS.md
+++ b/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.

From 4ab6b676184118d0907d46b693e317c24295d21b Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 11 Oct 2022 20:49:11 +0000
Subject: [PATCH 4/4] note on paste0 usage

---
 tests/testthat/test-sprintf_linter.R | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/testthat/test-sprintf_linter.R b/tests/testthat/test-sprintf_linter.R
index db68a865a..7fda7d1f3 100644
--- a/tests/testthat/test-sprintf_linter.R
+++ b/tests/testthat/test-sprintf_linter.R
@@ -4,6 +4,7 @@ patrick::with_parameters_test_that(
   {
     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)