Skip to content

Commit

Permalink
add templates for all cases, fill out through i
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Dec 7, 2023
1 parent efa7262 commit 7ce25e6
Show file tree
Hide file tree
Showing 39 changed files with 537 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,26 @@ test_that("expect_s4_class_linter skips allowed usages", {
})

test_that("expect_s4_class blocks simple disallowed usages", {
expect_lint(
"expect_true(is(x, 'data.frame'))",
rex::rex("expect_s4_class(x, k) is better than expect_true(is(x, k))"),
expect_s4_class_linter()
)
linter <- expect_s4_class_linter()
lint_msg <- rex::rex("expect_s4_class(x, k) is better than expect_true(is(x, k))")

expect_lint("expect_true(is(x, 'data.frame'))", lint_msg, linter)
# namespace qualification is irrelevant
expect_lint("testthat::expect_true(methods::is(x, 'SpatialPolygonsDataFrame'))", lint_msg, linter)
})

test_that("lints vectorize", {
lint_msg <- rex::rex("expect_s4_class(x, k) is better than expect_true(is(x, k))")

expect_lint(
"testthat::expect_true(methods::is(x, 'SpatialPolygonsDataFrame'))",
rex::rex("expect_s4_class(x, k) is better than expect_true(is(x, k))"),
trim_some("{
expect_true(is(x, 'data.frame'))
expect_true(is(x, 'SpatialPolygonsDataFrame))
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
expect_s4_class_linter()
)
})
20 changes: 18 additions & 2 deletions tests/testthat/test-expect_true_false_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,39 @@ test_that("expect_true_false_linter skips allowed usages", {
})

test_that("expect_true_false_linter blocks simple disallowed usages", {
linter <- expect_true_false_linter()

expect_lint(
"expect_equal(foo(x), TRUE)",
rex::rex("expect_true(x) is better than expect_equal(x, TRUE)"),
expect_true_false_linter()
linter
)

# expect_identical is treated the same as expect_equal
expect_lint(
"testthat::expect_identical(x, FALSE)",
rex::rex("expect_false(x) is better than expect_identical(x, FALSE)"),
expect_true_false_linter()
linter
)

# also caught when TRUE/FALSE is the first argument
expect_lint(
"expect_equal(TRUE, foo(x))",
rex::rex("expect_true(x) is better than expect_equal(x, TRUE)"),
linter
)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
expect_equal(x, TRUE)
expect_equal(x, FALSE)
}"),
list(
list("expect_true", line_number = 2L),
list("expect_false", line_number = 3L)
),
expect_true_false_linter()
)
})
14 changes: 14 additions & 0 deletions tests/testthat/test-expect_type_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,17 @@ local({
is_type = is_types
)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
expect_true(is.integer(x))
expect_equal(typeof(x), 'double')
}"),
list(
list("expect_true", line_number = 2L),
list("expect_equal", line_number = 3L)
),
expect_type_linter()
)
})
16 changes: 16 additions & 0 deletions tests/testthat/test-for_loop_index_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,19 @@ test_that("for_loop_index_linter blocks simple disallowed usages", {
# arbitrary nesting
expect_lint("for (x in foo(bar(y, baz(2, x)))) {}", lint_msg, linter)
})

test_that("lints vectorize", {
lint_msg <- "Don't re-use any sequence symbols as the index symbol in a for loop"

expect_lint(
trim_some("{
for (x in x) {}
for (y in y) {}
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
for_loop_index_linter()
)
})
21 changes: 21 additions & 0 deletions tests/testthat/test-function_return_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,24 @@ test_that("function_return_linter blocks simple disallowed usages", {
linter
)
})

test_that("lints vectorize", {
linter <- function_return_linter()
lint_msg <- rex::rex("Move the assignment outside of the return() clause")

expect_lint(
trim_some("{
function() {
return(x <- 1)
}
function() {
return(y <- 2)
}
}"),
list(
list(lint_msg, line_number = 3L),
list(lint_msg, line_number = 6L)
),
function_return_linter()
)
})
38 changes: 28 additions & 10 deletions tests/testthat/test-ifelse_censor_linter.R
Original file line number Diff line number Diff line change
@@ -1,54 +1,58 @@
test_that("ifelse_censor_linter skips allowed usages", {
expect_lint("ifelse(x == 2, x, y)", NULL, ifelse_censor_linter())
expect_lint("ifelse(x > 2, x, y)", NULL, ifelse_censor_linter())
linter <- ifelse_censor_linter()

expect_lint("ifelse(x == 2, x, y)", NULL, linter)
expect_lint("ifelse(x > 2, x, y)", NULL, linter)
})

test_that("ifelse_censor_linter blocks simple disallowed usages", {
linter <- ifelse_censor_linter()

expect_lint(
"ifelse(x < 0, 0, x)",
rex::rex("pmax(x, y) is preferable to ifelse(x < y, y, x)"),
ifelse_censor_linter()
linter
)

# other equivalents to base::ifelse()
expect_lint(
"if_else(x < 0, 0, x)",
rex::rex("pmax(x, y) is preferable to if_else(x < y, y, x)"),
ifelse_censor_linter()
linter
)
expect_lint(
"fifelse(x < 0, 0, x)",
rex::rex("pmax(x, y) is preferable to fifelse(x < y, y, x)"),
ifelse_censor_linter()
linter
)

# other equivalents for censoring
expect_lint(
"ifelse(x <= 0, 0, x)",
rex::rex("pmax(x, y) is preferable to ifelse(x <= y, y, x)"),
ifelse_censor_linter()
linter
)
expect_lint(
"ifelse(x > 0, x, 0)",
rex::rex("pmax(x, y) is preferable to ifelse(x > y, x, y)"),
ifelse_censor_linter()
linter
)
expect_lint(
"ifelse(x >= 0, x, 0)",
rex::rex("pmax(x, y) is preferable to ifelse(x >= y, x, y)"),
ifelse_censor_linter()
linter
)

# pairwise min/max (similar to censoring)
expect_lint(
"ifelse(x < y, x, y)",
rex::rex("pmin(x, y) is preferable to ifelse(x < y, x, y)"),
ifelse_censor_linter()
linter
)
expect_lint(
"ifelse(x >= y, y, x)",
rex::rex("pmin(x, y) is preferable to ifelse(x >= y, y, x)"),
ifelse_censor_linter()
linter
)

# more complicated expression still matches
Expand All @@ -60,6 +64,20 @@ test_that("ifelse_censor_linter blocks simple disallowed usages", {
expect_lint(
lines,
rex::rex("pmin(x, y) is preferable to ifelse(x > y, y, x)"),
linter
)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
ifelse(x >= y, y, x)
ifelse(x >= 0, x, 0)
}"),
list(
list("pmin", line_number = 2L),
list("pmax", line_number = 3L)
),
ifelse_censor_linter()
)
})
Expand Down
16 changes: 16 additions & 0 deletions tests/testthat/test-inner_combine_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,19 @@ patrick::with_parameters_test_that(
# "unknown POSIXct method argument", "c(as.POSIXct(x, zoo = zzz), as.POSIXct(y, zoo = zzz))",
)
)

test_that("lints vectorize", {
lint_msg <- rex::rex("Combine inputs to vectorized functions first")

expect_lint(
trim_some("{
c(sin(x), sin(y))
c(log(x), log(y))
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
inner_combine_linter()
)
})
14 changes: 14 additions & 0 deletions tests/testthat/test-is_numeric_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,17 @@ test_that("raw strings are handled properly when testing in class", {
expect_lint("class(x) %in% c(R'(integer)', 'numeric')", lint_msg, linter)
expect_lint('class(x) %in% c("numeric", R"--[integer]--")', lint_msg, linter)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
is.numeric(x) || is.integer(x)
class(x) %in% c('integer', 'numeric')
}"),
list(
list(rex::rex("`is.numeric(x) || is.integer(x)`"), line_number = 2L),
list(rex::rex('class(x) %in% c("integer", "numeric")'), line_number = 3L)
),
is_numeric_linter()
)
})
12 changes: 12 additions & 0 deletions tests/testthat/test-length_levels_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,15 @@ test_that("length_levels_linter blocks simple disallowed usages", {
length_levels_linter()
)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
)
})
12 changes: 12 additions & 0 deletions tests/testthat/test-length_test_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,15 @@ local({
.test_name = names(ops)
)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
)
})
12 changes: 12 additions & 0 deletions tests/testthat/test-lengths_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,15 @@ test_that("lengths_linter blocks simple disallowed usages with pipes", {
expect_lint("x |> map_int(length)", lint_msg, linter)
expect_lint("x %>% map_int(length)", lint_msg, linter)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
)
})
2 changes: 2 additions & 0 deletions tests/testthat/test-line_length_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ test_that("line_length_linter blocks disallowed usages", {
list(
list(
message = lint_msg,
line_number = 1L,
column_number = 81L
),
list(
message = lint_msg,
line_number = 2L,
column_number = 81L
)
),
Expand Down
9 changes: 6 additions & 3 deletions tests/testthat/test-literal_coercion_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,13 @@ patrick::with_parameters_test_that(
skip_if_not_installed("rlang")
test_that("multiple lints return custom messages", {
expect_lint(
"c(as.integer(1), lgl(1L))",
trim_some("{
as.integer(1)
lgl(1L)
}"),
list(
rex::rex("Use 1L instead of as.integer(1)"),
rex::rex("Use TRUE instead of lgl(1L)")
list(rex::rex("Use 1L instead of as.integer(1)"), line_number = 2L),
list(rex::rex("Use TRUE instead of lgl(1L)"), line_number = 3L)
),
literal_coercion_linter()
)
Expand Down
8 changes: 4 additions & 4 deletions tests/testthat/test-matrix_apply_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,13 @@ test_that("matrix_apply_linter works with multiple lints in a single expression"
linter <- matrix_apply_linter()

expect_lint(
"rbind(
trim_some("{
apply(x, 1, sum),
apply(y, 2:4, mean, na.rm = TRUE)
)",
}"),
list(
rex::rex("Use rowSums(x)"),
rex::rex("Use rowMeans(colMeans(y, na.rm = TRUE), dims = 3) or colMeans(y, na.rm = TRUE) if y has 4 dimensions")
list(rex::rex("rowSums(x)"), line_number = 2L),
list(rex::rex("rowMeans(colMeans(y, na.rm = TRUE), dims = 3)"), line_number = 3L)
),
linter
)
Expand Down
Loading

0 comments on commit 7ce25e6

Please sign in to comment.