fix: isExported is always TRUE after devtools::load_all() (#347)

DESCRIPTION

@@ -31,6 +31,7 @@ Imports:
 Suggests:
     ggplot2,
     pkgdown,
+    pkgload,
     testthat,
     webshot,
     withr

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:<pkg>` environment. Previously, running pkgnet after `devtools::load_all()` marked every function as exported. (#347 Thanks @hughjonesd!)
 
 # pkgnet 0.6.1
 ## NEW FEATURES

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

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:<pkg>, 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])
+})