From 361fdd17b46cf6c0209e239fb6e600e8db5b8699 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 20 Jun 2025 23:51:56 -0700 Subject: [PATCH 01/18] Add pkgchk_uses_dontrun as a watch check --- R/check-uses-dontrun.R | 80 ++++++++++++++++++++++++++++++++++++++++++ R/summarise-checks.R | 6 ++-- 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 R/check-uses-dontrun.R diff --git a/R/check-uses-dontrun.R b/R/check-uses-dontrun.R new file mode 100644 index 00000000..b4c27862 --- /dev/null +++ b/R/check-uses-dontrun.R @@ -0,0 +1,80 @@ +#' Check if examples use `\dontrun{}` +#' +#' This check identifies functions in the package documentation that have +#' examples using `\dontrun{}`. The use of `\dontrun{}` is discouraged by +#' CRAN and should only be used the example really cannot be executed +#' (e.g. because of missing additional software, missing API keys, ...) by +#' the user. Instead use methods to run the examples conditionally, such as +#' with the `@examplesIf()` roxygen tag. +#' +#' @param checks A 'pkgcheck' object with full \pkg{pkgstats} summary and +#' \pkg{goodpractice} results. +#' @return character vector of function names that have examples using `\dontrun{}`. +#' @noRd +pkgchk_uses_dontrun <- function(checks) { + rd_files <- list.files( + fs::path( + checks$pkg$path, + "man" + ), + pattern = "\\.Rd$", + full.names = TRUE + ) + + parsed_rds <- lapply( + rd_files, + tools::parse_Rd, + warningCalls = FALSE, + macros = FALSE, + permissive = TRUE + ) + + has_dontrun_examples <- vapply( + parsed_rds, + \(rd) { + # get_Rd_meta doesn't get the \dontrun part of the examples section, + # so need to look for it manually. + # Using as.character() + grepl() may be a bit imprecise, however parsing + # the Rd directly is messy, and I think this is sufficient + rd_char <- as.character(rd) + any(grepl("\\examples", rd_char, fixed = TRUE)) && + any(grepl("\\dontrun", rd_char, fixed = TRUE)) + }, + logical(1) + ) + + fn_names <- if (any(has_dontrun_examples)) { + vapply( + parsed_rds[has_dontrun_examples], + \(rd) { + fn_name <- get_Rd_meta(rd, "name") + ret <- if (length(fn_name) == 0) "" else fn_name + ret + }, + character(1) + ) + } else { + character(0) + } + + fn_names +} + +output_pkgchk_uses_dontrun <- function(checks) { + out <- list( + check_pass = length(checks$checks$uses_dontrun) == 0, + summary = "", + print = "" + ) + + if (!out$check_pass) { + out$summary <- "Examples should not use `\\dontrun{{}}` unless really necessary." + out$print <- paste0( + "The following functions have examples that use `\\dontrun{{}}`:\n'", + paste(checks$checks$uses_dontrun, collapse = "', '"), + "'.\nConsider using `@examplesIf()` to conditionally run examples instead." + ) + } + + return(out) +} diff --git a/R/summarise-checks.R b/R/summarise-checks.R index 8dad6f37..d421bdfa 100644 --- a/R/summarise-checks.R +++ b/R/summarise-checks.R @@ -142,7 +142,8 @@ order_checks <- function (fns) { # additionally explicitly listed below in `watch_checks()`: "obsolete_pkg_deps", "unique_fn_names", - "num_imports" + "num_imports", + "uses_dontrun" ) fns <- fns [which (fns %in% ord)] @@ -158,7 +159,8 @@ watch_checks <- function (output_fns) { watch_list <- c ( "obsolete_pkg_deps", "unique_fn_names", - "num_imports" + "num_imports", + "uses_dontrun" ) all_checks [which (all_checks %in% watch_list)] From 057a3fb1565102abf26642c1551f4e9d95721995 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 11 Jul 2025 15:53:59 -0700 Subject: [PATCH 02/18] Fixes to pkgchk_uses_dontrun * Use tryCatch when parsing Rd files * Use .Rd_get_section to pull out examples * Different summary if all vs some examples use dontrun --- R/check-uses-dontrun.R | 87 ++++++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 37 deletions(-) diff --git a/R/check-uses-dontrun.R b/R/check-uses-dontrun.R index b4c27862..af4acfae 100644 --- a/R/check-uses-dontrun.R +++ b/R/check-uses-dontrun.R @@ -21,60 +21,73 @@ pkgchk_uses_dontrun <- function(checks) { full.names = TRUE ) - parsed_rds <- lapply( - rd_files, - tools::parse_Rd, - warningCalls = FALSE, - macros = FALSE, - permissive = TRUE - ) + parsed_rds <- parse_rd_files(rd_files) - has_dontrun_examples <- vapply( - parsed_rds, - \(rd) { - # get_Rd_meta doesn't get the \dontrun part of the examples section, - # so need to look for it manually. - # Using as.character() + grepl() may be a bit imprecise, however parsing - # the Rd directly is messy, and I think this is sufficient - rd_char <- as.character(rd) - any(grepl("\\examples", rd_char, fixed = TRUE)) && - any(grepl("\\dontrun", rd_char, fixed = TRUE)) - }, - logical(1) - ) + # Only check Rds that actually have examples + has_egs <- pkgchk_fns_have_exs(checks) - fn_names <- if (any(has_dontrun_examples)) { - vapply( - parsed_rds[has_dontrun_examples], - \(rd) { - fn_name <- get_Rd_meta(rd, "name") - ret <- if (length(fn_name) == 0) "" else fn_name - ret - }, - character(1) - ) - } else { - character(0) - } + has_dontrun <- vapply( + parsed_rds[names(has_egs[has_egs])], + has_dontrun_examples, + FUN.VALUE = logical(1L) + ) - fn_names + has_dontrun } output_pkgchk_uses_dontrun <- function(checks) { out <- list( - check_pass = length(checks$checks$uses_dontrun) == 0, + check_pass = all(!checks$checks$uses_dontrun), summary = "", print = "" ) if (!out$check_pass) { - out$summary <- "Examples should not use `\\dontrun{{}}` unless really necessary." + out$summary <- if (all(checks$checks$uses_dontrun)) { + # When #248 is addressed, this condition should be :heavy_multiplication_x: + "All examples use `\\dontrun{}`." + } else { + # :eyes: when some examples use `\dontrun{}` + "Examples should not use `\\dontrun{{}}` unless really necessary." + } out$print <- paste0( "The following functions have examples that use `\\dontrun{{}}`:\n'", - paste(checks$checks$uses_dontrun, collapse = "', '"), + paste( + names(checks$checks$uses_dontrun[checks$checks$uses_dontrun]), + collapse = "', '" + ), "'.\nConsider using `@examplesIf()` to conditionally run examples instead." ) } return(out) } + + +## Utilities for parsing examples in Rd files +get_Rd_section <- utils::getFromNamespace (".Rd_get_section", "tools") + +parse_rd_files <- function(rd_files) { + rds <- lapply( + rd_files, + function(f) { + tryCatch( + tools::parse_Rd(f, warningCalls = FALSE, macros = FALSE, permissive = TRUE), + error = function(e) { + warning(sprintf("Error parsing Rd file '%s': %s", file, e$message)) + NULL + } + ) + } + ) + nms <- vapply(rds, get_Rd_meta, FUN.VALUE = character(1), "name") + stats::setNames(rds, nms) +} + +has_dontrun_examples <- function(rd) { + ex <- get_Rd_section(rd, "examples") + tags <- vapply(ex, function(exi) attr(exi, "Rd_tag"), character(1L)) + + # Check if there are any \dontrun blocks + any(tags == "\\dontrun") +} From 965e1994960d31d292cc94a969071887b3223b7b Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 11 Jul 2025 15:54:24 -0700 Subject: [PATCH 03/18] Increment number of checks in list-checks test --- tests/testthat/test-list-checks.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-list-checks.R b/tests/testthat/test-list-checks.R index 3f33a5f2..e44d7ca3 100644 --- a/tests/testthat/test-list-checks.R +++ b/tests/testthat/test-list-checks.R @@ -3,7 +3,7 @@ test_that ("list-checks", { chks <- list_pkgchecks (), "The following checks are currently implemented" ) - expect_length (chks, 22L) + expect_length (chks, 23L) expect_silent ( chks2 <- list_pkgchecks (quiet = TRUE) From ca6d7290cf0f82e94094b8bc683c0456ece11d49 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 11 Jul 2025 15:54:50 -0700 Subject: [PATCH 04/18] Add tests for chk_uses_dontrun --- tests/testthat/test-check-uses-dontrun.R | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/testthat/test-check-uses-dontrun.R diff --git a/tests/testthat/test-check-uses-dontrun.R b/tests/testthat/test-check-uses-dontrun.R new file mode 100644 index 00000000..06c6e26f --- /dev/null +++ b/tests/testthat/test-check-uses-dontrun.R @@ -0,0 +1,36 @@ +test_that ("check examples dont use dontrun", { + + checks <- make_check_data () + + # All examples use `\dontrun{}` + ci_out <- output_pkgchk_uses_dontrun (checks) + + expect_type (ci_out, "list") + expect_length (ci_out, 3L) + expect_named (ci_out, c ("check_pass", "summary", "print")) + expect_false (ci_out$check_pass) + expect_equal ( + ci_out$summary, + "All examples use `\\dontrun{}`." + ) + expect_length (ci_out$print, 1L) + expect_true (nzchar (ci_out$print)) + + # Some examples use `\dontrun{}` + checks$checks$uses_dontrun ["pkgstats_summary"] <- FALSE + ci_out <- output_pkgchk_uses_dontrun (checks) + + expect_false (ci_out$check_pass) + expect_equal ( + ci_out$summary, + "Examples should not use `\\dontrun{{}}` unless really necessary." + ) + + # No examples using `\dontrun{}` + checks$checks$uses_dontrun[] <- FALSE + ci_out <- output_pkgchk_uses_dontrun (checks) + + expect_true (ci_out$check_pass) + expect_equal (ci_out$summary, "") + expect_equal (ci_out$print, "") +}) From 97ce4b02aca540f1d73a0f0a4e9f85198844b5d4 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 22 Aug 2025 11:50:38 -0700 Subject: [PATCH 05/18] Extract out check_for_dontrun so testable --- R/check-uses-dontrun.R | 44 +++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/R/check-uses-dontrun.R b/R/check-uses-dontrun.R index af4acfae..2af4defe 100644 --- a/R/check-uses-dontrun.R +++ b/R/check-uses-dontrun.R @@ -21,18 +21,7 @@ pkgchk_uses_dontrun <- function(checks) { full.names = TRUE ) - parsed_rds <- parse_rd_files(rd_files) - - # Only check Rds that actually have examples - has_egs <- pkgchk_fns_have_exs(checks) - - has_dontrun <- vapply( - parsed_rds[names(has_egs[has_egs])], - has_dontrun_examples, - FUN.VALUE = logical(1L) - ) - - has_dontrun + check_for_dontrun(rd_files) } output_pkgchk_uses_dontrun <- function(checks) { @@ -45,7 +34,7 @@ output_pkgchk_uses_dontrun <- function(checks) { if (!out$check_pass) { out$summary <- if (all(checks$checks$uses_dontrun)) { # When #248 is addressed, this condition should be :heavy_multiplication_x: - "All examples use `\\dontrun{}`." + "All examples use `\\dontrun{}`." } else { # :eyes: when some examples use `\dontrun{}` "Examples should not use `\\dontrun{{}}` unless really necessary." @@ -63,16 +52,35 @@ output_pkgchk_uses_dontrun <- function(checks) { return(out) } +check_for_dontrun <- function(rd_files) { + parsed_rds <- parse_rd_files(rd_files) + + # Only check Rds that actually have examples + has_egs <- pkgchk_fns_have_exs(checks) + + has_dontrun <- vapply( + parsed_rds[names(has_egs[has_egs])], + has_dontrun_examples, + FUN.VALUE = logical(1L) + ) + + has_dontrun +} ## Utilities for parsing examples in Rd files -get_Rd_section <- utils::getFromNamespace (".Rd_get_section", "tools") +get_Rd_section <- utils::getFromNamespace(".Rd_get_section", "tools") parse_rd_files <- function(rd_files) { rds <- lapply( rd_files, function(f) { tryCatch( - tools::parse_Rd(f, warningCalls = FALSE, macros = FALSE, permissive = TRUE), + tools::parse_Rd( + f, + warningCalls = FALSE, + macros = FALSE, + permissive = TRUE + ), error = function(e) { warning(sprintf("Error parsing Rd file '%s': %s", file, e$message)) NULL @@ -85,9 +93,9 @@ parse_rd_files <- function(rd_files) { } has_dontrun_examples <- function(rd) { - ex <- get_Rd_section(rd, "examples") - tags <- vapply(ex, function(exi) attr(exi, "Rd_tag"), character(1L)) - + ex <- get_Rd_section(rd, "examples") + tags <- vapply(ex, function(exi) attr(exi, "Rd_tag"), character(1L)) + # Check if there are any \dontrun blocks any(tags == "\\dontrun") } From caf609547d427c14c10bc03dff8c6fedfa77bc53 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 22 Aug 2025 15:56:21 -0700 Subject: [PATCH 06/18] Abstract out fns_have_exs so can be called elsewhere --- R/check-fns-have-exs.R | 58 ++++++++++++++++++++++-------------------- R/check-uses-dontrun.R | 6 ++--- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/R/check-fns-have-exs.R b/R/check-fns-have-exs.R index b7376e6f..9370db18 100644 --- a/R/check-fns-have-exs.R +++ b/R/check-fns-have-exs.R @@ -7,11 +7,39 @@ #' @noRd pkgchk_fns_have_exs <- function (checks) { - rd <- list_rd_files (checks$pkg$path) # utils.R + files <- list_rd_files (checks$pkg$path) # utils.R - # don't check for examples in datasets (#103), identified by keyword + fns_have_exs(rd) +} + +output_pkgchk_fns_have_exs <- function (checks) { + + no_ex <- which (!checks$checks$fns_have_exs) + no_ex_fns <- names (checks$checks$fns_have_exs) [no_ex] + + out <- list ( + check_pass = length (no_ex) == 0L, + summary = "", + print = "" + ) # no print method + + out$summary <- ifelse (out$check_pass, + "All functions have examples.", + paste0 ( + "These functions do not have ", + "examples: [", + paste0 (no_ex_fns, collapse = ", "), + "]." + ) + ) + + return (out) +} + +fns_have_exs <- function(rd_files) { + # don't check for examples in datasets (#103), identified by keyword what <- c ("name", "docType", "keyword", "examples") - rd_dat <- vapply (rd, function (i) { + rd_dat <- vapply (rd_files, function (i) { rd_i <- tools::parse_Rd (i, permissive = TRUE) dat <- lapply (what, function (j) { get_Rd_meta (rd_i, j) @@ -44,27 +72,3 @@ pkgchk_fns_have_exs <- function (checks) { return (has_ex) } - -output_pkgchk_fns_have_exs <- function (checks) { - - no_ex <- which (!checks$checks$fns_have_exs) - no_ex_fns <- names (checks$checks$fns_have_exs) [no_ex] - - out <- list ( - check_pass = length (no_ex) == 0L, - summary = "", - print = "" - ) # no print method - - out$summary <- ifelse (out$check_pass, - "All functions have examples.", - paste0 ( - "These functions do not have ", - "examples: [", - paste0 (no_ex_fns, collapse = ", "), - "]." - ) - ) - - return (out) -} diff --git a/R/check-uses-dontrun.R b/R/check-uses-dontrun.R index 2af4defe..7aad3e0b 100644 --- a/R/check-uses-dontrun.R +++ b/R/check-uses-dontrun.R @@ -21,7 +21,7 @@ pkgchk_uses_dontrun <- function(checks) { full.names = TRUE ) - check_for_dontrun(rd_files) + uses_dontrun(rd_files) } output_pkgchk_uses_dontrun <- function(checks) { @@ -52,11 +52,11 @@ output_pkgchk_uses_dontrun <- function(checks) { return(out) } -check_for_dontrun <- function(rd_files) { +uses_dontrun <- function(rd_files) { parsed_rds <- parse_rd_files(rd_files) # Only check Rds that actually have examples - has_egs <- pkgchk_fns_have_exs(checks) + has_egs <- fns_have_exs(rd_files) has_dontrun <- vapply( parsed_rds[names(has_egs[has_egs])], From 415287d053aa31d433ca0ef65c90b89daf1f7de8 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 22 Aug 2025 16:37:10 -0700 Subject: [PATCH 07/18] Find bare R code and dontrun R code --- R/check-uses-dontrun.R | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/R/check-uses-dontrun.R b/R/check-uses-dontrun.R index 7aad3e0b..f21af8b3 100644 --- a/R/check-uses-dontrun.R +++ b/R/check-uses-dontrun.R @@ -93,9 +93,45 @@ parse_rd_files <- function(rd_files) { } has_dontrun_examples <- function(rd) { + browser() ex <- get_Rd_section(rd, "examples") tags <- vapply(ex, function(exi) attr(exi, "Rd_tag"), character(1L)) # Check if there are any \dontrun blocks - any(tags == "\\dontrun") + + # Code that is not wrapped in \dontrun has the attribute Rd_tag set to "RCODE" + # and is listed line-by-line in the examples section (i.e., each line has the + # Rd_tag "RCODE" attribute). Code that that is wrapped in \dontrun has the + # attribute Rd_tag set to "\\dontrun" and is nested in a list element, and + # within that list element the code lines have the Rd_tag attribute set to + # "VERB" (i.e. 'verbatim'; not run). + + # Remove RCODE lines that are just whitespace or comments + bare_r_code <- grep( + "(^\\s+$)|(^#)", + unlist(ex[tags == "RCODE"]), + invert = TRUE, + value = TRUE + ) + + dontrun_r_code <- grep( + "(^\\s+$)|(^#)", + unlist(ex[tags == "\\dontrun"]), + invert = TRUE, + value = TRUE + ) + + any_bare_r_code <- length(bare_r_code) > 0 + any_dont_run <- length(dontrun_r_code) > 0 + + if (any_bare_r_code && any_dont_run) { + # If there is bare R code and also \dontrun, we consider it a dontrun example + return("some") + } else if (any_dont_run) { + # If there is only \dontrun, we consider it a dontrun example + return("all") + } else { + # No dontrun examples found + return("none") + } } From 8427e10518761459159aa51910f22263829ea5ba Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 22 Aug 2025 16:50:27 -0700 Subject: [PATCH 08/18] typo --- R/check-fns-have-exs.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/check-fns-have-exs.R b/R/check-fns-have-exs.R index 9370db18..2e99d2e0 100644 --- a/R/check-fns-have-exs.R +++ b/R/check-fns-have-exs.R @@ -7,7 +7,7 @@ #' @noRd pkgchk_fns_have_exs <- function (checks) { - files <- list_rd_files (checks$pkg$path) # utils.R + rd <- list_rd_files (checks$pkg$path) # utils.R fns_have_exs(rd) } From 647ae118c6f4043d2c47d1850e7c698472e4d063 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 22 Aug 2025 16:50:43 -0700 Subject: [PATCH 09/18] remove browser() --- R/check-uses-dontrun.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/check-uses-dontrun.R b/R/check-uses-dontrun.R index f21af8b3..ab47c8e1 100644 --- a/R/check-uses-dontrun.R +++ b/R/check-uses-dontrun.R @@ -93,7 +93,7 @@ parse_rd_files <- function(rd_files) { } has_dontrun_examples <- function(rd) { - browser() + ex <- get_Rd_section(rd, "examples") tags <- vapply(ex, function(exi) attr(exi, "Rd_tag"), character(1L)) From ba9d0aad42b6c1dd1512abb216e8cfb5f1b949fb Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Wed, 27 Aug 2025 16:53:40 -0700 Subject: [PATCH 10/18] uses_dont run gives either all/some/none * Determine if all, some, or none of the examples are wrapped in `dont_run` * print/summary messages based on all/some/none * Tweak function names for clarity --- R/check-uses-dontrun.R | 56 +++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/R/check-uses-dontrun.R b/R/check-uses-dontrun.R index ab47c8e1..7eab64b8 100644 --- a/R/check-uses-dontrun.R +++ b/R/check-uses-dontrun.R @@ -21,50 +21,52 @@ pkgchk_uses_dontrun <- function(checks) { full.names = TRUE ) - uses_dontrun(rd_files) + rd_files_use_dontrun(rd_files) } output_pkgchk_uses_dontrun <- function(checks) { + out <- list( - check_pass = all(!checks$checks$uses_dontrun), + check_pass = all(checks$checks$uses_dontrun == "none"), summary = "", print = "" ) if (!out$check_pass) { - out$summary <- if (all(checks$checks$uses_dontrun)) { - # When #248 is addressed, this condition should be :heavy_multiplication_x: - "All examples use `\\dontrun{}`." + out$print <- "'.\nConsider using `@examplesIf()` to conditionally run examples instead." + if (all(checks$checks$uses_dontrun == "all")) { + out$summary <- "All examples use `\\dontrun{}`." + out$print <- paste0( + "All of your functions' examples use `\\dontrun{}`:\n'", + out$print + ) } else { - # :eyes: when some examples use `\dontrun{}` - "Examples should not use `\\dontrun{{}}` unless really necessary." + out$summary <- "Examples should not use `\\dontrun{}` unless really necessary." + out$print <- paste0( + "The following functions have examples that use `\\dontrun{}`:\n'", + paste( + names(checks$checks$uses_dontrun[checks$checks$uses_dontrun != "none"]), + collapse = "', '" + ), + out$print + ) } - out$print <- paste0( - "The following functions have examples that use `\\dontrun{{}}`:\n'", - paste( - names(checks$checks$uses_dontrun[checks$checks$uses_dontrun]), - collapse = "', '" - ), - "'.\nConsider using `@examplesIf()` to conditionally run examples instead." - ) } return(out) } -uses_dontrun <- function(rd_files) { +rd_files_use_dontrun <- function(rd_files) { parsed_rds <- parse_rd_files(rd_files) # Only check Rds that actually have examples has_egs <- fns_have_exs(rd_files) - has_dontrun <- vapply( + vapply( parsed_rds[names(has_egs[has_egs])], - has_dontrun_examples, - FUN.VALUE = logical(1L) + rd_has_dontrun_examples, + FUN.VALUE = character(1L) ) - - has_dontrun } ## Utilities for parsing examples in Rd files @@ -92,13 +94,11 @@ parse_rd_files <- function(rd_files) { stats::setNames(rds, nms) } -has_dontrun_examples <- function(rd) { +rd_has_dontrun_examples <- function(rd) { ex <- get_Rd_section(rd, "examples") tags <- vapply(ex, function(exi) attr(exi, "Rd_tag"), character(1L)) - # Check if there are any \dontrun blocks - # Code that is not wrapped in \dontrun has the attribute Rd_tag set to "RCODE" # and is listed line-by-line in the examples section (i.e., each line has the # Rd_tag "RCODE" attribute). Code that that is wrapped in \dontrun has the @@ -124,14 +124,14 @@ has_dontrun_examples <- function(rd) { any_bare_r_code <- length(bare_r_code) > 0 any_dont_run <- length(dontrun_r_code) > 0 + # We could report how many lines are wrapped in \dontrun, but for now we just + # return a summary of whether there are any examples that use \dontrun. if (any_bare_r_code && any_dont_run) { - # If there is bare R code and also \dontrun, we consider it a dontrun example return("some") - } else if (any_dont_run) { - # If there is only \dontrun, we consider it a dontrun example + } else if (any_dont_run && !any_bare_r_code) { return("all") } else { - # No dontrun examples found + # Either no examples or no dontrun blocks return("none") } } From 303af089d874602e6231e16500ede5982c5f7980 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Wed, 27 Aug 2025 16:56:43 -0700 Subject: [PATCH 11/18] Test check_uses_dontrun and internal function --- inst/Rd-for-testing-dontrun.Rd | 45 +++++++++++++++++++++ tests/testthat/_snaps/check-uses-dontrun.md | 7 ++++ tests/testthat/test-check-uses-dontrun.R | 25 ++++++++++-- 3 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 inst/Rd-for-testing-dontrun.Rd create mode 100644 tests/testthat/_snaps/check-uses-dontrun.md diff --git a/inst/Rd-for-testing-dontrun.Rd b/inst/Rd-for-testing-dontrun.Rd new file mode 100644 index 00000000..faca846e --- /dev/null +++ b/inst/Rd-for-testing-dontrun.Rd @@ -0,0 +1,45 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/example-function.R +\name{example_function} +\alias{example_function} +\title{Example Function} +\usage{ +example_function(x, y = NULL) +} +\arguments{ +\item{x}{A numeric vector} + +\item{y}{An optional numeric vector} +} +\value{ +A numeric vector +} +\description{ +This is an example function for demonstration purposes. +} +\examples{ +\dontrun{ +# First example wrapped in dontrun +example_function(1:5) +result <- example_function(c(1, 2, 3)) +print(result) +} + +# Second example - not wrapped +example_function(10) +simple_result <- example_function(c(5, 6, 7)) + +\dontrun{ +# Third example wrapped in dontrun +complex_data <- rnorm(1000) +result <- example_function(complex_data) +plot(result) +} + +# third example - not wrapped, more complex +my_fun <- function(x) { + example_function(x) * 2 +} + +my_fun(1:10) +} diff --git a/tests/testthat/_snaps/check-uses-dontrun.md b/tests/testthat/_snaps/check-uses-dontrun.md new file mode 100644 index 00000000..cc3c4a55 --- /dev/null +++ b/tests/testthat/_snaps/check-uses-dontrun.md @@ -0,0 +1,7 @@ +# check examples dont use dontrun + + Code + ci_out$print + Output + [1] "All of your functions' examples use `\\dontrun{}`:\n''.\nConsider using `@examplesIf()` to conditionally run examples instead." + diff --git a/tests/testthat/test-check-uses-dontrun.R b/tests/testthat/test-check-uses-dontrun.R index 06c6e26f..e34448ef 100644 --- a/tests/testthat/test-check-uses-dontrun.R +++ b/tests/testthat/test-check-uses-dontrun.R @@ -14,23 +14,40 @@ test_that ("check examples dont use dontrun", { "All examples use `\\dontrun{}`." ) expect_length (ci_out$print, 1L) - expect_true (nzchar (ci_out$print)) + expect_snapshot (ci_out$print) # Some examples use `\dontrun{}` - checks$checks$uses_dontrun ["pkgstats_summary"] <- FALSE + checks$checks$uses_dontrun ["pkgstats_summary"] <- "some" ci_out <- output_pkgchk_uses_dontrun (checks) expect_false (ci_out$check_pass) expect_equal ( ci_out$summary, - "Examples should not use `\\dontrun{{}}` unless really necessary." + "Examples should not use `\\dontrun{}` unless really necessary." + ) + expect_match( + ci_out$print, + "The following functions have examples that use `\\dontrun{}`", + fixed = TRUE ) # No examples using `\dontrun{}` - checks$checks$uses_dontrun[] <- FALSE + checks$checks$uses_dontrun[] <- "none" ci_out <- output_pkgchk_uses_dontrun (checks) expect_true (ci_out$check_pass) expect_equal (ci_out$summary, "") expect_equal (ci_out$print, "") }) + +test_that ("uses_dontrun works", { + + rd_files <- system.file("Rd-for-testing-dontrun.Rd", package = "pkgcheck") + + uses_dontrun_output <- rd_files_use_dontrun (rd_files) + + expect_type (uses_dontrun_output, "character") + expect_length (uses_dontrun_output, 1L) + expect_named (uses_dontrun_output, "example_function") + expect_equal (uses_dontrun_output, c(example_function = "some")) +}) From af3095633138820856313eaacad2d8d5995838c6 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Wed, 27 Aug 2025 17:14:44 -0700 Subject: [PATCH 12/18] Set check_type to "none_watch" when some examples are dontrun --- R/check-uses-dontrun.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/R/check-uses-dontrun.R b/R/check-uses-dontrun.R index 7eab64b8..8aa95dca 100644 --- a/R/check-uses-dontrun.R +++ b/R/check-uses-dontrun.R @@ -32,6 +32,9 @@ output_pkgchk_uses_dontrun <- function(checks) { print = "" ) + # This is set up so that if all examples use dontrun, it is a failure (red X), + # if no examples use dontrun, it is a pass (green check), and if some examples + # use dontrun, it is a watch (:eyes:) if (!out$check_pass) { out$print <- "'.\nConsider using `@examplesIf()` to conditionally run examples instead." if (all(checks$checks$uses_dontrun == "all")) { @@ -50,6 +53,7 @@ output_pkgchk_uses_dontrun <- function(checks) { ), out$print ) + out$check_type <- "none_watch" } } From 0c32ba554b10be142c1679c884cb0d167a52f90f Mon Sep 17 00:00:00 2001 From: mark padgham Date: Tue, 22 Jul 2025 13:01:59 +0200 Subject: [PATCH 13/18] Reapply changes to summarize-checks from 51b2fbc239 --- R/summarise-checks.R | 187 ++++++++++++++++++++----------------------- 1 file changed, 88 insertions(+), 99 deletions(-) diff --git a/R/summarise-checks.R b/R/summarise-checks.R index 3bbfadee..431cc2e2 100644 --- a/R/summarise-checks.R +++ b/R/summarise-checks.R @@ -11,60 +11,57 @@ #' Summarise main checklist items for editor report #' @param checks Result of main \link{pkgcheck} function #' @noRd -summarise_all_checks <- function(checks) { - pkg_env <- asNamespace("pkgcheck") - pkg_fns <- ls(pkg_env) - - output_fns <- gsub( - "^output\\_pkgchk\\_", - "", - grep("^output\\_pkgchk\\_", pkg_fns, value = TRUE) +summarise_all_checks <- function (checks) { + + pkg_env <- asNamespace ("pkgcheck") + pkg_fns <- ls (pkg_env) + + output_fns <- gsub ( + "^output\\_pkgchk\\_", "", + grep ("^output\\_pkgchk\\_", pkg_fns, value = TRUE) ) - has_covr <- "covr" %in% names(checks$goodpractice) + has_covr <- "covr" %in% names (checks$goodpractice) if (!has_covr) { - output_fns <- output_fns[which(!grepl("covr", output_fns))] + output_fns <- output_fns [which (!grepl ("covr", output_fns))] } - ordered_checks <- order_checks(output_fns) - out <- lapply( + ordered_checks <- order_checks (output_fns) + out <- lapply ( ordered_checks, - function(i) summarise_check(checks, i, pkg_env) + function (i) summarise_check (checks, i, pkg_env) ) # "watch" checks; issue #144, #248 - watch_index <- vapply( - out, - function(i) { - ret <- c(FALSE, FALSE) - if (!is.null(i) && "check_type" %in% names(attributes(i))) { - a <- attr(i, "check_type") - ret <- c(grepl("^watch", a), grepl("\\_watch$", a)) - } - return(ret) - }, - logical(2L) - ) - index_tick <- which(watch_index[1, ]) - out[index_tick] <- - gsub("\\:heavy\\_check\\_mark:", ":eyes:", out[index_tick]) - index_cross <- which(watch_index[2, ]) - out[index_cross] <- - gsub("\\:heavy\\_multiplication\\_x\\:", ":eyes:", out[index_cross]) - - out <- do.call(c, out) + watch_index <- vapply (out, function (i) { + ret <- c (FALSE, FALSE) + if (!is.null (i) && "check_type" %in% names (attributes (i))) { + a <- attr (i, "check_type") + ret <- c (grepl ("^watch", a), grepl ("\\_watch$", a)) + } + return (ret) + }, logical (2L)) + index_tick <- which (watch_index [1, ]) + out [index_tick] <- + gsub ("\\:heavy\\_check\\_mark:", ":eyes:", out [index_tick]) + index_cross <- which (watch_index [2, ]) + out [index_cross] <- + gsub ("\\:heavy\\_multiplication\\_x\\:", ":eyes:", out [index_cross]) + + out <- do.call (c, out) # Ensure all :eyes: come last: - index_eyes <- grep("\\:eyes\\:", out) - index_other <- seq_along(out) - if (length(index_eyes) > 0L) { - index_other <- index_other[-index_eyes] + index_eyes <- grep ("\\:eyes\\:", out) + index_other <- seq_along (out) + if (length (index_eyes) > 0L) { + index_other <- index_other [-index_eyes] } - out <- c(out[index_other], out[index_eyes]) + out <- c (out [index_other], out [index_eyes]) + - out <- c(out, summarise_extra_env_checks(checks)) + out <- c (out, summarise_extra_env_checks (checks)) - gp <- summarise_gp_checks(checks) - if (any(grepl("^rcmd\\_", names(gp)))) { - out <- c( + gp <- summarise_gp_checks (checks) + if (any (grepl ("^rcmd\\_", names (gp)))) { + out <- c ( out, gp$rcmd_errs, gp$rcmd_warns @@ -72,24 +69,24 @@ summarise_all_checks <- function(checks) { } # re-order "watch" checks to bottom - index1 <- grep("\\:heavy\\_(multiplication\\_x|check\\_mark)\\:", out) - index2 <- grep("\\:eyes\\:", out) - out <- out[c(index1, index2)] + index1 <- grep ("\\:heavy\\_(multiplication\\_x|check\\_mark)\\:", out) + index2 <- grep ("\\:eyes\\:", out) + out <- out [c (index1, index2)] - checks_okay <- !any(grepl(symbol_crs(), out)) + checks_okay <- !any (grepl (symbol_crs (), out)) if (!checks_okay) { - out <- c( + out <- c ( out, "", - paste0( + paste0 ( "**Important:** All failing checks above ", "must be addressed prior to proceeding" ) ) } - if (any(grepl("\\:eyes\\:", out))) { - out <- c( + if (any (grepl ("\\:eyes\\:", out))) { + out <- c ( out, "", "(Checks marked with :eyes: may be optionally addressed.)", @@ -97,34 +94,34 @@ summarise_all_checks <- function(checks) { ) } - attr(out, "checks_okay") <- checks_okay + attr (out, "checks_okay") <- checks_okay - return(out) + return (out) } -summarise_extra_env_checks <- function(checks) { - extra_env <- options("pkgcheck_extra_env")[[1]] - if (is.null(extra_env)) { - return(NULL) +summarise_extra_env_checks <- function (checks) { + + extra_env <- options ("pkgcheck_extra_env") [[1]] + if (is.null (extra_env)) { + return (NULL) } - if (!is.list(extra_env)) { - extra_env <- list(extra_env) + if (!is.list (extra_env)) { + extra_env <- list (extra_env) } - extra_chks <- lapply(extra_env, function(e) { - e <- env2namespace(e) - output_fns <- grep("^output\\_pkgchk\\_", ls(e), value = TRUE) - output_fns <- gsub("^output\\_pkgchk\\_", "", output_fns) - output_fns <- output_fns[which(output_fns %in% names(checks$checks))] - vapply( - output_fns, - function(i) summarise_check(checks, i, e), - character(1), + extra_chks <- lapply (extra_env, function (e) { + e <- env2namespace (e) + output_fns <- grep ("^output\\_pkgchk\\_", ls (e), value = TRUE) + output_fns <- gsub ("^output\\_pkgchk\\_", "", output_fns) + output_fns <- output_fns [which (output_fns %in% names (checks$checks))] + vapply (output_fns, + function (i) summarise_check (checks, i, e), + character (1), USE.NAMES = FALSE ) }) - return(unlist(extra_chks)) + return (unlist (extra_chks)) } #' Function to specify the order in which checks appear in the summary method. @@ -134,8 +131,9 @@ summarise_extra_env_checks <- function(checks) { #' @return Modified version of input list with functions ordered in specified #' sequence. #' @noRd -order_checks <- function(fns) { - ord <- c( +order_checks <- function (fns) { + + ord <- c ( "pkgname", "license", "has_citation", @@ -164,27 +162,14 @@ order_checks <- function(fns) { # additionally explicitly listed below in `watch_checks()`: "obsolete_pkg_deps", "unique_fn_names", - "num_imports", - "uses_dontrun" + "num_imports" ) - fns <- fns[which(fns %in% ord)] - ord <- ord[which(ord %in% fns)] # b/c 'covr' is removed w/o gp - fns <- fns[match(ord, fns)] + fns <- fns [which (fns %in% ord)] + ord <- ord [which (ord %in% fns)] # b/c 'covr' is removed w/o gp + fns <- fns [match (ord, fns)] - return(fns) -} - -watch_checks <- function(output_fns) { - all_checks <- order_checks(output_fns) - watch_list <- c( - "obsolete_pkg_deps", - "unique_fn_names", - "num_imports", - "uses_dontrun" - ) - - all_checks[which(all_checks %in% watch_list)] + return (fns) } #' Generic function to summarise checks based on result of corresponding @@ -196,30 +181,34 @@ watch_checks <- function(output_fns) { #' @param pkg_env A namespace environment generated by `env2namespace`. #' @return Check formatted to apepar in `summary` method #' @noRd -summarise_check <- function(checks, what, pkg_env) { - pkg_fns <- ls(pkg_env) - summary_fn <- paste0("output_pkgchk_", what) +summarise_check <- function (checks, what, pkg_env) { + + pkg_fns <- ls (pkg_env) + summary_fn <- paste0 ("output_pkgchk_", what) if (!summary_fn %in% pkg_fns) { - return(NULL) + return (NULL) } - chk_summary <- do.call(summary_fn, list(checks), envir = pkg_env) + chk_summary <- do.call (summary_fn, list (checks), envir = pkg_env) res <- NULL - if (sum(nchar(chk_summary$summary)) > 0L) { - res <- paste0( + if (sum (nchar (chk_summary$summary)) > 0L) { + res <- paste0 ( "- ", - ifelse(chk_summary$check_pass, symbol_tck(), symbol_crs()), + ifelse (chk_summary$check_pass, + symbol_tck (), + symbol_crs () + ), " ", chk_summary$summary ) } - if (!is.null(res) && "check_type" %in% names(chk_summary)) { - attr(res, "check_type") <- chk_summary$check_type + if (!is.null (res) && "check_type" %in% names (chk_summary)) { + attr (res, "check_type") <- chk_summary$check_type } - return(res) + return (res) } From 22ae97ab81bb896ccac6c9786d1e01089d042634 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Thu, 28 Aug 2025 11:42:43 -0700 Subject: [PATCH 14/18] Add uses_dontrun to summarize_checks --- R/summarise-checks.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/summarise-checks.R b/R/summarise-checks.R index 431cc2e2..ec8b1c5c 100644 --- a/R/summarise-checks.R +++ b/R/summarise-checks.R @@ -162,7 +162,8 @@ order_checks <- function (fns) { # additionally explicitly listed below in `watch_checks()`: "obsolete_pkg_deps", "unique_fn_names", - "num_imports" + "num_imports", + "uses_dontrun" ) fns <- fns [which (fns %in% ord)] From e30cb2a43c8b6ab0d17b3471dd1472ad07fbf681 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Thu, 28 Aug 2025 11:43:02 -0700 Subject: [PATCH 15/18] One more test --- tests/testthat/test-check-uses-dontrun.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-check-uses-dontrun.R b/tests/testthat/test-check-uses-dontrun.R index e34448ef..df7555c0 100644 --- a/tests/testthat/test-check-uses-dontrun.R +++ b/tests/testthat/test-check-uses-dontrun.R @@ -25,6 +25,7 @@ test_that ("check examples dont use dontrun", { ci_out$summary, "Examples should not use `\\dontrun{}` unless really necessary." ) + expect_equal(ci_out$check_type, "none_watch") expect_match( ci_out$print, "The following functions have examples that use `\\dontrun{}`", From 7b132d20d3a060c9bf61c01426432950afaa83bc Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 29 Aug 2025 09:46:08 -0700 Subject: [PATCH 16/18] Update extra-checks snapshots --- tests/testthat/_snaps/extra-checks/checks-extra.html | 1 + tests/testthat/_snaps/extra-checks/checks-extra.md | 1 + tests/testthat/_snaps/extra-checks/checks-print.md | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/testthat/_snaps/extra-checks/checks-extra.html b/tests/testthat/_snaps/extra-checks/checks-extra.html index 790aa35b..713f9409 100644 --- a/tests/testthat/_snaps/extra-checks/checks-extra.html +++ b/tests/testthat/_snaps/extra-checks/checks-extra.html @@ -33,6 +33,7 @@

Checks for pkgst
  • ❌ Package contains unexpected files.
  • ❌ Default GitHub branch of ‘master’ is not acceptable.
  • ✅ This is a statistical package which complies with all applicable standards
    • +
  • ❌ All examples use \dontrun{}.
  • 👀 Some goodpractice linters failed.
  • 👀 Package depends on the following obsolete packages: [blah,sp]
    • diff --git a/tests/testthat/_snaps/extra-checks/checks-extra.md b/tests/testthat/_snaps/extra-checks/checks-extra.md index 7d666baa..61dd84ed 100644 --- a/tests/testthat/_snaps/extra-checks/checks-extra.md +++ b/tests/testthat/_snaps/extra-checks/checks-extra.md @@ -15,6 +15,7 @@ git hash: [](https://github.com/ropensci-review-tools/pkgstats/tree/) - :heavy_multiplication_x: Package contains unexpected files. - :heavy_multiplication_x: Default GitHub branch of 'master' is not acceptable. - :heavy_check_mark: This is a statistical package which complies with all applicable standards +- :heavy_multiplication_x: All examples use `\dontrun{}`. - :eyes: Some goodpractice linters failed. - :eyes: Package depends on the following obsolete packages: [blah,sp] diff --git a/tests/testthat/_snaps/extra-checks/checks-print.md b/tests/testthat/_snaps/extra-checks/checks-print.md index 04c9b4d1..9c453d42 100644 --- a/tests/testthat/_snaps/extra-checks/checks-print.md +++ b/tests/testthat/_snaps/extra-checks/checks-print.md @@ -14,6 +14,7 @@ v Package has continuous integration checks, but no badges on README x Package contains unexpected files. x Default GitHub branch of 'master' is not acceptable. v This is a statistical package which complies with all applicable standards +x All examples use `\dontrun`. i Some goodpractice linters failed. i Package depends on the following obsolete packages: [blah,sp] From cec2b9e42d697fc962f60f4c904f5bc7e815cde4 Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Fri, 29 Aug 2025 11:01:17 -0700 Subject: [PATCH 17/18] Override uses_dontrun test result in test-extra-checks --- tests/testthat/test-extra-checks.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-extra-checks.R b/tests/testthat/test-extra-checks.R index 0a662eb3..f595f77d 100644 --- a/tests/testthat/test-extra-checks.R +++ b/tests/testthat/test-extra-checks.R @@ -90,6 +90,7 @@ test_that ("extra checks", { ex [which (!ex)] <- TRUE checks$checks$fns_have_exs <- ex checks$checks$branch_is_master <- FALSE + checks$checks$uses_dontrun[] <- "none" set.seed (1L) x <- capture.output (summary (checks), type = "message") From e5ec60dcee1ee5b50bf02ff9d0fbd6abd1a47c8d Mon Sep 17 00:00:00 2001 From: Andy Teucher Date: Tue, 2 Sep 2025 09:07:10 -0700 Subject: [PATCH 18/18] Add Andy as an author --- DESCRIPTION | 10 ++++++---- man/pkgcheck-package.Rd | 1 + 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index fbe4bc83..3b375bcd 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -7,12 +7,14 @@ Authors@R: c( person("Maëlle", "Salmon", role = "aut"), person("Jacob", "Wujciak-Jens", , "jacob@wujciak.de", role = "aut", comment = c(ORCID = "0000-0002-7281-3989")), - person(c("Kelli", "F."), "Johnson", , "kelli.johnson@noaa.gov", - role = "ctb", comment = c(ORCID = "0000-0002-5149-451X")), + person(c("Kelli", "F."), "Johnson", , "kelli.johnson@noaa.gov", role = "ctb", + comment = c(ORCID = "0000-0002-5149-451X")), person("Eunseop", "Kim", , "markean@pm.me", role = "aut", comment = c(ORCID = "0009-0000-2138-788X")), - person("Katrina", "Brock", , "katrinabrock266@gmail.com", - role = "ctb", comment = c(ORCID = "0009-0003-4678-9545")) + person("Katrina", "Brock", , "katrinabrock266@gmail.com", role = "ctb", + comment = c(ORCID = "0009-0003-4678-9545")), + person("Andy", "Teucher", , "andy.teucher@gmail.com", role = "aut", + comment = c(ORCID = "0000-0002-7840-692X")) ) Description: Check whether a package is ready for submission to rOpenSci's peer review system. diff --git a/man/pkgcheck-package.Rd b/man/pkgcheck-package.Rd index 6fad6873..31cbe8aa 100644 --- a/man/pkgcheck-package.Rd +++ b/man/pkgcheck-package.Rd @@ -24,6 +24,7 @@ Authors: \item Maëlle Salmon \item Jacob Wujciak-Jens \email{jacob@wujciak.de} (\href{https://orcid.org/0000-0002-7281-3989}{ORCID}) \item Eunseop Kim \email{markean@pm.me} (\href{https://orcid.org/0009-0000-2138-788X}{ORCID}) + \item Andy Teucher \email{andy.teucher@gmail.com} (\href{https://orcid.org/0000-0002-7840-692X}{ORCID}) } Other contributors: