Use stop(gettextf(.)) instead of stop(sprintf(.))

[MichaelChirico]

Follow-up split off from #575

[MichaelChirico]

I think this can be a more general translation_linter() that flags usages likely to cause issues for translations. For example any stop() call that uses implicit concatenation to build strings is likely to cause issues in translation. It's better to always use templates:

# bad: assumes English grammar / sentence structure
stop("Found ", n, " issues.")

# good: allows translator to decide word order by moving %d around
stop(gettextf("Found %d issues.", n))

# also good
stop(glue::glue("Found {n} issues."))

[IndrajeetPatil]

Good point.

condition_message_linter() does catch such usages with paste and suggests gettextf():

library(lintr)

lint(
  text = 'stop(paste("Found ", n, " issues."))',
  linters = condition_message_linter()
)
#> <text>:1:1: warning: [condition_message_linter] Don't use paste to build stop strings. Instead use the fact that these functions build condition message strings from their input (using "" as a separator). For translatable strings, prefer using gettextf().
#> stop(paste("Found ", n, " issues."))
#> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

^{Created on 2022-10-16 with reprex v2.0.2}

But stays silent with ...

library(lintr)

lint(
  text = 'stop("Found ", n, " issues.")',
  linters = condition_message_linter()
)

^{Created on 2022-10-16 with reprex v2.0.2}

Instead of introducing a new linter, should we just generalize condition_message_linter() to cover this case as well?

We can add a new parameter (say check_for_translation), which is by default set to TRUE (i.e., assumes that the user always wants to check if the way they are preparing a condition message will cause issues for translation).

[MichaelChirico]

Agree the purposes are pretty close, but think they should be kept separate.

Users that don't plan on translating their package would have no need for translation_linter() (other names could be portability_linter() or i18n_linter()), but still benefit from condition_message_linter() (which identifies overly-verbose calls to stop() and friends where the paste() part can be stripped out).

That applies all the more so to scripts/Rmd, which likely have little need to mark strings for translation.

translation_linter() can also grow to cover more use cases, e.g. cat("abc\n") could be caught and recommended as cat(gettext("abc\n")) to ensure the string is eligible for translation.

[MichaelChirico]

Another thing to lint for in translation_linter (or perhaps these should be individual linters with an i18n tag or similar):

stop(gettextf("%s", x))

stop() will attempt to translate the output of gettextf(), i.e., to re-translate the output, to avoid this the outer stop() call should get domain=NA:

stop(gettextf("%s", x), domain=NA)