Skip to content

Commit

Permalink
Fix S4 .local wrapper extraction
Browse files Browse the repository at this point in the history
I guess this never worked 🤷

Fixes #847
  • Loading branch information
hadley committed Aug 12, 2019
1 parent 059bd67 commit 8c7fc13
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 27 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# roxygen2 (development version)

* S4 methods that generate `.local()` wrapper no longer error with obscure
error message (#847).

* `@param` containing only whitespce gets a clear warning message (#869).

* A new system for providing custom roxygen2 metadata is now available. When
Expand Down
31 changes: 31 additions & 0 deletions R/object-from-call.R
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,34 @@ parser_setConstructorS3 <- function(call, env, block) {
value <- standardise_obj(name, get(name, env), env, block)
object(value, name)
}


# helpers -----------------------------------------------------------------

# When a generic has ... and a method adds new arguments, the S4 method
# wraps the definition inside another function which has the same arguments
# as the generic. This function figures out if that's the case, and extracts
# the original function if so.
#
# It's based on expression processing based on the structure of the
# constructed method which looks like:
#
# function (x, ...) {
# .local <- function (x, ..., y = 7) {}
# .local(x, ...)
# }
extract_method_fun <- function(x) {
fun <- x@.Data

method_body <- body(fun)
if (!is_call(method_body)) return(fun)
if (length(method_body) == 0) return(fun)
if (!identical(method_body[[1]], quote(`{`))) return(fun)

first_line <- method_body[[2]]
if (!is_call(first_line, name = "<-", n = 2)) return(fun)
if (!identical(first_line[[2]], quote(`.local`))) return(fun)

eval(first_line[[3]])
}

27 changes: 0 additions & 27 deletions R/object.R
Original file line number Diff line number Diff line change
Expand Up @@ -49,33 +49,6 @@ is_generator <- function(x) {
methods::is(x, "refObjectGenerator") || methods::is(x, "classGeneratorFunction")
}

# When a generic has ... and a method adds new arguments, the S4 method
# wraps the definition inside another function which has the same arguments
# as the generic. This function figures out if that's the case, and extracts
# the original function if so.
#
# It's based on expression processing based on the structure of the
# constructed method which looks like:
#
# function (x, ...) {
# .local <- function (x, ..., y = 7) {}
# .local(x, ...)
# }
extract_method_fun <- function(x) {
fun <- x@.Data

method_body <- body(fun)
if (!is.call(method_body)) return(fun)
if (!identical(method_body[[1]], quote(`{`))) return(fun)

first_line <- method_body[[2]]
if (!is.call(first_line)) return(fun)
if (!identical(first_line[[1]], quote(`<-`))) return(fun)
if (!identical(first_line[[2]], quote(`.local`))) return(fun)

first_line[[3]]
}

# Consistent naming scheme for R object classes --------------------------------
# (s3/s4/rc x generic/class/method, function, data)

Expand Down
12 changes: 12 additions & 0 deletions tests/testthat/test-object-from-call.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
test_that("finds arguments when S4 method wrapped inside .local()", {
env <- pkg_env()
setClass("Foo", where = env)
call <- quote(
setMethod('subset', 'Foo', function(x, foo, ...) {})
)
eval(call, envir = env)

obj <- object_from_call(call, env, block = NULL, file = NULL)
expect_s3_class(obj, "s4method")
expect_named(formals(obj$value@.Data), c("x", "foo", "..."))
})

0 comments on commit 8c7fc13

Please sign in to comment.