Is isS3stdGeneric() the right way to detect S3 generics?

[jonkeane]

When checking if a function is an S3 generic (after getting it from the NAMESPACE file for example), a call to isS3stdGeneric() is used to tell if the function is an S3 generic. This works well for standard generics (when the first expression in the body calls UseMethod() [1]), but doesn't work so well if there is any sort of pre-processing in the generic (e.g.vctrs::vec_ptype_full which runs some dots checking before calling UseMethod)

> isS3stdGeneric(vctrs::vec_ptype_full)
[1] FALSE

I've looked a bit and haven't yet found a function that will find both standard and non-standard S3 generics. Would it be better / ok to fall back to the prior check of is.function(fun)? Or have an option to do that?

[1] - https://stat.ethz.ch/R-manual/R-devel/library/utils/html/isS3stdGen.html

[MichaelChirico]

The manual seems pretty clear about the intent of the function:

A closure is considered a standard S3 generic if the first expression in its body calls UseMethod

That said, the body of utils::isS3stdGeneric is written in R & should be straightforward to extend:

isS3stdGeneric <- function (f) 
{
    bdexpr <- body(f)
    while (as.character(bdexpr[[1L]]) == "{") bdexpr <- bdexpr[[2L]]
    ret <- is.call(bdexpr) && identical(bdexpr[[1L]], as.name("UseMethod"))
    if (ret) 
        names(ret) <- bdexpr[[2L]]
    ret
}

Something like "the final expression of the body is UseMethod" instead sounds pretty reasonable. Things could get more complicated (e.g. if the final expression is an if() statement) but I think setting FALSE in that case is fine.

When implementing, this would be another case where https://github.com/jimhester/lintr/blob/master/.dev/compare_branches.R will come in handy to make sure we're not introducing any false positives by mistake.

[AshesITR]

Just played around a little bit:

isS3stdGeneric <- function(f) {
  bdexpr <- body(f)
  while (as.character(bdexpr[[1L]]) == "{") bdexpr <- tail(bdexpr, 1L)[[1L]]
  ret <- is.call(bdexpr) && identical(bdexpr[[1L]], as.name("UseMethod"))
  if (ret) 
    names(ret) <- bdexpr[[2L]]
  ret
}

Does the trick for vctrs::vec_ptype_full by picking the last expression in every block instead of the first, but I haven't tested anything else.

While at it, maybe it would be more useful to analyze if there is any call to UseMethod in the body?