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 expect_no_lint #2582

Merged
merged 8 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export(expect_length_linter)
export(expect_lint)
export(expect_lint_free)
export(expect_named_linter)
export(expect_no_lint)
export(expect_not_linter)
export(expect_null_linter)
export(expect_s3_class_linter)
Expand Down
3 changes: 2 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@
* `vector_logic_linter()` is extended to recognize incorrect usage of scalar operators `&&` and `||` inside subsetting expressions like `dplyr::filter(x, A && B)` (#2166, @MichaelChirico).
* `any_is_na_linter()` is extended to catch the unusual usage `NA %in% x` (#2113, @MichaelChirico).
* `make_linter_from_xpath()` errors up front when `lint_message` is missing (instead of delaying this error until the linter is used, #2541, @MichaelChirico).
* `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico).
* `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico).
* `expect_no_lint()` was added as new function to cover the typical use case of expecting no lint message, akin to the recent {testthat} functions like `expect_no_warning()` (#2580, @F-Noelle).

### New linters

Expand Down
14 changes: 11 additions & 3 deletions R/expect_lint.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#' Lint expectation
#'
#' This is an expectation function to test that the lints produced by `lint` satisfy a number of checks.
#' These are expectation functions to test specified linters on sample code in the `testthat` testing framework.
#' * `expect_lint` asserts that specified lints are generated.
#' * `expect_no_lint` asserts that no lints are generated.
#'
#' @param content a character vector for the file content to be linted, each vector element representing a line of
#' text.
Expand All @@ -22,7 +24,7 @@
#' @return `NULL`, invisibly.
#' @examples
#' # no expected lint
#' expect_lint("a", NULL, trailing_blank_lines_linter())
#' expect_no_lint("a", trailing_blank_lines_linter())
#'
#' # one expected lint
#' expect_lint("a\n", "trailing blank", trailing_blank_lines_linter())
Expand All @@ -42,7 +44,8 @@
expect_lint <- function(content, checks, ..., file = NULL, language = "en") {
if (!requireNamespace("testthat", quietly = TRUE)) {
stop( # nocov start
"'expect_lint' is designed to work within the 'testthat' testing framework, but 'testthat' is not installed.",
"'expect_lint' and 'expect_no_lint' are designed to work within the 'testthat' testing framework, ",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally this will just reference the actual call for clarity, but we can save that for later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"but 'testthat' is not installed.",
call. = FALSE
) # nocov end
}
Expand Down Expand Up @@ -123,6 +126,11 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") {
invisible(NULL)
}

#' @rdname expect_lint
#' @export
expect_no_lint <- function(content, ..., file = NULL, language = "en") {
expect_lint(content, NULL, ..., file = file, language = language)
}

#' Test that the package is lint free
#'
Expand Down
11 changes: 9 additions & 2 deletions man/expect_lint.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions tests/testthat/test-expect_lint.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ linter <- assignment_linter()
lint_msg <- "Use <-, not ="

test_that("no checks", {
expect_success(expect_lint("a", NULL, linter))
expect_success(expect_lint("a=1", NULL, list()))
expect_failure(expect_lint("a=1", NULL, linter))
expect_success(expect_no_lint("a", linter))
expect_success(expect_no_lint("a=1", list()))
expect_failure(expect_no_lint("a=1", linter))
})

test_that("single check", {
Expand Down
2 changes: 1 addition & 1 deletion vignettes/creating_linters.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ The main three aspects to test are:
1. Linter returns no lints when there is nothing to lint, e.g.

```r
expect_lint("blah", NULL, assignment_linter())
expect_no_lint("blah", assignment_linter())
```

2. Linter returns a lint when there is something to lint, e.g.
Expand Down
Loading