Skip to content

Use expect_no_lint() instead of expect_lint(., NULL) #2950

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 17, 2025
Merged

Use expect_no_lint() instead of expect_lint(., NULL) #2950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
715baf3
Use expect_no_lint() instead of expect_lint(., NULL).
mcol Oct 17, 2025
c9a4702
Merge branch 'main' into issue_2737_p1
MichaelChirico Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions tests/testthat/test-namespace_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
test_that("namespace_linter skips allowed usages", {
linter <- namespace_linter()

expect_lint("stats::sd", NULL, linter)
expect_lint("stats::sd(c(1,2,3))", NULL, linter)
expect_lint('"stats"::sd(c(1,2,3))', NULL, linter)
expect_lint('stats::"sd"(c(1,2,3))', NULL, linter)
expect_lint("stats::`sd`(c(1,2,3))", NULL, linter)
expect_no_lint("stats::sd", linter)
expect_no_lint("stats::sd(c(1,2,3))", linter)
expect_no_lint('"stats"::sd(c(1,2,3))', linter)
expect_no_lint('stats::"sd"(c(1,2,3))', linter)
expect_no_lint("stats::`sd`(c(1,2,3))", linter)

expect_lint("datasets::mtcars", NULL, linter)
expect_lint("stats:::print.formula", NULL, linter)
expect_lint('"stats":::print.formula', NULL, linter)
expect_no_lint("datasets::mtcars", linter)
expect_no_lint("stats:::print.formula", linter)
expect_no_lint('"stats":::print.formula', linter)
})

test_that("namespace_linter respects check_exports and check_nonexports arguments", {
expect_lint("stats::ssd(c(1,2,3))", NULL, namespace_linter(check_exports = FALSE))
expect_lint("stats:::ssd(c(1,2,3))", NULL, namespace_linter(check_nonexports = FALSE))
expect_lint("stats:::ssd(c(1,2,3))", NULL, namespace_linter(check_exports = FALSE, check_nonexports = FALSE))
expect_no_lint("stats::ssd(c(1,2,3))", namespace_linter(check_exports = FALSE))
expect_no_lint("stats:::ssd(c(1,2,3))", namespace_linter(check_nonexports = FALSE))
expect_no_lint("stats:::ssd(c(1,2,3))", namespace_linter(check_exports = FALSE, check_nonexports = FALSE))
})

test_that("namespace_linter can work with backticked symbols", {
skip_if_not_installed("rlang")
linter <- namespace_linter()

expect_lint("rlang::`%||%`", NULL, linter)
expect_lint("rlang::`%||%`()", NULL, linter)
expect_no_lint("rlang::`%||%`", linter)
expect_no_lint("rlang::`%||%`()", linter)

expect_lint("rlang::'%||%'", NULL, linter)
expect_lint("rlang::'%||%'()", NULL, linter)
expect_lint('rlang::"%||%"', NULL, linter)
expect_lint('rlang::"%||%"()', NULL, linter)
expect_no_lint("rlang::'%||%'", linter)
expect_no_lint("rlang::'%||%'()", linter)
expect_no_lint('rlang::"%||%"', linter)
expect_no_lint('rlang::"%||%"()', linter)

expect_lint("rlang::`%>%`", "'%>%' is not exported from {rlang}.", linter)
expect_lint("rlang::'%>%'()", "'%>%' is not exported from {rlang}.", linter)
Expand Down
46 changes: 23 additions & 23 deletions tests/testthat/test-redundant_ifelse_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
test_that("redundant_ifelse_linter skips allowed usages", {
linter <- redundant_ifelse_linter()

expect_lint("ifelse(x > 5, 0, 2)", NULL, linter)
expect_lint("ifelse(x > 5, TRUE, NA)", NULL, linter)
expect_lint("ifelse(x > 5, FALSE, NA)", NULL, linter)
expect_lint("ifelse(x > 5, TRUE, TRUE)", NULL, linter)
expect_no_lint("ifelse(x > 5, 0, 2)", linter)
expect_no_lint("ifelse(x > 5, TRUE, NA)", linter)
expect_no_lint("ifelse(x > 5, FALSE, NA)", linter)
expect_no_lint("ifelse(x > 5, TRUE, TRUE)", linter)

expect_lint("ifelse(x > 5, 0L, 2L)", NULL, linter)
expect_lint("ifelse(x > 5, 0L, 10L)", NULL, linter)
expect_no_lint("ifelse(x > 5, 0L, 2L)", linter)
expect_no_lint("ifelse(x > 5, 0L, 10L)", linter)
})

test_that("redundant_ifelse_linter blocks simple disallowed usages", {
Expand Down Expand Up @@ -111,38 +111,38 @@ test_that("redundant_ifelse_linter blocks usages equivalent to as.numeric, optio
test_that("allow10 works as intended", {
linter <- redundant_ifelse_linter(allow10 = TRUE)

expect_lint("ifelse(x > 5, 1L, 0L)", NULL, linter)
expect_lint("ifelse(x > 5, 0L, 1L)", NULL, linter)
expect_no_lint("ifelse(x > 5, 1L, 0L)", linter)
expect_no_lint("ifelse(x > 5, 0L, 1L)", linter)

expect_lint("ifelse(x > 5, 1, 0)", NULL, linter)
expect_lint("ifelse(x > 5, 0, 1)", NULL, linter)
expect_no_lint("ifelse(x > 5, 1, 0)", linter)
expect_no_lint("ifelse(x > 5, 0, 1)", linter)

expect_lint("dplyr::if_else(x > 5, 1L, 0L)", NULL, linter)
expect_lint("data.table::fifelse(x > 5, 0L, 1L)", NULL, linter)
expect_no_lint("dplyr::if_else(x > 5, 1L, 0L)", linter)
expect_no_lint("data.table::fifelse(x > 5, 0L, 1L)", linter)

expect_lint("if_else(x > 5, 1, 0)", NULL, linter)
expect_lint("fifelse(x > 5, 0, 1)", NULL, linter)
expect_no_lint("if_else(x > 5, 1, 0)", linter)
expect_no_lint("fifelse(x > 5, 0, 1)", linter)
})

test_that("ifelse(missing = ) gives correct lints", {
linter <- redundant_ifelse_linter()

expect_lint("if_else(x > 5, TRUE, FALSE, NA)", rex::rex("Just use the logical condition"), linter)
expect_lint("if_else(x > 5, TRUE, FALSE, TRUE)", NULL, linter)
expect_lint("if_else(x > 5, TRUE, FALSE, 5L)", NULL, linter)
expect_no_lint("if_else(x > 5, TRUE, FALSE, TRUE)", linter)
expect_no_lint("if_else(x > 5, TRUE, FALSE, 5L)", linter)

expect_lint("if_else(x > 5, 1L, 0L, NA_integer_)", rex::rex("Prefer as.integer(x)"), linter)
expect_lint("if_else(x > 5, 1L, 0L, 2L)", NULL, linter)
expect_lint("if_else(x > 5, 1L, 0L, 5)", NULL, linter)
expect_no_lint("if_else(x > 5, 1L, 0L, 2L)", linter)
expect_no_lint("if_else(x > 5, 1L, 0L, 5)", linter)

expect_lint("if_else(x > 5, 1, 0, NA_real_)", rex::rex("Prefer as.numeric(x)"), linter)
expect_lint("if_else(x > 5, 1, 0, 2)", NULL, linter)
expect_lint("if_else(x > 5, 1, 0, '5')", NULL, linter)
expect_no_lint("if_else(x > 5, 1, 0, 2)", linter)
expect_no_lint("if_else(x > 5, 1, 0, '5')", linter)

# TRUE/FALSE must be found in yes/no, not missing=
expect_lint("if_else(x > 5, 'a', TRUE, FALSE)", NULL, linter)
expect_lint("if_else(x > 5, 'a', 0L, 1L)", NULL, linter)
expect_lint("if_else(x > 5, 'a', 1, 0)", NULL, linter)
expect_no_lint("if_else(x > 5, 'a', TRUE, FALSE)", linter)
expect_no_lint("if_else(x > 5, 'a', 0L, 1L)", linter)
expect_no_lint("if_else(x > 5, 'a', 1, 0)", linter)
})

test_that("lints vectorize", {
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-which_grepl_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
test_that("which_grepl_linter skips allowed usages", {
# this _could_ be combined as p1|p2, but often it's cleaner to read this way
expect_lint("which(grepl(p1, x) | grepl(p2, x))", NULL, which_grepl_linter())
expect_no_lint("which(grepl(p1, x) | grepl(p2, x))", which_grepl_linter())
})

test_that("which_grepl_linter blocks simple disallowed usages", {
Expand Down
14 changes: 6 additions & 8 deletions tests/testthat/test-whitespace_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
test_that("whitespace_linter skips allowed usages", {
linter <- whitespace_linter()

expect_lint("blah", NULL, linter)
expect_lint(" blah", NULL, linter)
expect_lint(" blah", NULL, linter)
expect_lint("#\tblah", NULL, linter)
expect_no_lint("blah", linter)
expect_no_lint(" blah", linter)
expect_no_lint(" blah", linter)
expect_no_lint("#\tblah", linter)
})

test_that("whitespace_linter skips allowed tab usages inside strings", {
linter <- whitespace_linter()

expect_lint(
expect_no_lint(
'lint_msg <- "dont flag tabs if\tthey are inside a string."',
NULL,
linter
)

expect_lint(
expect_no_lint(
'lint_msg <- "dont flag tabs if\n\tthey are inside multiline strings."',
NULL,
linter
)
})
Expand Down
12 changes: 6 additions & 6 deletions tests/testthat/test-yoda_test_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
test_that("yoda_test_linter skips allowed usages", {
linter <- yoda_test_linter()

expect_lint("expect_equal(x, 2)", NULL, linter)
expect_no_lint("expect_equal(x, 2)", linter)
# namespace qualification doesn't matter
expect_lint("testthat::expect_identical(x, 'a')", NULL, linter)
expect_no_lint("testthat::expect_identical(x, 'a')", linter)
# two variables can't be distinguished which is expected/actual (without
# playing quixotic games trying to parse that out from variable names)
expect_lint("expect_equal(x, y)", NULL, linter)
expect_no_lint("expect_equal(x, y)", linter)
})

test_that("yoda_test_linter blocks simple disallowed usages", {
Expand All @@ -24,8 +24,8 @@ test_that("yoda_test_linter ignores strings in $ expressions", {
linter <- yoda_test_linter()

# the "key" here shows up at the same level of the parse tree as plain "key" normally would
expect_lint('expect_equal(x$"key", 2)', NULL, linter)
expect_lint('expect_equal(x@"key", 2)', NULL, linter)
expect_no_lint('expect_equal(x$"key", 2)', linter)
expect_no_lint('expect_equal(x@"key", 2)', linter)
})

# if we only inspect the first argument & ignore context, get false positives
Expand All @@ -34,7 +34,7 @@ local({
linter <- yoda_test_linter()
patrick::with_parameters_test_that(
"yoda_test_linter ignores usage in pipelines",
expect_lint(sprintf("foo() %s expect_identical(2)", pipe), NULL, linter),
expect_no_lint(sprintf("foo() %s expect_identical(2)", pipe), linter),
pipe = pipes,
.test_name = names(pipes)
)
Expand Down
Loading