Skip to content

Commit 7f6fb66

Browse files
fix(#135): evaluate @examples extraction without inline-R side effects (#136)
* fix(#135): evaluate @examples extraction without inline-R side effects `att_from_examples()` delegated example extraction to `roxygen2::parse_file()`, which unconditionally evaluates inline R embedded in roxygen markdown tags such as ``@param x `r helper("x")` ``. roxygen2 evaluates that R in the env returned by `roxy_meta_get("env")`, which falls back to `baseenv()` outside a full `roxygenise()` context; any package-local helper is therefore unresolvable, producing a cascade of "could not find function" messages on every affected tag. Replace the parse_file call with a focused regex extractor that pulls `@examples` / `@examplesIf` blocks straight from the source. Example detection no longer touches roxygen2's tokenizer, so inline R inside unrelated tags is left strictly alone. NAMESPACE auto-regen drops the now-unused `roxygen2::parse_file` and `roxygen2::block_get_tag_value` imports. Bumps version to 1.0.1 with a NEWS entry. * fix(#135): mirror att_from_rscript() defensive read pattern Address Copilot review on PR #136: - `extract_examples_lines()` was hard-coding `encoding = "UTF-8"` and bare-calling `readLines()`. The sibling `att_from_rscript()` uses `getOption("encoding")` and a `tryCatch()` that warns + returns `character(0)` on read failure. Mirror that pattern so a single unreadable source file no longer aborts the whole `att_from_examples()` scan, and so non-UTF8 locales (Windows) are respected by default. - Thread an `encoding` argument through `att_from_examples()` to expose the same knob users already have on `att_from_rscript()`. - NEWS: clarify that roxygen2's inline-R fallback env is `baseenv()` — base R is reachable, only package-local helpers are not. * fix(#135): handle @examplesIf condition + DCF blank line in test fixture Address Copilot's second review on PR #136: - `extract_examples_lines()` was including the payload of `@examplesIf` (a guard condition like `requireNamespace("foo")`) in the extracted example text, so any package mentioned only in the guard was falsely detected as a dependency. Split the tag detection so `@examplesIf` enters example mode but discards the condition; only the body lines that follow are captured. - Add a focused test asserting the @examplesIf path: the body's deps must be detected, the guard's must not. - The #135 regression test appended `Roxygen: list(markdown = TRUE)` to the dummypackage DESCRIPTION via `c(readLines(...), new_line)`, which preserved the file's trailing blank line and inserted the new field after a blank record separator. DCF parsers may then ignore it. Trim trailing empty lines first. --------- Co-authored-by: Vincent Guyader <10470699+VincentGuyader@users.noreply.github.com>
1 parent 8f084cd commit 7f6fb66

6 files changed

Lines changed: 198 additions & 14 deletions

File tree

DESCRIPTION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Package: attachment
22
Title: Deal with Dependencies
3-
Version: 1.0.0
3+
Version: 1.0.1
44
Authors@R: c(
55
person("Vincent", "Guyader", , "vincent@thinkr.fr", role = c("cre", "aut"),
66
comment = c(ORCID = "0000-0003-0671-9270")),

NAMESPACE

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ importFrom(glue,glue)
2929
importFrom(glue,glue_collapse)
3030
importFrom(knitr,purl)
3131
importFrom(magrittr,"%>%")
32-
importFrom(roxygen2,block_get_tag_value)
33-
importFrom(roxygen2,parse_file)
3432
importFrom(roxygen2,roxygenise)
3533
importFrom(stats,na.omit)
3634
importFrom(stats,setNames)

NEWS.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
# attachment 1.0.1
2+
3+
## Bug fixes
4+
5+
- `att_from_examples()` (and therefore `att_amend_desc()`) no longer chokes
6+
on inline R inside roxygen2 markdown tags such as
7+
`@param x \`r helper("x")\`` when `helper()` is a package-local function.
8+
Previously, `att_from_examples()` delegated to `roxygen2::parse_file()`,
9+
which evaluated inline R against `baseenv()` (roxygen2's fallback when
10+
no package env is registered) — base R is reachable, but package-local
11+
helpers are not, so each affected tag emitted
12+
`could not find function "helper"`.
13+
Example detection now uses a focused regex over the source instead of
14+
the full roxygen2 tokenizer (#135).
15+
116
# attachment 1.0.0
217

318
## Detection — new foundation

R/add_from_examples.R

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,29 @@
11
#' Get all packages called in examples from R files
22
#'
33
#' @param dir.r path to directory with R scripts.
4+
#' @param encoding Encoding passed to [readLines()] when reading source files.
5+
#' Defaults to `getOption("encoding")` so the system locale is respected,
6+
#' matching [att_from_rscript()].
47
#'
58
#' @return Character vector of packages called with library or require.
6-
#' @importFrom roxygen2 parse_file block_get_tag_value
79
#' @examples
810
#' dummypackage <- system.file("dummypackage",package = "attachment")
911
#'
1012
#' # browseURL(dummypackage)
1113
#' att_from_examples(dir.r = file.path(dummypackage,"R"))
1214

1315
#' @export
14-
att_from_examples <- function(dir.r = "R") {
16+
att_from_examples <- function(dir.r = "R", encoding = getOption("encoding")) {
1517
rfiles <- list.files(dir.r, full.names = TRUE, pattern = "\\.r$", ignore.case = TRUE,recursive = FALSE)
1618

1719
roxy_file <- tempfile("roxy.examples", fileext = ".R")
1820

19-
all_examples <- unlist(lapply(rfiles, function(the_file) {
20-
file_roxytags <- roxygen2::parse_file(the_file)
21-
res <- unlist(
22-
lapply(file_roxytags,
23-
function(x) roxygen2::block_get_tag_value(block = x, tag = "examples"))
24-
)
25-
res
26-
}))
21+
# Extract @examples / @examplesIf blocks via regex on the source instead of
22+
# roxygen2::parse_file(). parse_file() unconditionally evaluates inline R
23+
# found in roxygen markdown (e.g. `@param x \`r helper("x")\``), and the
24+
# evaluation env defaults to baseenv() outside a full roxygenise() context,
25+
# which makes any package-local helper unresolvable (issue #135).
26+
all_examples <- unlist(lapply(rfiles, extract_examples_lines, encoding = encoding))
2727
# Clean \dontrun and \donttest, and replace with '{' on next line
2828
all_examples_clean <-
2929
gsub(pattern = "\\\\dontrun\\s*\\{|\\\\donttest\\s*\\{", replacement = "#ICI\n{", x = all_examples)
@@ -43,3 +43,57 @@ att_from_examples <- function(dir.r = "R") {
4343

4444
return(all_deps)
4545
}
46+
47+
# Extract @examples / @examplesIf blocks from a single R source file, returning
48+
# the example code with the leading `#' ` removed. Mirrors the relevant subset
49+
# of roxygen2::parse_file() + block_get_tag_value(tag = "examples") behaviour
50+
# without triggering inline R evaluation of @param markdown.
51+
#
52+
# `@examplesIf` is treated specially: the payload on the tag line is a guard
53+
# *condition*, not example code, so it is discarded — including the condition
54+
# in the extracted text would cause packages mentioned only in the guard to be
55+
# falsely picked up as dependencies. The example body that follows is captured
56+
# normally.
57+
extract_examples_lines <- function(rfile, encoding = getOption("encoding")) {
58+
lines <- tryCatch(
59+
readLines(rfile, warn = FALSE, encoding = encoding),
60+
error = function(e) {
61+
warning(
62+
sprintf("Could not read R script '%s': %s", rfile, conditionMessage(e)),
63+
call. = FALSE
64+
)
65+
character(0)
66+
}
67+
)
68+
if (length(lines) == 0) return(character(0))
69+
70+
is_roxy <- grepl("^\\s*#'", lines)
71+
is_tag <- grepl("^\\s*#'\\s*@", lines)
72+
is_example_tag <- grepl("^\\s*#'\\s*@examples\\b", lines)
73+
is_examplesif_tag <- grepl("^\\s*#'\\s*@examplesIf\\b", lines)
74+
75+
out <- character()
76+
in_example <- FALSE
77+
for (i in seq_along(lines)) {
78+
if (is_examplesif_tag[i]) {
79+
in_example <- TRUE
80+
next
81+
}
82+
if (is_example_tag[i]) {
83+
in_example <- TRUE
84+
first_payload <- sub("^\\s*#'\\s*@examples\\s*", "", lines[i])
85+
if (nzchar(first_payload)) {
86+
out <- c(out, first_payload)
87+
}
88+
next
89+
}
90+
if (in_example) {
91+
if (!is_roxy[i] || is_tag[i]) {
92+
in_example <- FALSE
93+
next
94+
}
95+
out <- c(out, sub("^\\s*#'\\s?", "", lines[i]))
96+
}
97+
}
98+
out
99+
}

man/att_from_examples.Rd

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/testthat/test-att_from_namespace.R

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,3 +127,116 @@ test_that("bad namespace can be corrected", {
127127
unlink(dummypackage, recursive = TRUE)
128128
unlink(tmpdir, recursive = TRUE)
129129

130+
# Issue #135: inline R in @param must resolve package-local functions ----
131+
tmpdir <- tempfile(pattern = "pkginline")
132+
dir.create(tmpdir)
133+
file.copy(
134+
system.file("dummypackage", package = "attachment"), tmpdir,
135+
recursive = TRUE)
136+
dummypackage <- file.path(tmpdir, "dummypackage")
137+
138+
# Enable roxygen markdown so inline R inside @param is actually evaluated.
139+
# Strip trailing blank lines from the dummypackage DESCRIPTION before
140+
# appending: in DCF format a blank line starts a new record, so an
141+
# unstripped append would silently produce a second record and tooling may
142+
# ignore the new field.
143+
desc_file <- file.path(dummypackage, "DESCRIPTION")
144+
desc_lines <- readLines(desc_file)
145+
while (length(desc_lines) > 0 && !nzchar(desc_lines[length(desc_lines)])) {
146+
desc_lines <- desc_lines[-length(desc_lines)]
147+
}
148+
writeLines(c(desc_lines, "Roxygen: list(markdown = TRUE)"), desc_file)
149+
150+
# Add an exported helper that the @param will call inline
151+
helper_file <- file.path(dummypackage, "R", "helper.R")
152+
writeLines(
153+
c(
154+
"#' Return a doc snippet",
155+
"#' @param name a name",
156+
"#' @return a string",
157+
"#' @export",
158+
"helper <- function(name) {",
159+
" paste('a', name, 'parameter')",
160+
"}"
161+
),
162+
helper_file
163+
)
164+
165+
# Rewrite my_mean.R so its @param uses inline R calling the package-local helper
166+
my_mean_file <- file.path(dummypackage, "R", "my_mean.R")
167+
writeLines(
168+
c(
169+
"#' my_mean",
170+
"#'",
171+
"#' @param x `r helper(\"x\")`",
172+
"#'",
173+
"#' @export",
174+
"#' @importFrom magrittr %>%",
175+
"my_mean <- function(x){",
176+
" x <- x %>% stats::na.omit()",
177+
" sum(x) / base::length(x)",
178+
"}"
179+
),
180+
my_mean_file
181+
)
182+
183+
test_that("inline R in @param resolves package-local functions (#135)", {
184+
unlink(file.path(dummypackage, "man"), recursive = TRUE)
185+
186+
captured <- character()
187+
withCallingHandlers(
188+
att_amend_desc(dummypackage),
189+
message = function(m) {
190+
captured <<- c(captured, conditionMessage(m))
191+
invokeRestart("muffleMessage")
192+
},
193+
warning = function(w) {
194+
captured <<- c(captured, conditionMessage(w))
195+
invokeRestart("muffleWarning")
196+
}
197+
)
198+
199+
expect_false(
200+
any(grepl("could not find function", captured, fixed = TRUE)),
201+
info = paste0(
202+
"Inline R in @param failed to resolve `helper()`. Captured conditions:\n",
203+
paste(captured, collapse = "\n")
204+
)
205+
)
206+
207+
rd_file <- file.path(dummypackage, "man", "my_mean.Rd")
208+
expect_true(file.exists(rd_file))
209+
rd_content <- paste(readLines(rd_file), collapse = "\n")
210+
expect_match(rd_content, "a x parameter", fixed = TRUE)
211+
})
212+
213+
unlink(tmpdir, recursive = TRUE)
214+
215+
# @examplesIf condition payload must NOT be scanned for dependencies (#136 review) ----
216+
tmpdir <- tempfile(pattern = "pkgexamplesif")
217+
dir.create(tmpdir)
218+
ifpkg_dir <- file.path(tmpdir, "Rscripts")
219+
dir.create(ifpkg_dir)
220+
ifpkg_file <- file.path(ifpkg_dir, "fun.R")
221+
writeLines(
222+
c(
223+
"#' fun",
224+
"#' @param x a value",
225+
"#' @return x",
226+
"#' @export",
227+
"#' @examplesIf requireNamespace(\"fakeguardpkg\", quietly = TRUE)",
228+
"#' library(magrittr)",
229+
"#' x %>% identity()",
230+
"fun <- function(x) x"
231+
),
232+
ifpkg_file
233+
)
234+
235+
test_that("att_from_examples ignores @examplesIf condition for dep detection (#136 review)", {
236+
deps <- att_from_examples(dir.r = ifpkg_dir)
237+
expect_true("magrittr" %in% deps)
238+
expect_false("fakeguardpkg" %in% deps)
239+
})
240+
241+
unlink(tmpdir, recursive = TRUE)
242+

0 commit comments

Comments
 (0)