Skip to content

Commit

Permalink
[ci] add more {lintr} linters (#234)
Browse files Browse the repository at this point in the history
  • Loading branch information
jameslamb authored Jan 29, 2025
1 parent 6259c3d commit 02df417
Show file tree
Hide file tree
Showing 15 changed files with 330 additions and 281 deletions.
39 changes: 38 additions & 1 deletion .ci/lint_r_code.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

library(lintr)
library(lintr) # nolint[unused_import]

args <- commandArgs(
trailingOnly = TRUE
Expand Down Expand Up @@ -30,17 +30,50 @@ interactive_text <- paste0(

LINTERS_TO_USE <- list(
"absolute_path" = lintr::absolute_path_linter()
, "any_duplicated" = lintr::any_duplicated_linter()
, "any_is_na" = lintr::any_is_na_linter()
, "assignment" = lintr::assignment_linter()
, "backport" = lintr::backport_linter()
, "boolean_arithmetic" = lintr::boolean_arithmetic_linter()
, "braces" = lintr::brace_linter()
, "class_equals" = lintr::class_equals_linter()
, "commas" = lintr::commas_linter()
, "conjunct_test" = lintr::conjunct_test_linter()
, "duplicate_argument" = lintr::duplicate_argument_linter()
, "empty_assignment" = lintr::empty_assignment_linter()
, "equals_na" = lintr::equals_na_linter()
, "fixed_regex" = lintr::fixed_regex_linter()
, "for_loop_index" = lintr::for_loop_index_linter()
, "function_left" = lintr::function_left_parentheses_linter()
, "function_return" = lintr::function_return_linter()
, "implicit_assignment" = lintr::implicit_assignment_linter()
, "infix_spaces" = lintr::infix_spaces_linter()
, "inner_combine" = lintr::inner_combine_linter()
, "is_numeric" = lintr::is_numeric_linter()
, "lengths" = lintr::lengths_linter()
, "length_levels" = lintr::length_levels_linter()
, "length_test" = lintr::length_test_linter()
, "line_length" = lintr::line_length_linter(length = 150L)
, "literal_coercion" = lintr::literal_coercion_linter()
, "matrix" = lintr::matrix_apply_linter()
, "missing_argument" = lintr::missing_argument_linter()
, "non_portable_path" = lintr::nonportable_path_linter()
, "numeric_leading_zero" = lintr::numeric_leading_zero_linter()
, "outer_negation" = lintr::outer_negation_linter()
, "package_hooks" = lintr::package_hooks_linter()
, "paren_body" = lintr::paren_body_linter()
, "paste" = lintr::paste_linter()
, "quotes" = lintr::quotes_linter()
, "redundant_equals" = lintr::redundant_equals_linter()
, "regex_subset" = lintr::regex_subset_linter()
, "routine_registration" = lintr::routine_registration_linter()
, "scalar_in" = lintr::scalar_in_linter()
, "semicolon" = lintr::semicolon_linter()
, "seq" = lintr::seq_linter()
, "spaces_inside" = lintr::spaces_inside_linter()
, "spaces_left_parens" = lintr::spaces_left_parentheses_linter()
, "sprintf" = lintr::sprintf_linter()
, "string_boundary" = lintr::string_boundary_linter()
, "todo_comments" = lintr::todo_comment_linter(c("todo", "fixme", "to-do"))
, "trailing_blank" = lintr::trailing_blank_lines_linter()
, "trailing_white" = lintr::trailing_whitespace_linter()
Expand Down Expand Up @@ -71,6 +104,10 @@ LINTERS_TO_USE <- list(
)
)
, "unnecessary_concatenation" = lintr::unnecessary_concatenation_linter()
, "unnecessary_lambda" = lintr::unnecessary_lambda_linter()
, "unreachable_code" = lintr::unreachable_code_linter()
, "unused_import" = lintr::unused_import_linter()
, "vector_logic" = lintr::vector_logic_linter()
, "whitespace" = lintr::whitespace_linter()
)

Expand Down
16 changes: 8 additions & 8 deletions r-pkg/R/chomp_aggs.R
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ chomp_aggs <- function(aggs_json = NULL) {

# We can grab that nested data.frame and break out right now
outDT <- data.table::as.data.table(jsonList[["aggregations"]][[aggNames]][["buckets"]])
data.table::setnames(outDT, 'key', aggNames)
data.table::setnames(outDT, "key", aggNames)
return(outDT)
}

Expand Down Expand Up @@ -114,7 +114,7 @@ chomp_aggs <- function(aggs_json = NULL) {
aggNames[length(aggNames) + 1] <- names(colTypes[colTypes == "list"])

# Remove unwanted columns
badCols <- grep("doc_count", names(outDT))
badCols <- grep("doc_count", names(outDT), fixed = TRUE)
if (length(badCols) > 0) {
outDT <- outDT[, !badCols, with = FALSE]
}
Expand All @@ -124,7 +124,7 @@ chomp_aggs <- function(aggs_json = NULL) {

} else {
# Remove unwanted columns, but keep doc_count
badCols <- base::setdiff(grep("doc_count", names(outDT), value = TRUE), "doc_count")
badCols <- base::setdiff(grep("doc_count", names(outDT), value = TRUE, fixed = TRUE), "doc_count")
if (length(badCols) > 0) {
outDT <- outDT[, !badCols, with = FALSE]
}
Expand All @@ -150,7 +150,7 @@ chomp_aggs <- function(aggs_json = NULL) {
# Used in chomp_aggs. Call this by reference, not assignment.
#' @importFrom data.table setnames
.clean_aggs_colnames <- function(DT) {
old <- grep("buckets", names(DT), value = TRUE)
old <- grep("buckets", names(DT), value = TRUE, fixed = TRUE)
new <- gsub("\\.?buckets\\.?", "", old)
data.table::setnames(DT, old, new)
}
Expand Down Expand Up @@ -183,7 +183,7 @@ chomp_aggs <- function(aggs_json = NULL) {
all_convertible <- all(
vapply(
X = as.numeric(names(aggsList[["values"]]))
, FUN = function(val) {
, FUN = function(val) { # nolint[unnecessary_lambda]
return(!is.na(val))
}
, FUN.VALUE = TRUE
Expand All @@ -205,17 +205,17 @@ chomp_aggs <- function(aggs_json = NULL) {
.IsSigTermsAgg <- function(aggsList) {

# check 1 - has exactly two keys - "doc_count", "buckets"
if (! identical(sort(names(aggsList)), c('buckets', 'doc_count'))) {
if (! identical(sort(names(aggsList)), c("buckets", "doc_count"))) {
return(FALSE)
}

# check 2 - "buckets" is a data.frame
if (!is.data.frame(aggsList[['buckets']])) {
if (!is.data.frame(aggsList[["buckets"]])) {
return(FALSE)
}

# check 3 - "buckets" has at least the columns "key", "doc_count", and "bg_count"
if (!all(c('key', 'doc_count', 'bg_count') %in% names(aggsList[['buckets']]))) {
if (!all(c("key", "doc_count", "bg_count") %in% names(aggsList[["buckets"]]))) {
return(FALSE)
}

Expand Down
18 changes: 10 additions & 8 deletions r-pkg/R/chomp_hits.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,22 +65,24 @@ chomp_hits <- function(hits_json = NULL, keep_nested_data_cols = TRUE) {
}

# Strip "_source" from all the column names because blegh
data.table::setnames(batchDT, gsub("_source\\.", "", names(batchDT)))
data.table::setnames(batchDT, gsub("_source.", "", names(batchDT), fixed = TRUE))

# Warn the user if there's nested data
colTypes <- sapply(batchDT, mode)
if (any(colTypes == "list")) {
if (keep_nested_data_cols) {
msg <- paste("Keeping the following nested data columns."
, "Consider using unpack_nested_data for one:\n"
, paste(names(colTypes)[colTypes == "list"]
, collapse = ", "))
msg <- paste(
"Keeping the following nested data columns."
, "Consider using unpack_nested_data for one:\n"
, toString(names(colTypes)[colTypes == "list"])
)
log_info(msg)
} else {

msg <- paste("Deleting the following nested data columns:\n"
, paste(names(colTypes)[colTypes == "list"]
, collapse = ", "))
msg <- paste(
"Deleting the following nested data columns:\n"
, toString(names(colTypes)[colTypes == "list"])
)
log_warn(msg)
batchDT <- batchDT[, !names(colTypes[colTypes == "list"]), with = FALSE]
}
Expand Down
Loading

0 comments on commit 02df417

Please sign in to comment.