Skip to content

Commit

Permalink
#CodeHealth: add .lintr, apply it (#301)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Oct 24, 2023
1 parent 1904ee5 commit 72ff040
Show file tree
Hide file tree
Showing 20 changed files with 193 additions and 116 deletions.
36 changes: 36 additions & 0 deletions .lintr
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
linters: linters_with_defaults(
fixed_regex_linter(),
function_argument_linter(),
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
)),
# 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,
# TODO(michaelchirico): reactivate this. far too many
# false positives for now.
indentation_linter = NULL,
object_name_linter = NULL,
quotes_linter = NULL, # TODO(michaelchirico): switch to "'",
spaces_inside_linter = NULL
)
exclusions: list(
"inst/pkg",
"tests/testthat" = list(object_usage_linter = Inf),
"tests/testthat/test_packages",
"vignettes" = list(
implicit_integer_linter = Inf,
undesirable_function_linter = Inf
)
)
2 changes: 1 addition & 1 deletion R/check_untranslated_cat.R
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
#' unlink(tmp_pkg, recursive = TRUE)
#' rm(pkg, tmp_pkg, message_data)
#' @export
check_untranslated_cat <- function (message_data) {
check_untranslated_cat <- function(message_data) {
if (!is.data.table(message_data)) message_data = as.data.table(message_data)
# not iron-clad but it's a good first pass
cat_calls = unique(
Expand Down
2 changes: 1 addition & 1 deletion R/check_untranslated_src.R
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
#' unlink(tmp_pkg, recursive = TRUE)
#' rm(pkg, tmp_pkg, message_data)
#' @export
check_untranslated_src <- function (message_data) {
check_untranslated_src <- function(message_data) {
if (!is.data.table(message_data)) message_data = as.data.table(message_data)

return(message_data[
Expand Down
2 changes: 1 addition & 1 deletion R/explain_plurals.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ po_explain_plurals <- function(language, index) {
"Supply one language code (see ?translate_package)" =
is.character(language) && length(language) == 1L,
"If supplied, `index` should be a single non-negative number" =
missing(index) || (is.numeric(index) && length(index) == 1L && index > 0)
missing(index) || (is.numeric(index) && length(index) == 1L && index > 0L)
)
language_metadata <- .potools$KNOWN_LANGUAGES[.(language), nomatch = NULL]
if (!nrow(language_metadata)) {
Expand Down
10 changes: 5 additions & 5 deletions R/get_po_messages.R
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ get_po_messages <- function(po_file) {
stopf("Found %d msgid which differs from %d msgstr; corrupted .po file", n_singular, length(msgstr_start))
}

if ((n_plural == 0L && n_msgstr_plural > 0L) || (n_plural > 0 && n_msgstr_plural %% n_plural != 0L)) {
if ((n_plural == 0L && n_msgstr_plural > 0L) || (n_plural > 0L && n_msgstr_plural %% n_plural != 0L)) {
stopf(
"Found %d msgid_plural, which does not evenly divide %d msgstr[n]; corrupted .po file",
n_msgstr_plural, n_plural
)
}
# pre-calculate which lines contain message continuations. Append
# FALSE for a while loop to terminate gracefully on hitting file end
is_msg_continuation = c(grepl('^"', po_lines), FALSE)
is_msg_continuation = c(startsWith(po_lines, '"'), FALSE)

po_data = data.table(
message_source = message_source,
Expand Down Expand Up @@ -88,7 +88,7 @@ get_po_messages <- function(po_file) {
end = find_msg_end(start)
set(po_data, msg_j, 'msgid', build_msg(start, end, 'msgid'))

set(po_data, msg_j, 'fuzzy', as.integer(start != 1L && grepl("^#, fuzzy", po_lines[start-1L])))
set(po_data, msg_j, 'fuzzy', as.integer(start != 1L && startsWith(po_lines[start - 1L], "#, fuzzy")))

start = end + 1L
end = find_msg_end(start)
Expand All @@ -102,7 +102,7 @@ get_po_messages <- function(po_file) {
end = find_msg_end(start)
msg1 = build_msg(start, end, 'msgid')

set(po_data, msg_j, 'fuzzy', as.integer(start != 1L && grepl("^#, fuzzy", po_lines[start-1L])))
set(po_data, msg_j, 'fuzzy', as.integer(start != 1L && startsWith(po_lines[start - 1L], "#, fuzzy")))

start = end + 1L
end = find_msg_end(start)
Expand All @@ -112,7 +112,7 @@ get_po_messages <- function(po_file) {

start = end + 1L
msgstr_plural = character()
while (start <= po_length && grepl('^msgstr\\[', po_lines[start])) {
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
Expand Down
66 changes: 38 additions & 28 deletions R/get_r_messages.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Spiritual cousin version of tools::{x,xn}gettext. Instead of iterating the AST
# as R objects, do so from the parse data given by utils::getParseData().
get_r_messages <- function (dir, custom_translation_functions = NULL, is_base = FALSE, style = c("base", "explicit")) {
get_r_messages <- function(dir, custom_translation_functions = NULL, is_base = FALSE, style = c("base", "explicit")) {
style <- match.arg(style)

expr_data <- rbindlist(lapply(parse_r_files(dir, is_base), getParseData), idcol = 'file')
Expand Down Expand Up @@ -104,7 +104,7 @@ get_r_messages <- function (dir, custom_translation_functions = NULL, is_base =
msg_files = unique(msg$file)
if (is_base) {
paths <- file.path(dir, 'R', msg_files)
share_idx <- grepl('^share/R', msg_files)
share_idx <- startsWith(msg_files, 'share/R')
paths[share_idx] <- file.path(dir, '../../..', msg_files[share_idx])
} else {
paths <- file.path(dir, 'R', msg_files)
Expand Down Expand Up @@ -183,6 +183,7 @@ parse_r_files = function(dir, is_base) {
if (!dir.exists(file.path(r_share_dir, 'R'))) {
# templated to share with src-side message
stopf(
# nolint next: line_length_linter.
"Translation of the 'base' package can only be done on a local mirror of r-devel. Such a copy has a file %s at the top level that is required to proceed.",
"share/R/REMOVE.R"
)
Expand All @@ -208,6 +209,7 @@ parse_r_keywords = function(spec) {
if (ncol(keyval) != 2L) {
idx <- if (ncol(keyval) == 1L) seq_along(spec) else which(is.na(keyval$V2))
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])
)
Expand All @@ -219,6 +221,7 @@ parse_r_keywords = function(spec) {
dots_idx = grepl("^[.]{3}[\\](?:[a-zA-Z0-9._]+,)*[a-zA-Z0-9._]+$", keyval$V2)
if (any(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])
)
Expand All @@ -228,7 +231,7 @@ parse_r_keywords = function(spec) {
singular = list(
dots = lapply(
which(dots_idx),
function(ii) list(
function(ii) list( # nolint: brace_linter.
fname = keyval$V1[ii],
excluded_args = strsplit(gsub("^[.]{3}[\\]", "", keyval$V2[ii]), ",", fixed = TRUE)[[1L]]
)
Expand Down Expand Up @@ -305,7 +308,9 @@ domain_fmt_funs <- function(use_conditions = TRUE) {
NON_DOTS_ARGS = c("domain", "call.", "appendLF", "immediate.", "noBreaks.")

# for functions (e.g. domain_dots_funs) where we extract strings from ... arguments
get_dots_strings = function(expr_data, funs, arg_names, exclude = c('gettext', 'gettextf', 'ngettext'), recursive = TRUE) {
get_dots_strings = function(expr_data, funs, arg_names,
exclude = c('gettext', 'gettextf', 'ngettext'),
recursive = TRUE) {
call_neighbors = get_call_args(expr_data, funs)
call_neighbors = drop_suppressed_and_named(call_neighbors, expr_data, arg_names)

Expand Down Expand Up @@ -344,8 +349,9 @@ get_named_arg_strings = function(expr_data, fun, args, recursive = FALSE, plural
idx = shift(token, fill = '') == 'SYMBOL_SUB' & shift(text, fill = '') %chin% names(args)
if (any(idx) & !all(matched <- names(args) %chin% text[token == 'SYMBOL_SUB'])) {
stopf(
# nolint next: line_length_linter.
"In line %s of %s, found a call to %s that names only some of its messaging arguments explicitly. Expected all of [%s] to be named. Please name all or none of these arguments.",
expr_data[.BY, on = c(id = 'ancestor'), line1[1L]], .BY$file, .BY$fname, toString(names(args))
expr_data[.BY, on = c(id = 'ancestor'), line1[1L]], .BY$file, .BY$fname, toString(names(args)[!matched])
)
}
.(id = id[idx])
Expand Down Expand Up @@ -486,8 +492,8 @@ build_call = function(lines, comments, params) {
}

adjust_tabs = function(l) {
while((idx <- regexpr("\t", l, fixed = TRUE)) > 0L) {
l = sub("\t", strrep(" ", 9L-(idx %% 8L)), l, fixed = TRUE)
while ((idx <- regexpr("\t", l, fixed = TRUE)) > 0L) {
l = sub("\t", strrep(" ", 9L - (idx %% 8L)), l, fixed = TRUE)
}
l
}
Expand Down Expand Up @@ -523,26 +529,30 @@ clean_text = function(x) {
return(x)
}

string_schema = function() data.table(
file = character(),
# needed to build the call
parent = integer(),
# needed to order the strings correctly within the call
id = integer(),
fname = character(),
msgid = character(),
msgid_plural = list()
)
string_schema = function() {
data.table(
file = character(),
# needed to build the call
parent = integer(),
# needed to order the strings correctly within the call
id = integer(),
fname = character(),
msgid = character(),
msgid_plural = list()
)
}

# the schema for empty edge cases
r_message_schema = function() data.table(
type = character(),
file = character(),
msgid = character(),
msgid_plural = list(),
line_number = integer(),
call = character(),
is_repeat = logical(),
is_marked_for_translation = logical(),
is_templated = logical()
)
r_message_schema = function() {
data.table(
type = character(),
file = character(),
msgid = character(),
msgid_plural = list(),
line_number = integer(),
call = character(),
is_repeat = logical(),
is_marked_for_translation = logical(),
is_templated = logical()
)
}
Loading

0 comments on commit 72ff040

Please sign in to comment.