Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add {lintr} precommit hook and remove existing lints #1064

Merged
merged 14 commits into from
Nov 23, 2022
Merged
15 changes: 12 additions & 3 deletions .lintr
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
linters: linters_with_defaults(
commented_code_linter = NULL,
cyclocomp_linter = cyclocomp_linter(25),
cyclocomp_linter = cyclocomp_linter(40),
fixed_regex_linter = NULL,
function_argument_linter = NULL,
indentation_linter = NULL,
line_length_linter(120),
namespace_linter = NULL,
nested_ifelse_linter = NULL,
object_name_linter = NULL,
object_length_linter(50),
object_length_linter(70),
object_usage_linter = NULL,
todo_comment_linter = NULL,
extraction_operator_linter = NULL,
nonportable_path_linter = NULL,
string_boundary_linter = NULL,
undesirable_function_linter = NULL,
undesirable_operator_linter = NULL,
defaults = linters_with_tags(tags = NULL)
)
exclusions: list(
"inst",
"man",
"tests"
"tests",
"touchstone",
"vignettes"
)
13 changes: 12 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,22 @@ repos:
tests/testmanual/addins/.*invalid.*|
tests/testmanual/addins/r-valid\.R|
)$
- id: lintr
additional_dependencies:
- r-lib/lintr
exclude: >
(?x)^(
inst/.*|
man/.*|
tests/.*|
touchstone/.*|
vignettes/.*|
)$
- id: roxygenize
additional_dependencies:
- r-lib/pkgapi
- dplyr@1.0.9
- roxygen2@7.2.1
- roxygen2@7.2.2
- id: use-tidy-description
- id: spell-check
exclude: >
Expand Down
4 changes: 2 additions & 2 deletions R/communicate.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ communicate_warning <- function(changed, transformers) {
#' @keywords internal
communicate_summary <- function(changed, ruler_width) {
if (!getOption("styler.quiet", FALSE)) {
cli::cat_rule(width = max(40, ruler_width))
cli::cat_rule(width = max(40L, ruler_width))
cat("Status\tCount\tLegend \n")
cli::cat_bullet(
"\t", sum(!changed, na.rm = TRUE), "\tFile unchanged.",
Expand All @@ -36,7 +36,7 @@ communicate_summary <- function(changed, ruler_width) {
cli::cat_bullet(
bullet = "cross", "\t", sum(is.na(changed)), "\tStyling threw an error."
)
cli::cat_rule(width = max(40, ruler_width))
cli::cat_rule(width = max(40L, ruler_width))
}
}

Expand Down
8 changes: 4 additions & 4 deletions R/detect-alignment-utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ alignment_drop_comments <- function(pd_by_line) {
alignment_drop_last_expr <- function(pds_by_line) {
# TODO could be skipped if we know it's not a function dec
pd_last_line <- pds_by_line[[length(pds_by_line)]]
last_two_lines <- pd_last_line$token[c(nrow(pd_last_line) - 1, nrow(pd_last_line))]
last_two_lines <- pd_last_line$token[c(nrow(pd_last_line) - 1L, nrow(pd_last_line))]
if (identical(last_two_lines, c("')'", "expr"))) {
pd_last_line <- pd_last_line[-nrow(pd_last_line), ]
}
Expand All @@ -81,7 +81,7 @@ alignment_drop_last_expr <- function(pds_by_line) {
alignment_ensure_trailing_comma <- function(pd_by_line) {
last_pd <- last(pd_by_line)
# needed to make sure comma is added without space
last_pd$spaces[nrow(last_pd)] <- 0
last_pd$spaces[nrow(last_pd)] <- 0L
if (last(last_pd$token) == "','") {
return(pd_by_line)
} else {
Expand All @@ -94,7 +94,7 @@ alignment_ensure_trailing_comma <- function(pd_by_line) {
stylerignore = last_pd$stylerignore[1L],
indents = last_pd$indent[1L]
)
tokens$.lag_spaces <- 0
tokens$.lag_spaces <- 0L

tokens$lag_newlines <- tokens$pos_id <- NULL
pd_by_line[[length(pd_by_line)]] <- rbind(last_pd, tokens)
Expand Down Expand Up @@ -180,7 +180,7 @@ alignment_has_correct_spacing_around_comma <- function(pd_sub) {
if (length(comma_tokens) == 0L) {
return(TRUE)
}
relevant_comma_token <- comma_tokens[seq2(1, length(comma_tokens) - 1L)]
relevant_comma_token <- comma_tokens[seq2(1L, length(comma_tokens) - 1L)]
correct_spaces_before <- pd_sub$.lag_spaces[relevant_comma_token] == 0L
correct_spaces_after <- pd_sub$spaces[relevant_comma_token] > 0L
all(correct_spaces_before) && all(correct_spaces_after)
Expand Down
4 changes: 2 additions & 2 deletions R/detect-alignment.R
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ token_is_on_aligned_line <- function(pd_flat) {
max_previous_col <- max(current_col)

# first col has no leading ,
current_col <- nchar(by_line) - as.integer(column > 1)
current_col <- nchar(by_line) - as.integer(column > 1L)
# Problem `by_line` counting from comma before column 3, previous_line
# counting 1 space before ~
if (column > 1L) {
Expand All @@ -139,7 +139,7 @@ token_is_on_aligned_line <- function(pd_flat) {
current_col <- "^(,[\\s\\t]*)[^ ]*.*$" %>%
gsub("\\1", by_line, perl = TRUE) %>%
nchar() %>%
magrittr::subtract(1)
magrittr::subtract(1L)

if (column > 1L) {
# must add previous columns, as first column might not align
Expand Down
6 changes: 3 additions & 3 deletions R/environments.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ parser_version_get <- function() {
#' @rdname parser_version_set
parser_version_find <- function(pd) {
ifelse(any(pd$token == "equal_assign"),
2,
2L,
ifelse(any(pd$token == "expr_or_assign_or_help"),
3,
1
3L,
1L
)
)
}
Expand Down
6 changes: 3 additions & 3 deletions R/indent.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ indent_without_paren_for_while_fun <- function(pd, indent_by) {
return(pd)
}

if (pd$newlines[length(pd$newlines) - 1] == 0L) {
if (pd$newlines[length(pd$newlines) - 1L] == 0L) {
return(pd)
}
pd$indent[nrow] <- indent_by
Expand Down Expand Up @@ -60,7 +60,7 @@ indent_without_paren_if_else <- function(pd, indent_by) {
] > 0L

if (has_else_without_curly_or_else_chid && needs_indention_now) {
pd$indent[seq(else_idx + 1, nrow(pd))] <- indent_by
pd$indent[seq(else_idx + 1L, nrow(pd))] <- indent_by
}
pd
}
Expand Down Expand Up @@ -108,7 +108,7 @@ compute_indent_indices <- function(pd,
if (is.na(trigger)) {
return(numeric(0L))
}
start <- trigger + 1
start <- trigger + 1L
if (is.null(token_closing)) {
stop <- npd
} else {
Expand Down
2 changes: 1 addition & 1 deletion R/io.R
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ read_utf8_bare <- function(con, warn = TRUE) {
"The file ", con, " is not encoded in UTF-8. ",
"These lines contain invalid UTF-8 characters: "
),
paste(c(utils::head(i), if (n > 6) "..."), collapse = ", ")
toString(c(utils::head(i), if (n > 6L) "..."))
)
}
x
Expand Down
4 changes: 2 additions & 2 deletions R/nest.R
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ drop_cached_children <- function(pd) {
#' @keywords internal
find_pos_id_to_keep <- function(pd) {
if (pd$is_cached[1L]) {
pd$pos_id[pd$parent <= 0]
pd$pos_id[pd$parent <= 0L]
} else {
pd$pos_id
}
Expand Down Expand Up @@ -323,7 +323,7 @@ add_attributes_caching <- function(pd_flat, transformers, more_specs) {
#' @keywords internal
set_spaces <- function(spaces_after_prefix, force_one) {
if (force_one) {
rep(1, length(spaces_after_prefix))
rep(1L, length(spaces_after_prefix))
} else {
pmax(spaces_after_prefix, 1L)
}
Expand Down
4 changes: 2 additions & 2 deletions R/parse.R
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ has_crlf_as_first_line_sep <- function(message, initial_text) {
start_char <- as.numeric(split[3L])
offending_line <- initial_text[as.integer(split[2L])]
if (!is.na(offending_line)) {
if (substr(offending_line, start_char, start_char + 1) == "\r\n") {
if (substr(offending_line, start_char, start_char + 1L) == "\r\n") {
return(TRUE)
}
}
Expand Down Expand Up @@ -119,7 +119,7 @@ get_parse_data <- function(text, include_text = TRUE, ...) {
#' @keywords internal
add_id_and_short <- function(pd) {
pd$pos_id <- seq2(1L, nrow(pd))
pd$short <- substr(pd$text, 1, 5)
pd$short <- substr(pd$text, 1L, 5L)
pd
}

Expand Down
8 changes: 4 additions & 4 deletions R/relevel.R
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ flatten_operators_one <- function(pd_nested) {
pd_token_left <- c(special_token, "PIPE", math_token, "'$'")
pd_token_right <- c(
special_token, "PIPE", "LEFT_ASSIGN",
if (parser_version_get() > 1) "EQ_ASSIGN",
if (parser_version_get() > 1L) "EQ_ASSIGN",
"'+'", "'-'", "'~'"
)
pd_nested %>%
Expand Down Expand Up @@ -99,7 +99,7 @@ bind_with_child <- function(pd_nested, pos) {
wrap_expr_in_expr <- function(pd) {
create_tokens(
"expr", "",
pos_ids = create_pos_ids(pd, 1, after = FALSE),
pos_ids = create_pos_ids(pd, 1L, after = FALSE),
child = pd,
terminal = FALSE,
stylerignore = pd$stylerignore[1L],
Expand Down Expand Up @@ -145,7 +145,7 @@ wrap_expr_in_expr <- function(pd) {
#' )
#' @keywords internal
relocate_eq_assign <- function(pd) {
if (parser_version_get() < 2) {
if (parser_version_get() < 2L) {
post_visit_one(pd, relocate_eq_assign_nest)
} else {
pd
Expand Down Expand Up @@ -239,7 +239,7 @@ relocate_eq_assign_one <- function(pd) {

#' @keywords internal
add_line_col_to_wrapped_expr <- function(pd) {
if (nrow(pd) > 1) abort("pd must be a wrapped expression that has one row.")
if (nrow(pd) > 1L) abort("pd must be a wrapped expression that has one row.")
pd$line1 <- pd$child[[1L]]$line1[1L]
pd$line2 <- last(pd$child[[1L]]$line2)
pd$col1 <- pd$child[[1L]]$col1[1L]
Expand Down
2 changes: 1 addition & 1 deletion R/roxygen-examples-add-remove.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ remove_dont_mask <- function(roxygen) {
1L, 2L, if (roxygen[3L] == "\n") 3L, last(which(roxygen == "}"))
) %>% sort()
list(
code = roxygen[-mask], mask = paste(roxygen[seq2(1, 2)], collapse = "")
code = roxygen[-mask], mask = paste(roxygen[seq2(1L, 2L)], collapse = "")
)
}

Expand Down
8 changes: 4 additions & 4 deletions R/rules-line-breaks.R
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ set_line_break_before_curly_opening <- function(pd) {
linebreak_before_curly <- ifelse(is_function_call(pd),
# if in function call and has pipe, it is not recognized as function call
# and goes to else case
any(pd$lag_newlines[seq2(1, line_break_to_set_idx[1L])] > 0L),
any(pd$lag_newlines[seq2(1L, line_break_to_set_idx[1L])] > 0L),
# if not a function call, only break line if it is a pipe followed by {}
pd$token[line_break_to_set_idx] %in% c("SPECIAL-PIPE", "PIPE")
)
Expand Down Expand Up @@ -110,7 +110,7 @@ set_line_break_before_curly_opening <- function(pd) {
# non-curly expressions after curly expressions must have line breaks
if (length(should_not_be_on_same_line_idx) > 0L) {
comma_exprs_idx <- which(pd$token == "','")
comma_exprs_idx <- setdiff(comma_exprs_idx, 1 + is_not_curly_curly_idx)
comma_exprs_idx <- setdiff(comma_exprs_idx, 1L + is_not_curly_curly_idx)
non_comment_after_comma <- map_int(comma_exprs_idx,
next_non_comment,
pd = pd
Expand Down Expand Up @@ -310,7 +310,7 @@ set_line_break_after_opening_if_call_is_multi_line <- function(pd,
return(pd)
}
break_pos <- find_line_break_position_in_multiline_call(pd)
idx_nested <- next_non_comment(pd, 2)
idx_nested <- next_non_comment(pd, 2L)
if (pd_is_multi_line(pd$child[[idx_nested]]) && sum(pd$lag_newlines) > 0L) {
break_pos <- c(break_pos, idx_nested)
}
Expand Down Expand Up @@ -407,7 +407,7 @@ set_line_break_after_ggplot2_plus <- function(pd) {
which(lead(pd$token == "COMMENT"))
)

pd$lag_newlines[plus_without_comment_after + 1] <- 1L
pd$lag_newlines[plus_without_comment_after + 1L] <- 1L
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions R/rules-tokens.R
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ wrap_multiline_curly <- function(pd, indent_by, key_token, space_after = 1L) {
pd, all_to_be_wrapped_ind, indent_by, space_after
)

if (nrow(pd) > 5L) pd$lag_newlines[6] <- 0L
if (nrow(pd) > 5L) pd$lag_newlines[6L] <- 0L
}
pd
}
Expand All @@ -126,7 +126,7 @@ wrap_multiline_curly <- function(pd, indent_by, key_token, space_after = 1L) {
#' already wrapped into a such.
#' @inheritParams wrap_multiline_curly
#' @keywords internal
wrap_else_multiline_curly <- function(pd, indent_by = 2, space_after = 0L) {
wrap_else_multiline_curly <- function(pd, indent_by = 2L, space_after = 0L) {
if (contains_else_expr(pd) &&
pd_is_multi_line(pd) &&
contains_else_expr_that_needs_braces(pd) &&
Expand Down
2 changes: 1 addition & 1 deletion R/set-assert-args.R
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ assert_tokens <- function(tokens) {
invalid_tokens <- tokens[!(tokens %in% lookup_tokens()$token)]
if (length(invalid_tokens) > 0L) {
abort(paste(
"Token(s)", paste0(invalid_tokens, collapse = ", "), "are invalid.",
"Token(s)", toString(invalid_tokens), "are invalid.",
"You can lookup all valid tokens and their text",
"with styler:::lookup_tokens(). Make sure you supply the values of",
"the column 'token', not 'text'."
Expand Down
2 changes: 1 addition & 1 deletion R/style-guides.R
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ scope_normalize <- function(scope, name = substitute(scope)) {
if (!all((scope %in% levels))) {
abort(paste(
"all values in", name, "must be one of the following:",
paste(levels, collapse = ", ")
toString(levels)
))
}

Expand Down
2 changes: 1 addition & 1 deletion R/stylerignore.R
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#' @keywords internal
env_add_stylerignore <- function(pd_flat) {
if (!env_current$any_stylerignore) {
env_current$stylerignore <- pd_flat[0, ]
env_current$stylerignore <- pd_flat[0L, ]
return()
}
pd_flat_temp <- pd_flat[pd_flat$terminal | pd_flat$is_cached, ] %>%
Expand Down
2 changes: 1 addition & 1 deletion R/testing-public-api.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ test_dry <- function(path, styler, styled = FALSE) {
summary <- styler(path, dry = "on")
checker <- ifelse(styled, testthat::expect_false, testthat::expect_true)
checker(summary$changed)
testthat::expect_true(identical(before, readLines(path)))
testthat::expect_identical(before, readLines(path))

if (styled) {
testthat::expect_error(styler(path, dry = "fail"), NA)
Expand Down
8 changes: 4 additions & 4 deletions R/testing.R
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,11 @@ copy_to_tempdir <- function(path_perm = testthat_file()) {
#' @keywords internal
n_times_faster_with_cache <- function(x1, x2 = x1, ...,
fun = styler::style_text,
n = 3,
n = 3L,
clear = "always") {
rlang::arg_match(clear, c("always", "final", "never", "all but last"))

out <- purrr::map(1:n, n_times_faster_bench,
out <- purrr::map(1L:n, n_times_faster_bench,
x1 = x1, x2 = x2, fun = fun,
..., n = n, clear = clear
)
Expand All @@ -258,7 +258,7 @@ n_times_faster_bench <- function(i, x1, x2, fun, ..., n, clear) {
first <- system.time(fun(x1, ...))

if (is.null(x2)) {
second <- c(elapsed = 1)
second <- c(elapsed = 1L)
} else {
second <- system.time(fun(x2, ...))
}
Expand Down Expand Up @@ -368,7 +368,7 @@ test_transformers_drop <- function(transformers) {
rlang::abort(paste(
"transformers_drop specifies exclusion rules for transformers that ",
"are not in the style guilde. Please add the rule to the style guide ",
"or remove the dropping rules:", paste(diff, collapse = ", ")
"or remove the dropping rules:", toString(diff)
))
}
})
Expand Down
4 changes: 2 additions & 2 deletions R/token-create.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ create_tokens <- function(tokens,
list(
token = tokens,
text = texts,
short = substr(texts, 1, 5),
short = substr(texts, 1L, 5L),
lag_newlines = lag_newlines,
newlines = lead(lag_newlines),
pos_id = pos_ids,
Expand Down Expand Up @@ -85,7 +85,7 @@ create_pos_ids <- function(pd, pos, by = 0.1, after = FALSE, n = 1L) {
}
first <- find_start_pos_id(pd, pos, by, direction, after)
new_ids <- seq(first,
to = first + direction * (n - 1) * by, by = by * direction
to = first + direction * (n - 1L) * by, by = by * direction
)
validate_new_pos_ids(new_ids, after)
new_ids
Expand Down
Loading