Skip to content

Commit

Permalink
linters [a-c]
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico committed Dec 6, 2023
1 parent cfea37f commit da94526
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 19 deletions.
27 changes: 21 additions & 6 deletions tests/testthat/test-any_is_na_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,29 @@ test_that("any_is_na_linter skips allowed usages", {
})

test_that("any_is_na_linter blocks simple disallowed usages", {
linter <- any_is_na_linter()
lint_message <- rex::rex("anyNA(x) is better than any(is.na(x)).")
expect_lint("any(is.na(x))", lint_message, any_is_na_linter())

expect_lint("any(is.na(foo(x)))", lint_message, any_is_na_linter())

expect_lint("any(is.na(x))", lint_message, linter)
expect_lint("any(is.na(foo(x)))", lint_message, linter)
# na.rm doesn't really matter for this since is.na can't return NA
expect_lint("any(is.na(x), na.rm = TRUE)", lint_message, any_is_na_linter())

expect_lint("any(is.na(x), na.rm = TRUE)", lint_message, linter)
# also catch nested usage
expect_lint("foo(any(is.na(x)))", lint_message, any_is_na_linter())
expect_lint("foo(any(is.na(x)))", lint_message, linter)
})

test_that("lints vectorize", {
lint_message <- rex::rex("anyNA(x) is better than any(is.na(x)).")

expect_lint(
trim_some("{
any(is.na(foo(x)))
any(is.na(y), na.rm = TRUE)
}"),
list(
list(lint_message, line_number = 2L),
list(lint_message, line_number = 3L)
),
any_is_na_linter()
)
})
15 changes: 10 additions & 5 deletions tests/testthat/test-assignment_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,17 @@ test_that("%<>% throws a lint", {

test_that("multiple lints throw correct messages", {
expect_lint(
"{ x <<- 1; y ->> 2; z -> 3; x %<>% as.character() }",
trim_some("{
x <<- 1
y ->> 2
z -> 3
x %<>% as.character()
}"),
list(
list(message = "Replace <<- by assigning to a specific environment"),
list(message = "Replace ->> by assigning to a specific environment"),
list(message = "Use <-, not ->"),
list(message = "Avoid the assignment pipe %<>%")
list(message = "Replace <<- by assigning to a specific environment", line_number = 2L),
list(message = "Replace ->> by assigning to a specific environment", line_number = 3L),
list(message = "Use <-, not ->", line_number = 4L),
list(message = "Avoid the assignment pipe %<>%", line_number = 5L)
),
assignment_linter(allow_cascading_assign = FALSE)
)
Expand Down
10 changes: 7 additions & 3 deletions tests/testthat/test-backport_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,14 @@ test_that("backport_linter detects backwards-incompatibility", {
)

expect_lint(
"trimws(...names())",
trim_some("
trimws(
...names()
)
"),
list(
rex::rex("trimws (R 3.2.0) is not available for dependency R >= 3.0.0."),
rex::rex("...names (R 4.1.0) is not available for dependency R >= 3.0.0.")
list(rex::rex("trimws (R 3.2.0) is not available for dependency R >= 3.0.0."), line_number = 1L),
list(rex::rex("...names (R 4.1.0) is not available for dependency R >= 3.0.0."), line_number = 2L)
),
backport_linter("3.0.0")
)
Expand Down
16 changes: 16 additions & 0 deletions tests/testthat/test-boolean_arithmetic_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,19 @@ test_that("boolean_arithmetic_linter requires use of any() or !any()", {
expect_lint("sum(x == y) != 0", lint_msg, linter)
expect_lint("sum(grepl(pattern, x)) > 0L", lint_msg, linter)
})

test_that("lints vectorize", {
lint_msg <- rex::rex("Use any() to express logical aggregations.")

expect_lint(
trim_some("{
length(which(x == y)) > 0L
sum(x == y) != 0
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
boolean_arithmetic_linter()
)
})
16 changes: 16 additions & 0 deletions tests/testthat/test-class_equals_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,19 @@ test_that("class_equals_linter skips usage for subsetting", {
linter
)
})

test_that("lints vectorize", {
lint_msg <- rex::rex("Use inherits(x, 'class-name'), is.<class> or is(x, 'class')")

expect_lint(
trim_some("{
'character' %in% class(x)
class(x) == 'character'
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
class_equals_linter()
)
})
16 changes: 14 additions & 2 deletions tests/testthat/test-commas_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ test_that("returns the correct linting (with default parameters)", {
expect_lint(
"fun(1 ,1)",
list(
msg_before,
msg_after
list(msg_before, column_number = 6L),
list(msg_after, column_number = 8L)
),
linter
)
Expand All @@ -39,6 +39,18 @@ test_that("returns the correct linting (with default parameters)", {
expect_lint("switch(op , x = foo, y = bar)", msg_before, linter)
expect_lint("switch(op, x = foo, y = bar(a = 4 , b = 5))", msg_before, linter)
expect_lint("fun(op, x = foo , y = switch(bar, a = 4, b = 5))", msg_before, linter)
expect_lint(
trim_some("
switch(op ,
x = foo,y = bar
)
"),
list(
list(msg_before, line_number = 1L),
list(msg_after, line_number = 2L)
),
linter
)

expect_lint(
"fun(op ,bar)",
Expand Down
14 changes: 14 additions & 0 deletions tests/testthat/test-condition_call_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,17 @@ patrick::with_parameters_test_that(
},
call_name = c("stop", "warning")
)

test_that("lints vectorize", {
expect_lint(
trim_some("{
stop(e)
warning(w)
}"),
list(
list("stop", line_number = 2L),
list("warning", line_number = 3L)
),
condition_call_linter()
)
})
4 changes: 2 additions & 2 deletions tests/testthat/test-condition_message_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ test_that("condition_message_linter blocks simple disallowed usages", {
)
"),
list(
list(message = rex::rex("Don't use paste to build stop strings.")),
list(message = rex::rex("Don't use paste to build warning strings"))
list(message = rex::rex("Don't use paste to build stop strings."), line_number = 3L),
list(message = rex::rex("Don't use paste to build warning strings"), line_number = 4L)
),
condition_message_linter()
)
Expand Down
14 changes: 14 additions & 0 deletions tests/testthat/test-conjunct_test_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,17 @@ test_that("filter() is assumed to be dplyr::filter() by default, unless o/w spec
linter
)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
stopifnot(A && B)
filter(DF, A & B)
}"),
list(
list("stopifnot", line_number = 2L),
list("filter", line_number = 3L)
),
conjunct_test_linter()
)
})
16 changes: 16 additions & 0 deletions tests/testthat/test-consecutive_assertion_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,22 @@ test_that("Mixing test functions is fine", {
)
})

test_that("lints vectorie", {
expect_lint(
trim_some("{
stopifnot(A)
stopifnot(B)
assert_that(C)
assert_that(D)
}"),
list(
list("stopifnot", line_number = 2L),
list("assert_that", line_number = 4L)
),
consecutive_assertion_linter()
)
})

test_that("old name consecutive_stopifnot_linter() is deprecated", {
expect_warning(
{
Expand Down
18 changes: 18 additions & 0 deletions tests/testthat/test-consecutive_mutate_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,21 @@ test_that("native pipe is linted", {
# Ditto mixed pipes
expect_lint("DF %>% mutate(a = 1) |> mutate(b = 2)", lint_msg, linter)
})

test_that("lints vectorize", {
lint_msg <- rex::rex("Unify consecutive calls to mutate().")

expect_lint(
trim_some("
DF %>%
mutate(a = 1) %>%
mutate(b = 2) %>%
mutate(c = 3)
"),
list(
list(lint_msg, line_number = 3L),
list(lint_msg, line_number = 4L)
),
consecutive_mutate_linter()
)
})
2 changes: 1 addition & 1 deletion tests/testthat/test-cyclocomp_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ test_that("returns the correct linting", {
expect_lint(complex_lines, lint_msg, cc_linter_2)
expect_lint(
complex_lines,
rex::rex("Reduce the cyclomatic complexity of this function from 10 to at most 2."),
list(rex::rex("Reduce the cyclomatic complexity of this function from 10 to at most 2."), line_number = 1L),
cc_linter_2
)
expect_lint(complex_lines, NULL, cyclocomp_linter(10L))
Expand Down

0 comments on commit da94526

Please sign in to comment.