S3 methods in local environment are not identified

[mschubert]

Describe the bug

Originally mentioned in #2, identification of local S3 methods is mentioned to be fixed in 7847dd5 (2018).

#47 seems to be for S3 methods in packages specifically.

Related: mschubert/clustermq#256

Reproduce example

f = function(i) UseMethod('f')
f.numeric = function(i) sqrt(i)
globals::globalsOf({ f(1) }, locals=TRUE, recursive=TRUE) # addS3 is no longer documented
# list()

The originally included test also seems to fail now, although it is no longer available. I also no longer see any mention on S3 in the tests or any changes to S3 behavior in the NEWS.

Expected behavior

Shouldn't this identify f.numeric?

Session information

R version 4.3.2, globals_0.16.2

[HenrikBengtsson]

Not a solution, but a clarification. globalsOf() takes an R expression, i.e. in your current example it searches for globals in the value of { f(1) } == 1, i.e. globals::globalsOf({ f(1) }, locals=TRUE, recursive=TRUE) == globals::globalsOf(1, locals=TRUE, recursive=TRUE). So, what you meant here is:

f <- function(i) UseMethod('f')
f.numeric <- function(i) sqrt(i)
expr <- quote({ f(1) })
g <- globals::globalsOf(expr, locals=TRUE)  ## recursive=TRUE is the default, so skipping
str(g)

which outputs:

List of 3
 $ {        :.Primitive("{") 
 $ f        :function (i)  
  ..- attr(*, "srcref")= 'srcref' int [1:8] 1 6 1 31 6 31 1 1
  .. ..- attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x5a758f668c48> 
 $ UseMethod:function (generic, object)  
 - attr(*, "where")=List of 3
  ..$ {        :<environment: base> 
  ..$ f        :<environment: R_GlobalEnv> 
  ..$ UseMethod:<environment: base> 
 - attr(*, "class")= chr [1:2] "Globals" "list"

Here we see it detects {, f, and UseMethod as global ("unbound") variables. BTW, using locals=TRUE makes no difference here.

As you noticed, it does not method f.numeric() at all. Now, that would be a whole other level of skill sets for globalsOf(). To be able to get to that, it would need to know what class of object is passed to f(). In this case it would be easy, because we can even from static-code inspection see that it's a constant, namely numeric 1. A more realistic scenario would be an expression like:

x <- some_fcn(z)
y <- f(x) 

In this case, we cannot know what class x has without running the code. Because of this, it's impossible to infer that f.numeric() is a needed global in order to resolve this expression (e.g. in a future).

An alternative approach could be to make globalsOf() aware of S3 generic functions and how they work in R. It could inspect the globals it has found and look for generic functions. It would then identify f() as an S3 generic function. We could then take a conservative approach and say that all S3 methods for generic function f() should also be included, i.e.

> methods("f")
[1] f.numeric
see '?methods' for accessing help and source code

This could work. The problem with this approach though is that there might be lots of such methods. For instance, consider you used print() instead of f() above. In my currnt R session, that we result in 250 S3 methods;

> mthds <- methods("print")
> str(mthds)
 'MethodsFunction' chr [1:250] "print.{" "print.acf" "print.activeConcordance" "print.AES" "print.anova" "print.aov" "print.aovlist" "print.ar" "print.Arima" "print.arima0" "print.AsIs" "print.aspell" "print.aspell_inspect_context" "print.bibentry" "print.Bibtex" "print.bioconductor_releases" "print.bioconductor_settings" "print.biocValid" ...
 - attr(*, "info")='data.frame':	250 obs. of  4 variables:
  ..$ visible: logi [1:250] TRUE FALSE FALSE FALSE FALSE FALSE ...
  ..$ from   : chr [1:250] ".GlobalEnv" "registered S3method for print" "registered S3method for print" "registered S3method for print" ...
  ..$ generic: chr [1:250] "print" "print" "print" "print" ...
  ..$ isS4   : logi [1:250] FALSE FALSE FALSE FALSE FALSE FALSE ...
 - attr(*, "byclass")= logi FALSE

Gist emulating what could be done:

library(globals)

print <- function(i) UseMethod('f')
print.numeric <- function(i) sqrt(i)
expr <- quote({ print(1) })
g <- globalsOf(expr)

## Should be done internally by globalsOf()
names <- methods("print")
mthds <- lapply(names, FUN = function(name) {
  res <- getAnywhere(name)
  fcn <- res[[2]][[1]]
})
names(mthds) <- names
where <- lapply(mthds, FUN = environment)
attr(mthds, "where") <- where

## Identify packages of the S3 methods
pkgs <- vapply(where, FUN.VALUE = NA_character_, FUN = function(env) {
  name <- environmentName(env)
  tryCatch({ asNamespace(name); name }, error = function(ex) "")
})
pkgs <- unname(pkgs)
upkgs <- unique(pkgs)
upkgs <- upkgs[nzchar(upkgs)]

## Drop what's in a package
mthds <- mthds[!pkgs %in% upkgs]

g2 <- as.Globals(mthds)
g <- c(g, g2)
length(g)
#> [1] 9

The future framework uses cleanup() to drop globals part of base R namespaces;

g <- cleanup(g)
length(g)
#> [1] 7

Then we only need to export the globals in g and make sure to load the packages upkgs.

Anyway, that's the gist. I mostly scribbled it down to share my train of though and for my future self. I have to think a lot more about this approach, e.g. consider unwanted side effects, before making a move on this.