From aee0cda460211bdc3886f8fd185edb8eefe3d153 Mon Sep 17 00:00:00 2001 From: Jay Qi Date: Fri, 20 Feb 2026 01:39:15 -0500 Subject: [PATCH 1/8] Add check for externalptr; add tryCatch on as.list --- R/FunctionReporter.R | 56 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/R/FunctionReporter.R b/R/FunctionReporter.R index 146f59e1..f320c91b 100644 --- a/R/FunctionReporter.R +++ b/R/FunctionReporter.R @@ -1,12 +1,12 @@ #' Function Interdependency Reporter -#' +#' #' @description #' This reporter looks at the network of interdependencies of its #' defined functions. Measures of centrality from graph theory can indicate #' which function is most important to a package. Combined with unit test #' coverage information---also provided by this reporter--- it can be used #' as a powerful tool to prioritize test writing. -#' +#' #' @details #' \subsection{R6 Method Support:}{ #' R6 classes are supported, with their methods treated as functions by the @@ -396,9 +396,9 @@ FunctionReporter <- R6::R6Class( .parse_function <- function (x) { # If expression x is not an atomic value or symbol (i.e., name of object) or # an environment pointer then we can break x up into list of components - listable <- (!is.atomic(x) && !is.symbol(x) && !is.environment(x)) + listable <- .is_listable_expr(x) if (!is.list(x) && listable) { - x <- as.list(x) + x <- .try_list(x) if (length(x) > 0){ # Check for expression of the form foo$bar @@ -410,21 +410,21 @@ FunctionReporter <- R6::R6Class( } } else { # make empty lists "not listable" so recursion stops - listable <- FALSE + listable <- FALSE } } if (listable){ - + # If do.call and first argument is string (atomic), covert to call if (length(x) >= 2){ if (deparse(x[[1]])[1] == "do.call" & is.character(x[[2]])){ x[[2]] <- parse(text=x[[2]]) } } - + # Filter out atomic values because we don't care about them x <- Filter(f = Negate(is.atomic), x = x) @@ -439,6 +439,40 @@ FunctionReporter <- R6::R6Class( return(out) } +# [description] check if expression can be expanded into a list of components +.is_listable_expr <- function(x) { + # Atomic value + if (is.atomic(x)){return(FALSE)} + # Symbol (i.e., name of object) + if (is.symbol(x)){return(FALSE)} + # Environment + if (!is.environment(x)){return(FALSE)} + # Raw external pointer to non-R memory/state (e.g., for C/C++ code) + if (typeof(x) == "externalptr"){return(FALSE)} + + return(TRUE) +} + +# [description] +.try_list <- function(x) { + tryCatch( + as.list(x), + error = function(e) { + log_warn(sprintf( + paste0( + ".parse_function: as.list() failed for ", + "typeof=%s class=%s; treating as unlistable. Error: %s", + ), + typeof(x), + paste(class(x), collapse = ","), + conditionMessage(e) + )) + listable <<- FALSE + return(x) + } + ) +} + # [description] given an R6 class, returns a data.table # enumerating all of its public, active binding, and private methods #' @importFrom assertthat assert_that @@ -648,12 +682,12 @@ FunctionReporter <- R6::R6Class( # If expression x is not an atomic value or symbol (i.e., name of object) or # an environment pointer then we can break x up into list of components - listable <- (!is.atomic(x) && !is.symbol(x) && !is.environment(x)) + listable <- .is_listable_expr(x) # If it is not a list but listable... if (!is.list(x) && listable) { # Convert to list - xList <- as.list(x) + xList <- .try_list(x) if (length(xList) > 0){ # Check if expression x is from _$_ if (identical(xList[[1]], quote(`$`))) { @@ -677,10 +711,10 @@ FunctionReporter <- R6::R6Class( # List is zero length. This might occur when encountering a "break" command. # Make empty list "non-listable" so recursion stops in following step. listable <- FALSE - } + } } - + if (listable){ # Filter out atomic values because we don't care about them From 58eeff092043b9d2f393850f20f487d5b164f2ed Mon Sep 17 00:00:00 2001 From: Jay Qi Date: Fri, 20 Feb 2026 01:42:46 -0500 Subject: [PATCH 2/8] Add instruction to file an issue --- R/FunctionReporter.R | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/R/FunctionReporter.R b/R/FunctionReporter.R index f320c91b..20bf4a8c 100644 --- a/R/FunctionReporter.R +++ b/R/FunctionReporter.R @@ -398,7 +398,7 @@ FunctionReporter <- R6::R6Class( # an environment pointer then we can break x up into list of components listable <- .is_listable_expr(x) if (!is.list(x) && listable) { - x <- .try_list(x) + x <- .try_as_list(x) if (length(x) > 0){ # Check for expression of the form foo$bar @@ -454,14 +454,16 @@ FunctionReporter <- R6::R6Class( } # [description] -.try_list <- function(x) { +.try_as_list <- function(x) { tryCatch( as.list(x), error = function(e) { log_warn(sprintf( paste0( ".parse_function: as.list() failed for ", - "typeof=%s class=%s; treating as unlistable. Error: %s", + "typeof=%s class=%s; treating as unlistable. ", + "Please report to pkgnet maintainers in an issue. ", + "Error: %s", ), typeof(x), paste(class(x), collapse = ","), @@ -687,7 +689,7 @@ FunctionReporter <- R6::R6Class( # If it is not a list but listable... if (!is.list(x) && listable) { # Convert to list - xList <- .try_list(x) + xList <- .try_as_list(x) if (length(xList) > 0){ # Check if expression x is from _$_ if (identical(xList[[1]], quote(`$`))) { From 218e1dfd8afa01926b26319023b3e3e27a0c8016 Mon Sep 17 00:00:00 2001 From: Jay Qi Date: Fri, 20 Feb 2026 01:58:07 -0500 Subject: [PATCH 3/8] Fix incorrect ! --- R/FunctionReporter.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/FunctionReporter.R b/R/FunctionReporter.R index 20bf4a8c..1965d87b 100644 --- a/R/FunctionReporter.R +++ b/R/FunctionReporter.R @@ -446,7 +446,7 @@ FunctionReporter <- R6::R6Class( # Symbol (i.e., name of object) if (is.symbol(x)){return(FALSE)} # Environment - if (!is.environment(x)){return(FALSE)} + if (is.environment(x)){return(FALSE)} # Raw external pointer to non-R memory/state (e.g., for C/C++ code) if (typeof(x) == "externalptr"){return(FALSE)} From 553630fd752549b4841e5e8d473ec4d20a907177 Mon Sep 17 00:00:00 2001 From: Jay Qi Date: Mon, 23 Feb 2026 22:16:32 -0500 Subject: [PATCH 4/8] Fix .try_as_list listable setting; add tests --- R/FunctionReporter.R | 32 ++++++++++++------- tests/testthat/test-FunctionReporter-class.R | 33 ++++++++++++++++++++ 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/R/FunctionReporter.R b/R/FunctionReporter.R index 1965d87b..1debffd0 100644 --- a/R/FunctionReporter.R +++ b/R/FunctionReporter.R @@ -398,9 +398,11 @@ FunctionReporter <- R6::R6Class( # an environment pointer then we can break x up into list of components listable <- .is_listable_expr(x) if (!is.list(x) && listable) { - x <- .try_as_list(x) + result <- .try_as_list(x) + x <- result$value + listable <- result$listable - if (length(x) > 0){ + if (listable && length(x) > 0){ # Check for expression of the form foo$bar # We still want to split it up because foo might be a function # but we want to get rid of bar, because it's a symbol in foo's namespace @@ -408,7 +410,7 @@ FunctionReporter <- R6::R6Class( if (identical(x[[1]], quote(`$`))) { x <- x[1:2] } - } else { + } else if (listable) { # make empty lists "not listable" so recursion stops listable <- FALSE } @@ -456,21 +458,26 @@ FunctionReporter <- R6::R6Class( # [description] .try_as_list <- function(x) { tryCatch( - as.list(x), + list( + value = as.list(x), + listable = TRUE + ), error = function(e) { log_warn(sprintf( paste0( - ".parse_function: as.list() failed for ", + "Expression parsing: as.list() failed for ", "typeof=%s class=%s; treating as unlistable. ", "Please report to pkgnet maintainers in an issue. ", - "Error: %s", + "Error: %s" ), typeof(x), paste(class(x), collapse = ","), conditionMessage(e) )) - listable <<- FALSE - return(x) + list( + value = x, + listable = FALSE + ) } ) } @@ -689,8 +696,10 @@ FunctionReporter <- R6::R6Class( # If it is not a list but listable... if (!is.list(x) && listable) { # Convert to list - xList <- .try_as_list(x) - if (length(xList) > 0){ + result <- .try_as_list(x) + xList <- result$value + listable <- result$listable + if (listable && length(xList) > 0){ # Check if expression x is from _$_ if (identical(xList[[1]], quote(`$`))) { # Check if expression x is of form self$foo, private$foo, or super$foo @@ -709,7 +718,7 @@ FunctionReporter <- R6::R6Class( # Left Hand is not a _$_. Proceed as normal list. x <- xList } - } else { + } else if (listable) { # List is zero length. This might occur when encountering a "break" command. # Make empty list "non-listable" so recursion stops in following step. listable <- FALSE @@ -731,4 +740,3 @@ FunctionReporter <- R6::R6Class( } return(out) } - diff --git a/tests/testthat/test-FunctionReporter-class.R b/tests/testthat/test-FunctionReporter-class.R index 631eeb2d..34cdaa54 100644 --- a/tests/testthat/test-FunctionReporter-class.R +++ b/tests/testthat/test-FunctionReporter-class.R @@ -340,6 +340,39 @@ test_that(".parse_R6_expression correctly parses expressions containing a next s }) }) +test_that(".is_listable_expr treats external pointers as unlistable", { + ptr <- new("externalptr") + expect_false(pkgnet:::.is_listable_expr(ptr)) +}) + +test_that(".parse_function falls back when as.list fails on listable objects", { + if (!methods::isClass("PkgnetNoListable")) { + methods::setClass("PkgnetNoListable", slots = c(x = "numeric")) + } + obj <- methods::new("PkgnetNoListable", x = 1) + + expect_true(pkgnet:::.is_listable_expr(obj)) + expect_error(as.list(obj)) + + result <- expect_no_error(pkgnet:::.parse_function(obj)) + expect_true(is.character(result)) + expect_length(result, 1) +}) + +test_that(".parse_R6_expression falls back when as.list fails on listable objects", { + if (!methods::isClass("PkgnetNoListable")) { + methods::setClass("PkgnetNoListable", slots = c(x = "numeric")) + } + obj <- methods::new("PkgnetNoListable", x = 1) + + expect_true(pkgnet:::.is_listable_expr(obj)) + expect_error(as.list(obj)) + + result <- expect_no_error(pkgnet:::.parse_R6_expression(obj)) + expect_true(is.character(result)) + expect_length(result, 1) +}) + test_that("FunctionReporter R6 edge extraction handles case where all methods have the same number of dependencies", { From 4e6aa3fc18a004013bb84c4ae539748d576fdc78 Mon Sep 17 00:00:00 2001 From: Jay Qi Date: Mon, 23 Feb 2026 22:29:14 -0500 Subject: [PATCH 5/8] Add NEWS.md entry --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 4ed06ba5..1a959d8e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ ## CHANGES ## BUGFIXES +* Fixed runtime error when `FunctionReporter` extract edges from a function containing expressions of `externalptr` type. `FunctionReporter` will generally now ignore unknown expression types and instead log an error. (#344) # pkgnet 0.6.0 ## NEW FEATURES From 5c718d6b1be0982ecfdbcf093b2e7fa3b5e484f7 Mon Sep 17 00:00:00 2001 From: Jay Qi <2721979+jayqi@users.noreply.github.com> Date: Mon, 23 Feb 2026 23:22:18 -0500 Subject: [PATCH 6/8] Add description for .try_as_list Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- R/FunctionReporter.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/FunctionReporter.R b/R/FunctionReporter.R index 1debffd0..3a8fc967 100644 --- a/R/FunctionReporter.R +++ b/R/FunctionReporter.R @@ -455,7 +455,10 @@ FunctionReporter <- R6::R6Class( return(TRUE) } -# [description] +# [description] attempt to coerce an expression to a list, returning a list +# containing the result as `value` and a `listable` flag; on +# error, log a warning and return the original object as +# unlistable. .try_as_list <- function(x) { tryCatch( list( From 51435c4a173e621ada86e13e861b0ff4d63b17ad Mon Sep 17 00:00:00 2001 From: Jay Qi <2721979+jayqi@users.noreply.github.com> Date: Mon, 23 Feb 2026 23:22:40 -0500 Subject: [PATCH 7/8] Fix incorrect log type Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 1a959d8e..e0896a8c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,7 +4,7 @@ ## CHANGES ## BUGFIXES -* Fixed runtime error when `FunctionReporter` extract edges from a function containing expressions of `externalptr` type. `FunctionReporter` will generally now ignore unknown expression types and instead log an error. (#344) +* Fixed runtime error when `FunctionReporter` extract edges from a function containing expressions of `externalptr` type. `FunctionReporter` will generally now ignore unknown expression types and instead log a warning. (#344) # pkgnet 0.6.0 ## NEW FEATURES From e2edc12de82a200dc0848935e589782c266f05e3 Mon Sep 17 00:00:00 2001 From: Jay Qi Date: Mon, 23 Feb 2026 23:27:31 -0500 Subject: [PATCH 8/8] Assert warning isntead of no error --- tests/testthat/test-FunctionReporter-class.R | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-FunctionReporter-class.R b/tests/testthat/test-FunctionReporter-class.R index 34cdaa54..d5d40d28 100644 --- a/tests/testthat/test-FunctionReporter-class.R +++ b/tests/testthat/test-FunctionReporter-class.R @@ -354,7 +354,10 @@ test_that(".parse_function falls back when as.list fails on listable objects", { expect_true(pkgnet:::.is_listable_expr(obj)) expect_error(as.list(obj)) - result <- expect_no_error(pkgnet:::.parse_function(obj)) + result <- expect_warning( + pkgnet:::.parse_function(obj), + regexp = "Expression parsing: as\\.list\\(\\) failed" + ) expect_true(is.character(result)) expect_length(result, 1) }) @@ -368,7 +371,10 @@ test_that(".parse_R6_expression falls back when as.list fails on listable object expect_true(pkgnet:::.is_listable_expr(obj)) expect_error(as.list(obj)) - result <- expect_no_error(pkgnet:::.parse_R6_expression(obj)) + result <- expect_warning( + pkgnet:::.parse_R6_expression(obj), + regexp = "Expression parsing: as\\.list\\(\\) failed" + ) expect_true(is.character(result)) expect_length(result, 1) })