From 52bc8d7be3c0ade50aed69c03d346d8f20c4f2a4 Mon Sep 17 00:00:00 2001 From: Aron Atkins Date: Fri, 17 Jul 2026 14:42:54 -0400 Subject: [PATCH 1/2] chore: remove superassignment operator and nolint Add direct test coverage to prove compatibility. --- DESCRIPTION | 2 +- R/aaa-globals.R | 5 +-- man/packrat-options.Rd | 2 -- tests/testthat/test-globals.R | 60 +++++++++++++++++++++++++++++++++++ tests/testthat/test-hash.R | 2 +- 5 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 tests/testthat/test-globals.R diff --git a/DESCRIPTION b/DESCRIPTION index 8e217205..bd04ff3f 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -33,4 +33,4 @@ Suggests: withr Config/testthat/edition: 3 Encoding: UTF-8 -RoxygenNote: 7.3.2 +Config/roxygen2/version: 8.0.0 diff --git a/R/aaa-globals.R b/R/aaa-globals.R index 1a6c9be1..ebacc0ca 100644 --- a/R/aaa-globals.R +++ b/R/aaa-globals.R @@ -11,6 +11,7 @@ merge_list <- function(x, y) { } new_defaults <- function(value = list()) { + env <- environment() defaults <- value get <- function(name, default = FALSE, drop = TRUE) { @@ -35,11 +36,11 @@ new_defaults <- function(value = list()) { if (is.null(names(dots)) && length(dots) == 1 && is.list(dots[[1]])) { if (length(dots <- dots[[1]]) == 0) return() } - defaults <<- merge(dots) # nolint + assign("defaults", merge(dots), envir = env) invisible(NULL) } merge <- function(values) merge_list(defaults, values) - restore <- function(target = value) defaults <<- target # nolint + restore <- function(target = value) assign("defaults", target, envir = env) list(get = get, set = set, merge = merge, restore = restore) } diff --git a/man/packrat-options.Rd b/man/packrat-options.Rd index 09289146..beb98ba5 100644 --- a/man/packrat-options.Rd +++ b/man/packrat-options.Rd @@ -1,6 +1,5 @@ % Generated by roxygen2: do not edit by hand % Please edit documentation in R/options.R -\docType{data} \name{packrat-options} \alias{packrat-options} \alias{get_opts} @@ -115,4 +114,3 @@ packrat::opts$external.packages() packrat::opts$external.packages(c("devtools", "knitr")) } } -\keyword{datasets} diff --git a/tests/testthat/test-globals.R b/tests/testthat/test-globals.R new file mode 100644 index 00000000..906eb640 --- /dev/null +++ b/tests/testthat/test-globals.R @@ -0,0 +1,60 @@ +test_that("new_defaults get returns values", { + d <- new_defaults(list(a = 1, b = 2)) + + # entire list when name is missing + expect_identical(d$get(), list(a = 1, b = 2)) + + # single value, dropped by default + expect_identical(d$get("a"), 1) + + # named subset when drop = FALSE + expect_identical(d$get("a", drop = FALSE), list(a = 1)) + expect_identical(d$get(c("a", "b")), list(a = 1, b = 2)) + + # default = TRUE ignores anything set later + d$set(a = 99) + expect_identical(d$get("a"), 99) + expect_identical(d$get("a", default = TRUE), 1) +}) + +test_that("new_defaults set merges named arguments", { + d <- new_defaults(list(a = 1, b = 2)) + + d$set(a = 10, c = 3) + expect_identical(d$get(), list(a = 10, b = 2, c = 3)) +}) + +test_that("new_defaults set merges a single list argument", { + d <- new_defaults(list(a = 1, b = 2)) + + d$set(list(a = 10, c = 3)) + expect_identical(d$get(), list(a = 10, b = 2, c = 3)) +}) + +test_that("new_defaults set with no arguments leaves defaults untouched", { + d <- new_defaults(list(a = 1)) + + expect_null(d$set()) + expect_null(d$set(list())) + expect_identical(d$get(), list(a = 1)) +}) + +test_that("new_defaults restore replaces all defaults", { + d <- new_defaults(list(a = 1, b = 2)) + d$set(a = 10) + + d$restore(list(z = 99)) + expect_identical(d$get(), list(z = 99)) + + # restoring with no target reverts to the original value + d$restore() + expect_identical(d$get(), list(a = 1, b = 2)) +}) + +test_that("new_defaults merge does not mutate stored defaults", { + d <- new_defaults(list(a = 1, b = 2)) + + expect_identical(d$merge(list(a = 10, c = 3)), list(a = 10, b = 2, c = 3)) + # merge is non-destructive + expect_identical(d$get(), list(a = 1, b = 2)) +}) diff --git a/tests/testthat/test-hash.R b/tests/testthat/test-hash.R index 29ac8ed9..47b8baa3 100644 --- a/tests/testthat/test-hash.R +++ b/tests/testthat/test-hash.R @@ -18,7 +18,7 @@ test_that("hash function is available and has expected arguments", { expect_identical( formals(hash), pairlist( - path = quote(expr = ), # nolint: infix_spaces_linter. r-lib/lintr#1889 + path = quote(expr = ), descLookup = as.name("installedDescLookup") ) ) From b08bb1356c55a05e23bcbc9ba07bc55229f1d0b8 Mon Sep 17 00:00:00 2001 From: Aron Atkins Date: Fri, 17 Jul 2026 14:52:46 -0400 Subject: [PATCH 2/2] fix: correct origLibPaths lookup Discovered while auditing .packrat_mutables references. As of 3c87034, the state lived directly on the `.packrat` environment. With 5108287, the state moved into a `new_defaults()` object, `.packrat_mutables`, which requires `$get("field")`. With cd77393, the `.packrat_mutables$origLibPaths` reference was added, mixing the old and NEW styles. The `.packrat_mutables$origLibPaths` has always resolved to `NULL` and was likely not noticed because `origLibPaths` frequently contains overlapping entries. Includes a test confirming the use of origLibPaths. --- R/dependencies.R | 2 +- tests/testthat/test-dependencies.R | 41 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/R/dependencies.R b/R/dependencies.R index 110e02c1..19ca86ce 100644 --- a/R/dependencies.R +++ b/R/dependencies.R @@ -52,7 +52,7 @@ appDependencies <- function( libPaths <- c( libDir(project), .libPaths(), - .packrat_mutables$origLibPaths + .packrat_mutables$get("origLibPaths") ) ignores <- packrat::opts$ignored.packages() diff --git a/tests/testthat/test-dependencies.R b/tests/testthat/test-dependencies.R index 382c303c..369db865 100644 --- a/tests/testthat/test-dependencies.R +++ b/tests/testthat/test-dependencies.R @@ -243,3 +243,44 @@ withTestContext({ expect_equal(deps, c("oatmeal")) }) }) + +test_that("appDependencies searches origLibPaths when resolving dependencies", { + # A library holding an installed package (origlibpkg) whose dependency + # (origlibdep) can only be discovered by reading origlibpkg's DESCRIPTION + # from this library. + origLib <- tempfile("origlib") + installFakePkg <- function(name, imports = NULL) { + pkgDir <- file.path(origLib, name) + dir.create(pkgDir, recursive = TRUE) + desc <- c(paste("Package:", name), "Version: 1.0.0") + if (!is.null(imports)) { + desc <- c(desc, paste("Imports:", imports)) + } + writeLines(desc, file.path(pkgDir, "DESCRIPTION")) + } + installFakePkg("origlibpkg", imports = "origlibdep") + installFakePkg("origlibdep") + + # Point origLibPaths at that library, restoring afterwards. + oldOrigLibPaths <- packrat:::.packrat_mutables$get("origLibPaths") + packrat:::.packrat_mutables$set(origLibPaths = origLib) + on.exit( + packrat:::.packrat_mutables$set(origLibPaths = oldOrigLibPaths), + add = TRUE + ) + + # A project whose only stated dependency is origlibpkg. + projDir <- tempfile("proj") + dir.create(projDir) + writeLines("library(origlibpkg)", file.path(projDir, "code.R")) + + # An empty package database leaves origLib as the only place origlibpkg's + # dependencies can be resolved. + deps <- packrat:::appDependencies( + projDir, + available.packages = matrix(nrow = 0, ncol = 0), + implicit.packrat.dependency = FALSE + ) + + expect_true("origlibdep" %in% deps) +})