From d5e43c542a679580a1d19c4eb17772a425942fb7 Mon Sep 17 00:00:00 2001 From: yandere-sliver Date: Tue, 28 May 2024 14:53:07 +0200 Subject: [PATCH 1/6] Make require_testthat helper function --- R/expect_lint.R | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/R/expect_lint.R b/R/expect_lint.R index c0c9915ae..40c78ac5d 100644 --- a/R/expect_lint.R +++ b/R/expect_lint.R @@ -42,13 +42,8 @@ #' ) #' @export expect_lint <- function(content, checks, ..., file = NULL, language = "en") { - if (!requireNamespace("testthat", quietly = TRUE)) { - stop( # nocov start - "'expect_lint' and 'expect_no_lint' are designed to work within the 'testthat' testing framework, ", - "but 'testthat' is not installed.", - call. = FALSE - ) # nocov end - } + require_testthat("expect_lint") + old_lang <- set_lang(language) on.exit(reset_lang(old_lang)) @@ -69,7 +64,7 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { wrong_number_fmt <- "got %d lints instead of %d%s" if (is.null(checks)) { msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_str) - return(testthat::expect(n_lints %==% 0L, msg)) + return(expect(n_lints %==% 0L, msg)) } if (!is.list(checks) || !is.null(names(checks))) { # vector or named list @@ -79,7 +74,7 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { if (n_lints != length(checks)) { msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_str) - return(testthat::expect(FALSE, msg)) + return(expect(FALSE, msg)) } local({ @@ -115,7 +110,7 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { call. = FALSE ) } - testthat::expect(ok, msg) + expect(ok, msg) }) }, lints, @@ -129,20 +124,22 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { #' @rdname expect_lint #' @export expect_no_lint <- function(content, ..., file = NULL, language = "en") { + require_testthat("expect_no_lint") expect_lint(content, NULL, ..., file = file, language = language) } #' Test that the package is lint free #' -#' This function is a thin wrapper around lint_package that simply tests there are no -#' lints in the package. It can be used to ensure that your tests fail if the package -#' contains lints. +#' This function is a thin wrapper around lint_package that simply tests there are no lints in the package. +#' It can be used to ensure that your tests fail if the package contains lints. #' #' @param ... arguments passed to [lint_package()] #' @export expect_lint_free <- function(...) { - testthat::skip_on_cran() - testthat::skip_on_covr() + require_testthat("expect_lint_free") + + skip_on_cran() + skip_on_covr() lints <- lint_package(...) has_lints <- length(lints) > 0L @@ -151,10 +148,20 @@ expect_lint_free <- function(...) { if (has_lints) { lint_output <- format(lints) } - result <- testthat::expect( + result <- expect( !has_lints, paste0("Not lint free\n", lint_output) ) invisible(result) } + +# Helper function to check if testthat is installed. +require_testthat <- function(name) { + if (!requireNamespace("testthat", quietly = TRUE)) { + stop( # nocov start + name, "is designed to work within the 'testthat' testing framework, but 'testthat' is not installed.", + call. = FALSE + ) # nocov end + } +} From af7c4cd60003326458fc8de68eca230188d14a92 Mon Sep 17 00:00:00 2001 From: yandere-sliver Date: Tue, 28 May 2024 15:03:16 +0200 Subject: [PATCH 2/6] Update NEWS.md --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 77ace633f..a3fff9f25 100644 --- a/NEWS.md +++ b/NEWS.md @@ -56,6 +56,7 @@ * `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). * `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). +* `expect_lint_free()` and other functions that rely on the {testthat} framework now have a consistent error message. (#2585, @F-Noelle). ### New linters From 5a710c88414175b063c6169fc5f17315d5c5d99c Mon Sep 17 00:00:00 2001 From: yandere-sliver Date: Tue, 28 May 2024 15:17:49 +0200 Subject: [PATCH 3/6] Add back testthat scopeing --- R/expect_lint.R | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/R/expect_lint.R b/R/expect_lint.R index 40c78ac5d..060c04b5e 100644 --- a/R/expect_lint.R +++ b/R/expect_lint.R @@ -64,7 +64,7 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { wrong_number_fmt <- "got %d lints instead of %d%s" if (is.null(checks)) { msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_str) - return(expect(n_lints %==% 0L, msg)) + return(testthat::expect(n_lints %==% 0L, msg)) } if (!is.list(checks) || !is.null(names(checks))) { # vector or named list @@ -74,7 +74,7 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { if (n_lints != length(checks)) { msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_str) - return(expect(FALSE, msg)) + return(testthat::expect(FALSE, msg)) } local({ @@ -110,7 +110,7 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { call. = FALSE ) } - expect(ok, msg) + testthat::expect(ok, msg) }) }, lints, @@ -138,8 +138,8 @@ expect_no_lint <- function(content, ..., file = NULL, language = "en") { expect_lint_free <- function(...) { require_testthat("expect_lint_free") - skip_on_cran() - skip_on_covr() + testthat::skip_on_cran() + testthat::skip_on_covr() lints <- lint_package(...) has_lints <- length(lints) > 0L @@ -148,7 +148,7 @@ expect_lint_free <- function(...) { if (has_lints) { lint_output <- format(lints) } - result <- expect( + result <- testthat::expect( !has_lints, paste0("Not lint free\n", lint_output) ) From cc167c42c99b7194924400b2713328f505cc6bcc Mon Sep 17 00:00:00 2001 From: yandere-sliver Date: Tue, 28 May 2024 15:23:26 +0200 Subject: [PATCH 4/6] Roxygenise --- man/expect_lint_free.Rd | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/man/expect_lint_free.Rd b/man/expect_lint_free.Rd index 87ee5be8f..b842d0f28 100644 --- a/man/expect_lint_free.Rd +++ b/man/expect_lint_free.Rd @@ -10,7 +10,6 @@ expect_lint_free(...) \item{...}{arguments passed to \code{\link[=lint_package]{lint_package()}}} } \description{ -This function is a thin wrapper around lint_package that simply tests there are no -lints in the package. It can be used to ensure that your tests fail if the package -contains lints. +This function is a thin wrapper around lint_package that simply tests there are no lints in the package. +It can be used to ensure that your tests fail if the package contains lints. } From 130c02fa53d19101addfd8705b64c0acdbef0d62 Mon Sep 17 00:00:00 2001 From: yandere-sliver Date: Tue, 28 May 2024 21:13:01 +0200 Subject: [PATCH 5/6] Add space in stop message --- R/expect_lint.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/expect_lint.R b/R/expect_lint.R index 060c04b5e..292469133 100644 --- a/R/expect_lint.R +++ b/R/expect_lint.R @@ -160,7 +160,7 @@ expect_lint_free <- function(...) { require_testthat <- function(name) { if (!requireNamespace("testthat", quietly = TRUE)) { stop( # nocov start - name, "is designed to work within the 'testthat' testing framework, but 'testthat' is not installed.", + name, " is designed to work within the 'testthat' testing framework, but 'testthat' is not installed.", call. = FALSE ) # nocov end } From af6a43b8886468b2ee65d5ef68c2b5a6f3981541 Mon Sep 17 00:00:00 2001 From: yandere-sliver Date: Wed, 29 May 2024 01:14:43 +0200 Subject: [PATCH 6/6] Add a default to require_testthat --- NEWS.md | 4 +++- R/expect_lint.R | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/NEWS.md b/NEWS.md index a3fff9f25..eedfe7f71 100644 --- a/NEWS.md +++ b/NEWS.md @@ -56,7 +56,6 @@ * `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). * `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). -* `expect_lint_free()` and other functions that rely on the {testthat} framework now have a consistent error message. (#2585, @F-Noelle). ### New linters @@ -83,6 +82,9 @@ * `object_name_linter()` and `object_length_linter()` ignore {rlang} name injection like `x |> mutate("{new_name}" := foo(col))` (#1926, @MichaelChirico). No checking is applied in such cases. {data.table} in-place assignments like `DT[, "sPoNGeBob" := "friend"]` are still eligible for lints. +## Notes +* `expect_lint_free()` and other functions that rely on the {testthat} framework now have a consistent error message. (#2585, @F-Noelle). + # lintr 3.1.2 ## New and improved features diff --git a/R/expect_lint.R b/R/expect_lint.R index 292469133..ca4650dd1 100644 --- a/R/expect_lint.R +++ b/R/expect_lint.R @@ -42,7 +42,7 @@ #' ) #' @export expect_lint <- function(content, checks, ..., file = NULL, language = "en") { - require_testthat("expect_lint") + require_testthat() old_lang <- set_lang(language) on.exit(reset_lang(old_lang)) @@ -124,7 +124,7 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { #' @rdname expect_lint #' @export expect_no_lint <- function(content, ..., file = NULL, language = "en") { - require_testthat("expect_no_lint") + require_testthat() expect_lint(content, NULL, ..., file = file, language = language) } @@ -136,7 +136,7 @@ expect_no_lint <- function(content, ..., file = NULL, language = "en") { #' @param ... arguments passed to [lint_package()] #' @export expect_lint_free <- function(...) { - require_testthat("expect_lint_free") + require_testthat() testthat::skip_on_cran() testthat::skip_on_covr() @@ -157,7 +157,7 @@ expect_lint_free <- function(...) { } # Helper function to check if testthat is installed. -require_testthat <- function(name) { +require_testthat <- function(name = linter_auto_name(-5L)) { if (!requireNamespace("testthat", quietly = TRUE)) { stop( # nocov start name, " is designed to work within the 'testthat' testing framework, but 'testthat' is not installed.",