Skip to content

Commit

Permalink
Do not lint seq() calls with extra arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
Bisaloo committed Jun 22, 2024
1 parent 231abff commit 678de9f
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
12 changes: 8 additions & 4 deletions R/seq_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#' )
#'
#' lint(
#' text = "unlist(lapply(x, seq_len))",
#' text = "unlist(lapply(x, seq, by = 2))",
#' linters = seq_linter()
#' )
#'
Expand Down Expand Up @@ -78,10 +78,14 @@ seq_linter <- function() {

map_funcs <- c("sapply", "lapply", "map")
seq_funcs <- xp_text_in_table(c("seq_len", "seq"))
# count(expr) = 3 because we only want seq() calls without extra arguments
sequence_xpath <- glue("
parent::expr[following-sibling::expr/SYMBOL[ {seq_funcs} ]]
/parent::expr[preceding-sibling::expr/SYMBOL_FUNCTION_CALL[text() = 'unlist']]"
)
parent::expr/parent::expr[
count(expr) = 3
and expr/SYMBOL[ {seq_funcs} ]
and preceding-sibling::expr/SYMBOL_FUNCTION_CALL[text() = 'unlist']
]
")

## The actual order of the nodes is document order
## In practice we need to handle length(x):1
Expand Down
10 changes: 10 additions & 0 deletions tests/testthat/test-seq_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ test_that("finds potential sequence() replacements", {
)
})

test_that("sequence() is not recommended for complex seq() calls", {
linter <- seq_linter()

expect_lint(
"unlist(lapply(x, seq, from = 2))",
NULL,
linter
)
})

test_that("Message vectorization works for multiple lints", {
linter <- seq_linter()

Expand Down

0 comments on commit 678de9f

Please sign in to comment.