From aa6a3cbee35a491e6e0194c33638634b43e70968 Mon Sep 17 00:00:00 2001 From: bburns632 Date: Mon, 31 Aug 2026 22:25:00 -0500 Subject: [PATCH] fix: determine exports from namespace metadata, not attached env (#347) FunctionReporter listed `ls("package:")` after force-attaching the package with `require()`. devtools::load_all() copies the entire namespace into that attached environment, so every function looked exported. Use `getNamespaceExports()` on the namespace the reporter already loads. This also means pkgnet no longer attaches the package it is analyzing as a side effect. The Windows covr workaround unconditionally detached `package:`, which only worked because `require()` had attached it. Guard the detach on the package actually being on the search path, and condition the re-attach on the same flag. Co-Authored-By: Claude Opus 5 (1M context) --- DESCRIPTION | 1 + NEWS.md | 1 + R/FunctionReporter.R | 19 ++++++------- tests/testthat/test-FunctionReporter-class.R | 29 ++++++++++++++++++++ 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 7a263d0..9c6a898 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -31,6 +31,7 @@ Imports: Suggests: ggplot2, pkgdown, + pkgload, testthat, webshot, withr diff --git a/NEWS.md b/NEWS.md index 569985c..e4443e0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ ## CHANGES ## BUGFIXES +* `FunctionReporter` now determines exported functions from the package namespace's export metadata via `getNamespaceExports()` instead of listing the attached `package:` environment. Previously, running pkgnet after `devtools::load_all()` marked every function as exported. (#347 Thanks @hughjonesd!) # pkgnet 0.6.1 ## NEW FEATURES diff --git a/R/FunctionReporter.R b/R/FunctionReporter.R index 3a8fc96..9ea6e10 100644 --- a/R/FunctionReporter.R +++ b/R/FunctionReporter.R @@ -147,8 +147,10 @@ FunctionReporter <- R6::R6Class( log_info(sprintf("Calculating test coverage for %s...", self$pkg_name)) # workaround for covr conflict with loaded packages on windows - if(.Platform$OS.type == "windows") { - detach(paste0('package:',self$pkg_name), unload = TRUE, character.only = TRUE) + pkg_search_name <- paste0('package:', self$pkg_name) + reattach_pkg <- .Platform$OS.type == "windows" && pkg_search_name %in% search() + if (reattach_pkg) { + detach(pkg_search_name, unload = TRUE, character.only = TRUE) } pkgCovDT <- data.table::as.data.table(covr::package_coverage( @@ -158,7 +160,7 @@ FunctionReporter <- R6::R6Class( )) # workaround for covr conflict with loaded packages on windows - if(.Platform$OS.type == "windows") { + if (reattach_pkg) { attachNamespace(self$pkg_name) } @@ -236,13 +238,10 @@ FunctionReporter <- R6::R6Class( ) # Figure out which functions are exported - # We need the package to be loaded first - suppressPackageStartupMessages({ - require(self$pkg_name - , lib.loc = .libPaths()[1] - , character.only = TRUE) - }) - exported_obj_names <- ls(sprintf("package:%s", self$pkg_name)) + # Use the namespace's export metadata rather than the contents of the + # attached package environment. devtools::load_all() can attach + # unexported objects, which would otherwise look exported. + exported_obj_names <- getNamespaceExports(pkg_env) nodes[, isExported := node %in% exported_obj_names] # Check if we have R6 functions diff --git a/tests/testthat/test-FunctionReporter-class.R b/tests/testthat/test-FunctionReporter-class.R index d5d40d2..007eaca 100644 --- a/tests/testthat/test-FunctionReporter-class.R +++ b/tests/testthat/test-FunctionReporter-class.R @@ -390,3 +390,32 @@ test_that("FunctionReporter R6 edge extraction handles case where all methods ha TARGET = c("couplet_2", "couplet_1"),key = c("SOURCE","TARGET"))) }) + + +test_that("FunctionReporter determines exports from the namespace, not the attached environment", { + + testthat::skip_if_not_installed("pkgload") + + # Start from a clean slate so load_all() does not have to patch an + # already-loaded namespace + if (isNamespaceLoaded("silverstein")) { + unloadNamespace("silverstein") + } + + # devtools::load_all() copies the whole namespace into package:, so + # listing that environment makes every function look exported (#347) + pkgload::load_all( + path = system.file("silverstein", package = "pkgnet") + , quiet = TRUE + ) + on.exit(pkgload::unload("silverstein"), add = TRUE) + + # couplet_1 is not in silverstein's NAMESPACE, but load_all() attaches it + expect_true("couplet_1" %in% ls("package:silverstein")) + + testObj <- FunctionReporter$new()$set_package('silverstein') + + expect_false(testObj$nodes[node == "couplet_1", isExported]) + expect_false(testObj$nodes[node == "couplet_2", isExported]) + expect_true(testObj$nodes[node == "Carrots$public_methods$initialize", isExported]) +})