[code-quality] Use all_linters(), apply

MichaelChirico · MichaelChirico · commit 872277d20578 · 2025-12-14T23:55:14.000-08:00
diff --git a/.lintr b/.lintr
@@ -1,32 +1,37 @@
-linters: linters_with_defaults(
+linters: all_linters(
     fixed_regex_linter(),
     function_argument_linter(),
+    implicit_assignment_linter(allow_lazy = TRUE, allow_scoped = TRUE),
     implicit_integer_linter(allow_colon = TRUE),
     infix_spaces_linter(exclude_operators = c("=", "*", "/")),
-    # TODO(michaelchirico): Enable once quotes used for parallelism are supported
-    # keyword_quote_linter(),
     line_length_linter(120L),
     string_boundary_linter(),
-    undesirable_function_linter(c(
-      sapply = NA
-    )),
+    # undesirable_function_linter(modify_defaults(
+    #   default_undesirable_functions,
+    #   sapply = NA
+    # )),
     # TODO(michaelchirico): Enable after #2245
     # unnecessary_nested_if_linter(),
     assignment_linter = NULL,
     # TODO(michaelchirico): reactivate this and spaces_inside_linter()
     #   once they support 'empty' i argument DT[ , j]
     commas_linter = NULL,
     commented_code_linter = NULL,
-    cyclocomp_linter = NULL,
+    cyclocomp_linter = NULL, # TODO(michaelchirico): fix translate_package().
     # TODO(michaelchirico): reactivate this. far too many
     #   false positives for now.
     indentation_linter = NULL,
+    # TODO(michaelchirico): Enable once quotes used for parallelism are supported
+    keyword_quote_linter = NULL,
+    nonportable_path_linter = NULL,
     object_name_linter = NULL,
     quotes_linter = NULL, # TODO(michaelchirico): switch to "'",
-    spaces_inside_linter = NULL
+    spaces_inside_linter = NULL,
+    todo_comment_linter = NULL # TODO(michaelchirico): [hehe] activate this & assign TODOs bugs
   )
 exclusions: list(
     "inst/pkg",
+    "tests/testthat.R" = list(undesirable_function_linter = Inf),
     "tests/testthat" = list(object_usage_linter = Inf),
     "tests/testthat/test_packages",
     "vignettes" = list(
diff --git a/R/check_cracked_messages.R b/R/check_cracked_messages.R
@@ -73,15 +73,15 @@ attr(check_cracked_messages, "diagnostic_tag") =
 # take a call like stop("a", i, "b", j) and suggest
 #   stop(domain=NA, gettextf("a%sb%s", i, j))
 build_gettextf_call = function(e) {
-  call <- as.character(e[[1L]])
+  call_str <- as.character(e[[1L]])
   rest <- e[-1L]
   arg_names <- names(rest)
   if (is.null(arg_names) || all(keep_args <- arg_names %chin% c("", "domain"))) {
-    as.character(glue('{call}(domain=NA, {gettextify(rest)})'))
+    as.character(glue('{call_str}(domain=NA, {gettextify(rest)})'))
   } else {
     # if other arguments, e.g. call., appendLF, immediate. are present, keep them with the right call (#227)
     kwargs = toString(paste(names(rest[!keep_args]), "=", vapply(rest[!keep_args], deparse1, character(1L))))
-    as.character(glue('{call}(domain=NA, {gettextify(rest[keep_args])}, {kwargs})'))
+    as.character(glue('{call_str}(domain=NA, {gettextify(rest[keep_args])}, {kwargs})'))
   }
 }
 
diff --git a/R/find_fuzzy_messages.R b/R/find_fuzzy_messages.R
@@ -6,7 +6,7 @@ find_fuzzy_messages <- function(message_data, lang_file) {
     message('Typically, this means the corresponding error messages have been refactored.')
     message('Reproducing these messages here for your reference since they might still provide some utility.')
 
-    dashes = strrep('-', .9*getOption('width'))
+    dashes = strrep('-', 0.9*getOption('width'))
     old_message_data[idx & type == 'singular', {
       if (.N > 0L) {
         message(' ** SINGULAR MESSAGES **')
diff --git a/R/get_message_data.R b/R/get_message_data.R
@@ -95,6 +95,7 @@ get_message_data = function(
   if (verbose && dir.exists(file.path(dir, "R"))) message('Getting R-level messages...')
   r_message_data = get_r_messages(
     dir,
+    # nolint next: backport_linter. False positive on 'R'.
     custom_translation_functions = custom_translation_functions$R,
     style = style,
     is_base = is_base
diff --git a/R/get_po_messages.R b/R/get_po_messages.R
@@ -84,38 +84,38 @@ get_po_messages <- function(po_file) {
 
   msg_j = 1L
   while (msg_j <= length(msgid_start)) {
-    start = msgid_start[msg_j]
-    end = find_msg_end(start)
-    set(po_data, msg_j, 'msgid', build_msg(start, end, 'msgid'))
+    start_j = msgid_start[msg_j]
+    end_j = find_msg_end(start_j)
+    set(po_data, msg_j, 'msgid', build_msg(start_j, end_j, 'msgid'))
 
-    set(po_data, msg_j, 'fuzzy', as.integer(start != 1L && startsWith(po_lines[start - 1L], "#, fuzzy")))
+    set(po_data, msg_j, 'fuzzy', as.integer(start_j != 1L && startsWith(po_lines[start_j - 1L], "#, fuzzy")))
 
-    start = end + 1L
-    end = find_msg_end(start)
-    set(po_data, msg_j, 'msgstr', build_msg(start, end, 'msgstr'))
+    start_j = end_j + 1L
+    end_j = find_msg_end(start_j)
+    set(po_data, msg_j, 'msgstr', build_msg(start_j, end_j, 'msgstr'))
     msg_j = msg_j + 1L
   }
 
   plural_i = 1L
   while (plural_i <= length(msgid_plural_start)) {
-    start = msgid_plural_start[plural_i]
-    end = find_msg_end(start)
-    msg1 = build_msg(start, end, 'msgid')
+    start_j = msgid_plural_start[plural_i]
+    end_j = find_msg_end(start_j)
+    msg1 = build_msg(start_j, end_j, 'msgid')
 
-    set(po_data, msg_j, 'fuzzy', as.integer(start != 1L && startsWith(po_lines[start - 1L], "#, fuzzy")))
+    set(po_data, msg_j, 'fuzzy', as.integer(start_j != 1L && startsWith(po_lines[start_j - 1L], "#, fuzzy")))
 
-    start = end + 1L
-    end = find_msg_end(start)
-    msg2 = build_msg(start, end, 'msgid_plural')
+    start_j = end_j + 1L
+    end_j = find_msg_end(start_j)
+    msg2 = build_msg(start_j, end_j, 'msgid_plural')
 
     set(po_data, msg_j, 'msgid_plural', list(c(msg1, msg2)))
 
-    start = end + 1L
+    start_j = end_j + 1L
     msgstr_plural = character()
-    while (start <= po_length && startsWith(po_lines[start], 'msgstr[')) {
-      end = find_msg_end(start)
-      msgstr_plural = c(msgstr_plural, build_msg(start, end, 'msgstr\\[\\d+\\]'))
-      start = end + 1L
+    while (start_j <= po_length && startsWith(po_lines[start_j], 'msgstr[')) {
+      end_j = find_msg_end(start_j)
+      msgstr_plural = c(msgstr_plural, build_msg(start_j, end_j, 'msgstr\\[\\d+\\]'))
+      start_j = end_j + 1L
     }
     set(po_data, msg_j, 'msgstr_plural', list(msgstr_plural))
 
@@ -125,11 +125,9 @@ get_po_messages <- function(po_file) {
 
   # somewhat hacky approach -- strip the comment markers & recurse;
   #   retain the same file pattern to ensure 'message_source' is correct
-  writeLines(
-    gsub("^#~ ", "", grep("^#~ ", po_lines, value = TRUE)),
-    tmp <- tempfile(pattern = basename(po_file))
-  )
+  tmp <- tempfile(pattern = basename(po_file))
   on.exit(unlink(tmp))
+  writeLines(gsub("^#~ ", "", grep("^#~ ", po_lines, value = TRUE)), tmp)
   deprecated = get_po_messages(tmp)
   if (nrow(deprecated) > 0L) {
     set(deprecated, NULL, 'fuzzy', 2L)
diff --git a/R/get_r_messages.R b/R/get_r_messages.R
@@ -219,11 +219,11 @@ parse_r_keywords = function(spec) {
   named_idx = grepl("^[a-zA-Z0-9._]+\\|[0-9]+$", keyval$V2)
   plural_idx = grepl("^[a-zA-Z0-9._]+\\|[0-9]+,[a-zA-Z0-9._]+\\|[0-9]+$", keyval$V2)
   dots_idx = grepl("^[.]{3}[\\](?:[a-zA-Z0-9._]+,)*[a-zA-Z0-9._]+$", keyval$V2)
-  if (any(idx <- !named_idx & !dots_idx & !plural_idx)) {
+  if (any(bad_idx <- !named_idx & !dots_idx & !plural_idx)) {
     stopf(
       # nolint next: line_length_linter.
       "Invalid custom translator specification(s): %s.\nAll inputs for R must be key-value pairs like fn:arg1|n1[,arg2|n2] or fn:...\\arg1,...,argn.",
-      toString(spec[idx])
+      toString(spec[bad_idx])
     )
   }
 
diff --git a/R/get_src_messages.R b/R/get_src_messages.R
@@ -265,11 +265,11 @@ get_file_src_messages = function(file, custom_params = NULL) {
 parse_src_keywords = function(spec) {
   if (!length(spec)) return(data.table(NULL))
 
-  if (!all(idx <- grepl("[a-zA-Z0-9_]+:[0-9]+", spec))) {
+  if (!all(kwd_idx <- grepl("[a-zA-Z0-9_]+:[0-9]+", spec))) {
     stopf(
       # nolint next: line_length_linter.
       "Invalid custom translator specification(s): %s.\nAll inputs for src must be key-value pairs like fn:arg1. Custom plural messagers are not yet supported.",
-      toString(spec[!idx])
+      toString(spec[!kwd_idx])
     )
   }
 
diff --git a/R/msgmerge.R b/R/msgmerge.R
@@ -4,17 +4,17 @@
 # https://docs.oracle.com/cd/E36784_01/html/E36870/msgmerge-1.html#scrolltoc
 run_msgmerge <- function(po_file, pot_file, previous = FALSE, verbose = TRUE) {
   check_potools_sys_reqs("msgmerge")
-  args <- c(
+  msgmerge_args <- c(
     "--update", shQuote(path.expand(po_file)),
     "--backup=off",
     if (previous) "--previous", #show previous match for fuzzy matches
     shQuote(path.expand(pot_file))
   )
 
   if (verbose) {
-    message("Running system command msgmerge ", paste(args, collapse = " "), "...")
+    message("Running system command msgmerge ", paste(msgmerge_args, collapse = " "), "...")
   }
-  val <- system2("msgmerge", args, stdout = TRUE, stderr = TRUE)
+  val <- system2("msgmerge", msgmerge_args, stdout = TRUE, stderr = TRUE)
   if (!identical(attr(val, "status", exact = TRUE), NULL)) {
     # nocov these warnings? i don't know how to trigger them as of this writing.
     warningf("Running msgmerge on './po/%s' failed:\n  %s", basename(po_file), paste(val, collapse = "\n"))
@@ -27,7 +27,7 @@ run_msgmerge <- function(po_file, pot_file, previous = FALSE, verbose = TRUE) {
     warningf("tools::checkPoFile() found some issues in %s", po_file)
     print(res)
   }
-  return(invisible())
+  invisible()
 }
 
 run_msgfmt = function(po_file, mo_file, verbose) {
@@ -80,21 +80,21 @@ update_en_quot_mo_files <- function(dir, verbose) {
 # https://docs.oracle.com/cd/E36784_01/html/E36870/msginit-1.html#scrolltoc
 run_msginit <- function(po_path, pot_path, locale, width = 80L, verbose = TRUE) {
   check_potools_sys_reqs("msginit")
-  args <- c(
+  msginit_args <- c(
     "-i", shQuote(path.expand(pot_path)),
     "-o", shQuote(path.expand(po_path)),
     "-l", shQuote(locale),
     "-w", width,
     "--no-translator" # don't consult user-email etc
   )
   if (verbose) {
-    message("Running system command msginit ", paste(args, collapse = " "), "...")
+    message("Running system command msginit ", paste(msginit_args, collapse = " "), "...")
   }
-  val <- system2("msginit", args, stdout = TRUE, stderr = TRUE)
+  val <- system2("msginit", msginit_args, stdout = TRUE, stderr = TRUE)
   if (!identical(attr(val, "status", exact = TRUE), NULL)) {
     stopf("Running msginit on '%s' failed", pot_path)
   } else if (verbose) {
     messagef(paste(val, collapse = "\n"))
   }
-  return(invisible())
+  invisible()
 }
diff --git a/R/po_compile.R b/R/po_compile.R
@@ -33,12 +33,12 @@ po_compile = function(dir = ".", package = NULL, lazy = TRUE, verbose = TRUE) {
   }
 
   for (ii in seq_len(nrow(po_metadata))) {
-    row <- po_metadata[ii]
-    if (verbose) messagef("Recompiling '%s' %s translation", row$language, row$type)
-    run_msgfmt(row$po, row$mo, verbose = verbose)
+    row_ii <- po_metadata[ii]
+    if (verbose) messagef("Recompiling '%s' %s translation", row_ii$language, row_ii$type)
+    run_msgfmt(row_ii$po, row_ii$mo, verbose = verbose)
   }
 
-  return(invisible())
+  invisible()
 }
 
 
diff --git a/R/po_create.R b/R/po_create.R
@@ -20,13 +20,13 @@ po_create <- function(languages, dir = ".", verbose = !is_testing()) {
   po_files <- po_language_files(languages, dir)
 
   for (ii in seq_len(nrow(po_files))) {
-    row <- po_files[ii]
-    if (file.exists(row$po_path)) {
-      if (verbose) messagef("Updating '%s' %s translation", row$language, row$type)
-      run_msgmerge(row$po_path, row$pot_path, previous = TRUE, verbose = verbose)
+    row_ii <- po_files[ii]
+    if (file.exists(row_ii$po_path)) {
+      if (verbose) messagef("Updating '%s' %s translation", row_ii$language, row_ii$type)
+      run_msgmerge(row_ii$po_path, row_ii$pot_path, previous = TRUE, verbose = verbose)
     } else {
-      if (verbose) messagef("Creating '%s' %s translation", row$language, row$type)
-      run_msginit(row$po_path, row$pot_path, locale = row$language, verbose = verbose)
+      if (verbose) messagef("Creating '%s' %s translation", row_ii$language, row_ii$type)
+      run_msginit(row_ii$po_path, row_ii$pot_path, locale = row_ii$language, verbose = verbose)
     }
   }
 
diff --git a/R/po_update.R b/R/po_update.R
@@ -36,9 +36,9 @@ po_update <- function(dir = ".", lazy = TRUE, verbose = !is_testing()) {
   }
 
   for (ii in seq_len(nrow(meta))) {
-    row <- meta[ii]
-    if (verbose) messagef("Updating '%s' %s translation", row$language, row$type)
-    run_msgmerge(row$po, row$pot, previous = TRUE, verbose = verbose)
+    row_ii <- meta[ii]
+    if (verbose) messagef("Updating '%s' %s translation", row_ii$language, row_ii$type)
+    run_msgmerge(row_ii$po, row_ii$pot, previous = TRUE, verbose = verbose)
   }
 
   invisible(meta)
diff --git a/R/read_translation.R b/R/read_translation.R
@@ -50,6 +50,7 @@ read_translation = function(msgid, type, file, call, fuzzy, msgstr, metadata) {
       ))
     }
   } else {
+    # nolint next: unnecessary_nesting_linter. False positive due to '='; try removing.
     if (fuzzy == 1L) {
       fuzzy_tag = gettextf(
         "\n **Note: a similar message was previously translated as: **\n%s",
@@ -102,7 +103,7 @@ prompt = function(..., conn = .potools$prompt_conn, require_type = NULL) {
     out = readLines(conn, n=1L, encoding="UTF-8")
     # issue often encountered in dev when adjusting the test packages & things get bumped around...
     # nocov start
-    if (is.na(out)) {
+    if (is.na(out)) { # nolint: unnecessary_nesting_linter. False positive due to '='. Try removing.
       stopf("Connection empty when trying to read prompt %s", do.call(paste, list(...)))
     }
     # nocov end
@@ -113,7 +114,7 @@ prompt = function(..., conn = .potools$prompt_conn, require_type = NULL) {
   if (typeof(out) == require_type) return(out)
 
   messagef("Input must be of type '%s', but received '%s'. Trying again.", require_type, typeof(out))
-  return(prompt(..., conn=conn, require_type=require_type))
+  prompt(..., conn=conn, require_type=require_type)
 }
 
 prompt_with_templates = function(msgid_metadata, prompt_msg) {
diff --git a/R/translate_package.R b/R/translate_package.R
@@ -333,15 +333,15 @@ translate_package = function(
   desc_data = get_desc_data(dir)
   package <- desc_data['Package']
   is_base <- package == 'base'
-  version <- desc_data['Version']
+  version <- desc_data['Version'] # nolint: object_overwrite_linter.
 
   po_dir <- file.path(dir, 'po')
   r_potfile <- file.path(po_dir, glue("R-{package}.pot"))
   src_potfile <- file.path(po_dir, sprintf("%s.pot", if (is_base) 'R' else package))
-  update = file.exists(po_dir) && (file.exists(r_potfile) || file.exists(src_potfile))
+  tr_update = file.exists(po_dir) && (file.exists(r_potfile) || file.exists(src_potfile))
 
   if (verbose) {
-    if (update) {
+    if (tr_update) {
       # is it worthwhile to try and distinguish the creation time of the
       #  R pot file and the src pot file? probably not...
       messagef(
@@ -353,7 +353,7 @@ translate_package = function(
       messagef("Starting translations for package '%s'", package)
     }
   }
-  if (!update) dir.create(po_dir, showWarnings = FALSE)
+  if (!tr_update) dir.create(po_dir, showWarnings = FALSE)
 
   message_data = get_message_data(dir, custom_translation_functions, verbose=verbose)
 
@@ -409,7 +409,7 @@ translate_package = function(
     message_data[type == 'plural', 'msgstr_plural' := .(list(rep("", metadata$nplurals)))]
 
     lang_file <- file.path(po_dir, glue("R-{language}.po"))
-    if (update && file.exists(lang_file)) {
+    if (tr_update && file.exists(lang_file)) {
       if (verbose) {
         messagef(
           'Found existing R translations for %s (%s/%s) in %s. Running msgmerge...',
@@ -424,7 +424,7 @@ translate_package = function(
     }
 
     lang_file <- file.path(po_dir, glue("{language}.po"))
-    if (update && file.exists(lang_file)) {
+    if (tr_update && file.exists(lang_file)) {
       if (verbose) {
         messagef(
           'Found existing src translations for %s (%s/%s) in %s. Running msgmerge...',
@@ -522,7 +522,7 @@ translate_package = function(
 
   # TODO: reinstate source marker tags, at least for src .pot file & maybe for R .pot file too?
   po_compile(dir, package, verbose = verbose)
-  return(invisible())
+  invisible()
 }
 
 # take from those present in r-devel:
diff --git a/R/utils.R b/R/utils.R
@@ -22,7 +22,7 @@ get_desc_data = function(dir, fields = c('Package', 'Version')) {
   if (missing(fields) && (nrow(desc_data) != 1L || anyNA(desc_data))) {
     stopf('%s is not a package (missing Package and/or Version field in DESCRIPTION)', normalizePath(dir))
   }
-  return(drop(desc_data))
+  drop(desc_data)
 }
 
 # msgmerge | msgmerge.R | run_msgmerge()
@@ -76,7 +76,7 @@ check_potools_sys_reqs = function(which = SYSTEM_REQUIREMENTS) {
       toString(which[is_missing]), platform_msg
     ))
   }
-  return(TRUE)
+  TRUE
 }
 # nocov end
 
@@ -87,7 +87,7 @@ list_package_files = function(dir, subdir, subsubdirs = character(), pattern = N
     subsubdir_ = file.path(subdir, subsubdir)
     if (dir.exists(subsubdir_)) files = c(files, file.path(subsubdir, list.files(subsubdir_, pattern = pattern)))
   }
-  return(files)
+  files
 }
 
 # patch analogous fix for Bugzilla#18025 here
diff --git a/R/write_po_file.R b/R/write_po_file.R

Original file line number	Diff line number	Diff line change
`@@ -219,11 +219,11 @@ parse_r_keywords = function(spec) {`
`219`	`219`	`named_idx = grepl("^[a-zA-Z0-9._]+\\\|[0-9]+$", keyval$V2)`
`220`	`220`	`plural_idx = grepl("^[a-zA-Z0-9._]+\\\|[0-9]+,[a-zA-Z0-9._]+\\\|[0-9]+$", keyval$V2)`
`221`	`221`	`dots_idx = grepl("^[.]{3}[\\](?:[a-zA-Z0-9._]+,)*[a-zA-Z0-9._]+$", keyval$V2)`
`222`		`- if (any(idx <- !named_idx & !dots_idx & !plural_idx)) {`
	`222`	`+ if (any(bad_idx <- !named_idx & !dots_idx & !plural_idx)) {`
`223`	`223`	`stopf(`
`224`	`224`	`# nolint next: line_length_linter.`
`225`	`225`	`"Invalid custom translator specification(s): %s.\nAll inputs for R must be key-value pairs like fn:arg1\|n1[,arg2\|n2] or fn:...\\arg1,...,argn.",`
`226`		`- toString(spec[idx])`
	`226`	`+ toString(spec[bad_idx])`
`227`	`227`	`)`
`228`	`228`	`}`
`229`	`229`
Original file line number	Diff line number	Diff line change
`@@ -265,11 +265,11 @@ get_file_src_messages = function(file, custom_params = NULL) {`
`265`	`265`	`parse_src_keywords = function(spec) {`
`266`	`266`	`if (!length(spec)) return(data.table(NULL))`
`267`	`267`
`268`		`- if (!all(idx <- grepl("[a-zA-Z0-9_]+:[0-9]+", spec))) {`
	`268`	`+ if (!all(kwd_idx <- grepl("[a-zA-Z0-9_]+:[0-9]+", spec))) {`
`269`	`269`	`stopf(`
`270`	`270`	`# nolint next: line_length_linter.`
`271`	`271`	`"Invalid custom translator specification(s): %s.\nAll inputs for src must be key-value pairs like fn:arg1. Custom plural messagers are not yet supported.",`
`272`		`- toString(spec[!idx])`
	`272`	`+ toString(spec[!kwd_idx])`
`273`	`273`	`)`
`274`	`274`	`}`
`275`	`275`
Original file line number	Diff line number	Diff line change
`@@ -33,12 +33,12 @@ po_compile = function(dir = ".", package = NULL, lazy = TRUE, verbose = TRUE) {`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`for (ii in seq_len(nrow(po_metadata))) {`
`36`		`- row <- po_metadata[ii]`
`37`		`- if (verbose) messagef("Recompiling '%s' %s translation", row$language, row$type)`
`38`		`- run_msgfmt(row$po, row$mo, verbose = verbose)`
	`36`	`+ row_ii <- po_metadata[ii]`
	`37`	`+ if (verbose) messagef("Recompiling '%s' %s translation", row_ii$language, row_ii$type)`
	`38`	`+ run_msgfmt(row_ii$po, row_ii$mo, verbose = verbose)`
`39`	`39`	`}`
`40`	`40`
`41`		`- return(invisible())`
	`41`	`+ invisible()`
`42`	`42`	`}`
`43`	`43`
`44`	`44`