diff --git a/NAMESPACE b/NAMESPACE index 16148e7b..713dcf05 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -17,6 +17,7 @@ S3method(print,S7_S3_class) S3method(print,S7_any) S3method(print,S7_base_class) S3method(print,S7_class) +S3method(print,S7_external_class) S3method(print,S7_external_generic) S3method(print,S7_generic) S3method(print,S7_method) @@ -87,6 +88,7 @@ export(method_explain) export(methods_register) export(new_S3_class) export(new_class) +export(new_external_class) export(new_external_generic) export(new_generic) export(new_object) diff --git a/NEWS.md b/NEWS.md index 94f68d5e..0de661bf 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # S7 (development version) +* Internal changes to support R-devel (4.6) (#592, #593, #598, #600). * New `:=` operator creates and names an object in one step, so `Foo := new_class()` is equivalent to `Foo <- new_class(name = "Foo")` (#658). * Errors thrown by S7 now report the function where they occurred, making it easier to track down the source of a problem (#646). * `class_POSIXct` uses the `tzone` attribute (not `tz`), and allows it to be absent (#401). @@ -23,6 +24,7 @@ * `new_class()` now allows properties named `names`, `dim`, `dimnames`, `class`, `comment`, `tsp`, and `row.names`. But property names beginning with `_` are now reserved for internal use (#579). * `new_class()` experimentally allows `class_environment` as a parent again, so you can build S7 objects that share R's reference semantics for environments. This support is provisional: because environments are mutated in place, some operations behave differently than for value-typed S7 objects, and the API may change. `S7_data()` and `S7_data<-()` error on environment-based objects, since they would otherwise destroy the object's S7 attributes in place (#590). * `new_class()`'s default constructor now respects properties overridden in a subclass: the subclass's default is used (#467) and its setter is run during construction (#585). Values for overridden properties are passed to both the parent constructor and the new object, so a subclass can override a parent property whose default is mandatory. +* `new_external_class()` creates a delayed reference to an S7 class in another package (or your own package, but not yet defined). It is useful for registering methods on classes from suggested packages (#573) and for creating self-referential or mutually recursive classes (#250). * `new_object()` now gives an informative error when `.parent` is a class specification rather than an instance of the parent class (#409). * `new_object()` no longer materialises ALTREP parent values (e.g. `seq_len()`), so constructing an S7 object that wraps a large compact integer sequence is now O(1) in memory instead of O(n) (@kschaubroeck, #607). * `new_object()` no longer re-runs property validators for properties inherited unchanged from an already-validated parent class, so constructing an instance of a deeply nested class hierarchy validates each property exactly once (#539). diff --git a/R/class-spec.R b/R/class-spec.R index 59741cae..62e2d843 100644 --- a/R/class-spec.R +++ b/R/class-spec.R @@ -53,6 +53,7 @@ is_foundation_class <- function(x) { is_union(x) || is_base_class(x) || is_S3_class(x) || + is_external_class(x) || is_class_missing(x) || is_class_any(x) } @@ -72,6 +73,8 @@ class_type <- function(x) { "S7_union" } else if (is_S3_class(x)) { "S7_S3" + } else if (is_external_class(x)) { + "S7_external" } else if (is_S4_class(x)) { "S4" } else { @@ -90,6 +93,7 @@ class_friendly <- function(x) { S7_base = "a base type", S7_union = "an S7 union", S7_S3 = "an S3 class", + S7_external = "an external S7 class", ) } @@ -185,6 +189,7 @@ class_constructor <- function(.x) { S7_base = .x$constructor, S7_union = class_constructor(.x$classes[[1]]), S7_S3 = .x$constructor, + S7_external = class_constructor(resolve_external_class_req(.x)), stop2(sprintf("Can't construct %s.", class_friendly(.x)), call = NULL) ) } @@ -199,6 +204,7 @@ class_validate <- function(class, object) { S7 = class@validator, S7_base = class$validator, S7_S3 = class$validator, + S7_external = class_validate(resolve_external_class_req(class), object), NULL ) @@ -238,6 +244,7 @@ class_desc <- function(x) { S7_base = paste0("<", x$class, ">"), S7_union = oxford_or(unlist(lapply(x$classes, class_desc))), S7_S3 = paste0("S3<", paste0(x$class, collapse = "/"), ">"), + S7_external = paste0("<", x$class_name, ">"), ) } @@ -256,6 +263,7 @@ class_dispatch <- function(x) { S7 = c(S7_class_name(x), class_dispatch(x@parent)), S7_base = c(x$class, "S7_object"), S7_S3 = c(x$class, "S7_object"), + S7_external = class_dispatch(resolve_external_class_req(x)), stop2("Unsupported class type.", call = NULL) ) } @@ -271,6 +279,7 @@ class_register <- function(x) { S7 = S7_class_name(x), S7_base = x$class, S7_S3 = x$class[[1]], + S7_external = x$class_name, stop2("Unsupported class type.", call = NULL) ) } @@ -290,6 +299,7 @@ class_deparse <- function(x) { paste0("new_union(", paste(classes, collapse = ", "), ")") }, S7_S3 = paste0("new_S3_class(", deparse1(x$class), ")"), + S7_external = sprintf("new_external_class(%s, %s)", x$package, x$name), ) } @@ -304,6 +314,7 @@ class_inherits <- function(x, what) { S7_base = what$class == base_class(x), S7_union = any(vlapply(what$classes, class_inherits, x = x)), S7_S3 = !isS4(x) && class_dispatch_extends(what$class, class(x)), + S7_external = inherits(x, "S7_object") && inherits(x, what$class_name), ) } @@ -334,6 +345,12 @@ class_extends <- function(child, parent) { methods::extends(child@className, parent@className) } else if (is_class(parent) && parent@name == "S7_object") { is_class(child) + } else if (is_external_class(child)) { + child <- resolve_external_class_req(child) + class_extends(child, parent) + } else if (is_external_class(parent)) { + parent <- resolve_external_class_req(parent) + class_extends(child, parent) } else { # handle S7, S3, and base types. class_dispatch_extends(class_dispatch(parent), class_dispatch(child)) diff --git a/R/external-class.R b/R/external-class.R new file mode 100644 index 00000000..f102762b --- /dev/null +++ b/R/external-class.R @@ -0,0 +1,209 @@ +#' Classes in other packages +#' +#' @description +#' An external class is a lightweight placeholder for an S7 class defined in +#' another package (or in your own package and needed before it's fully +#' defined). It carries only the package and class name, and is resolved to +#' the real S7 class when needed. +#' +#' External classes are useful in two situations: +#' +#' * To register a method for a generic in your package, dispatching on a class +#' from a soft dependency. The method will be registered when `pkg` is loaded +#' (using the same machinery as [new_external_generic()]). +#' +#' ```R +#' SomeClass <- new_external_class("pkg", "SomeClass") +#' method(my_generic, SomeClass) <- ... +#' ``` +#' +#' * To refer to a class that hasn't been defined yet, such as a +#' self-referential or mutually recursive class. +#' +#' ```R +#' tree_stub <- new_external_class("mypkg", "tree") +#' new_class("tree", properties = list(child = NULL | tree_stub)) +#' ``` +#' +#' Make sure to call [S7_on_load()] in your package's `.onLoad()` so that +#' deferred method registrations fire when the relevant package is loaded. +#' +#' External classes can not currently be used as parents in [new_class()]. +#' We hope to relax that restriction in the near future. +#' +#' @param package Package the class is defined in. +#' @param name Name of the class, as a string. +#' @inheritParams new_external_generic version +#' @returns An S7 external class, i.e. a list with S3 class `S7_external_class`. +#' @export +#' @examples +#' # Refer to a class in another package without taking a hard dependency: +#' Tibble <- new_external_class("tibble", "tbl_df") +#' Tibble +#' +#' # Self-referential class: the `child` property can be another `tree`, +#' # or `NULL` to terminate the chain. +#' tree_stub <- new_external_class("mypkg", "tree") +#' tree <- new_class( +#' name = "tree", +#' package = "mypkg", +#' properties = list(child = NULL | tree_stub) +#' ) +new_external_class <- function(package, name, version = NULL) { + if (!is_string(package)) { + stop2("`package` must be a string.") + } + if (!is_string(name)) { + stop2("`name` must be a string.") + } + + out <- list( + package = package, + name = name, + class_name = paste0(package, "::", name), + version = version + ) + class(out) <- "S7_external_class" + out +} + +is_external_class <- function(x) { + inherits(x, "S7_external_class") +} + +class_has_external_class <- function(x) { + if (is_external_class(x)) { + TRUE + } else if (is_union(x)) { + any(vlapply(x$classes, class_has_external_class)) + } else { + FALSE + } +} + +signature_has_external_class <- function(signature) { + any(vlapply(signature, class_has_external_class)) +} + +class_external_deps <- function(x) { + if (is_external_class(x)) { + list(x) + } else if (is_union(x)) { + flatten_external_deps(lapply(x$classes, class_external_deps)) + } else { + list() + } +} + +signature_external_deps <- function(signature) { + flatten_external_deps(lapply(signature, class_external_deps)) +} + +flatten_external_deps <- function(x) { + unlist(x, recursive = FALSE, use.names = FALSE) +} + +#' @export +print.S7_external_class <- function(x, ...) { + cat( + " ", + x$class_name, + if (!is.null(x$version)) paste0(" (>= ", x$version, ")"), + "\n", + sep = "" + ) + invisible(x) +} + +dep_available <- function(dep) { + isNamespaceLoaded(dep$package) && + (is.null(dep$version) || getNamespaceVersion(dep$package) >= dep$version) +} + +# Make it mockable +getNamespaceVersion <- NULL + +resolve_signature <- function(signature) { + for (i in seq_along(signature)) { + signature[[i]] <- resolve_class_req(signature[[i]]) + } + signature +} + +resolve_class_req <- function(x) { + if (is_external_class(x)) { + resolve_external_class_req(x) + } else if (is_union(x)) { + do.call(new_union, lapply(x$classes, resolve_class_req)) + } else { + x + } +} + +find_external_class <- function(x) { + ns <- asNamespace(x$package) + if (exists(x$name, envir = ns, inherits = FALSE)) { + obj <- get(x$name, envir = ns, inherits = FALSE) + if (is_external_class_match(obj, x)) { + return(obj) + } + } + + # Also consider cases where the constructor isn't named the same as the class + for (name in ls(ns, all.names = TRUE)) { + obj <- get(name, envir = ns, inherits = FALSE) + if (is_external_class_match(obj, x)) { + return(obj) + } + } + + NULL +} + +is_external_class_match <- function(obj, x) { + is_class(obj) && + (identical(S7_class_name(obj), x$class_name) || + (identical(obj@name, x$name) && + (is.null(obj@package) || identical(obj@package, x$package)))) +} + +# Required resolution: errors if the external class can't be resolved (e.g. +# its package isn't loaded). Used wherever we need the real class: registering +# or looking up methods, checking property overrides in a subclass, and +# constructing or validating an instance. +resolve_external_class_req <- function(x) { + prefix <- sprintf("Can't find external class <%s>:\n", x$class_name) + if (!requireNamespace(x$package, quietly = TRUE)) { + stop2( + paste0(prefix, sprintf("* Package '%s' is not installed.", x$package)), + call = NULL + ) + } + + if (!is.null(x$version) && getNamespaceVersion(x$package) < x$version) { + stop2( + paste0( + prefix, + sprintf( + "* Package '%s' needs version %s, but only %s is available.", + x$package, + x$version, + getNamespaceVersion(x$package) + ) + ), + call = NULL + ) + } + + class <- find_external_class(x) + if (is.null(class)) { + stop2( + paste0( + prefix, + sprintf("* Packages '%s' doesn't contain '%s'.", x$package, x$name) + ), + call = NULL + ) + } + class +} diff --git a/R/external-generic.R b/R/external-generic.R index fab97db3..1537620a 100644 --- a/R/external-generic.R +++ b/R/external-generic.R @@ -88,34 +88,53 @@ external_generic_version_ok <- function(generic, ns) { is.null(generic$version) || getNamespaceVersion(ns) >= generic$version } -registrar <- function(generic, signature, method, env) { +registrar <- function(deps, generic, signature, method, env) { # Force all arguments + deps generic signature method env function(...) { - ns <- asNamespace(generic$package) - if (external_generic_version_ok(generic, ns)) { - if (!exists(generic$name, envir = ns, inherits = FALSE)) { - msg <- sprintf( - "[S7] Failed to find generic %s() in package %s", - generic$name, - generic$package - ) - warning(msg, call. = FALSE) - } else { - generic_fun <- get(generic$name, envir = ns, inherits = FALSE) - register_method(generic_fun, signature, method, env, package = NULL) - } + if (!all(vlapply(deps, dep_available))) { + return(invisible()) } + + generic_fun <- resolve_generic(generic) + if (is.null(generic_fun)) { + return(invisible()) + } + + signature <- resolve_signature(signature) + register_method(generic_fun, signature, method, env, package = NULL) } } -external_methods_reset <- function(package) { - S7_methods_table(package) <- list() - invisible() +# Collects all external dependencies (the generic + any external classes) +# into a single list. Each entry has at minimum `package` + `version`. +method_deps <- function(generic, signature) { + c(list(generic), signature_external_deps(signature)) +} +method_deps_packages <- function(deps) { + unique(vcapply(deps, function(dep) dep$package)) +} + +resolve_generic <- function(generic) { + ns <- asNamespace(generic$package) + if (exists(generic$name, envir = ns, inherits = FALSE)) { + get(generic$name, envir = ns, inherits = FALSE) + } else { + warning( + sprintf( + "[S7] Failed to find generic %s() in package %s", + generic$name, + generic$package + ), + call. = FALSE + ) + NULL + } } external_methods_add <- function( diff --git a/R/hooks.R b/R/hooks.R index 11d11396..468d8f3c 100644 --- a/R/hooks.R +++ b/R/hooks.R @@ -48,12 +48,7 @@ methods_register <- function() { } S7_on_load_ <- function(env) { - package <- packageName(env) - - hooks_remove(package) # always start from a clean slate - hooks <- hooks_add(package) - hooks_run_loaded(hooks) # run hooks for loaded packages - + hooks_set_and_run(packageName(env)) invisible() } @@ -97,51 +92,58 @@ S7_on_unload_ <- function(env) { invisible() } -# Add a hook for each method that registers it when its generic's package is -# loaded. Returns the added hooks, named by the package they're attached to. -hooks_add <- function(package) { - ns <- asNamespace(package) - tbl <- S7_methods_table(package) - pkgs <- vcapply(tbl, function(x) x$generic$package) +# Start from a clean slate, then register each method whose dependency packages +# are already loaded, and add a hook so it (re)registers whenever one of those +# packages is loaded in the future. +hooks_set_and_run <- function(package) { + hooks_remove(package) - hooks <- lapply(tbl, function(x) { - register <- registrar(x$generic, x$signature, x$method, ns) - hook <- S7_hook(register, package) - setHook(packageEvent(x$generic$package, "onLoad"), hook) - hook - }) - names(hooks) <- pkgs - hooks_packages(package) <- union(hooks_packages(package), pkgs) - hooks + pkgs <- character() + for (x in S7_methods_table(package)) { + hook <- hook_add(package, x) + hook$run() + pkgs <- c(pkgs, hook$pkgs) + } + + # Record packages with hooks so we can remove them on unload + hooks_packages(package) <- unique(pkgs) + invisible() } -# Remove our hooks for `package`. -hooks_remove <- function(package) { - tbl <- S7_methods_table(package) - pkgs <- unique(c( - hooks_packages(package), - vcapply(tbl, function(x) x$generic$package) - )) +# Add a hook that (re)registers method `x` whenever one of its dependency +# packages is loaded, and return its registrar so it can also be run now. +hook_add <- function(package, x) { + ns <- asNamespace(package) + + deps <- method_deps(x$generic, x$signature) + register <- registrar(deps, x$generic, x$signature, x$method, ns) + hook <- S7_hook(register, package) + pkgs <- method_deps_packages(deps) for (pkg in pkgs) { - event <- packageEvent(pkg, "onLoad") - hooks <- getHook(event) - ours <- vlapply(hooks, is_S7_hook, package = package) - if (any(ours)) { - setHook(event, hooks[!ours], action = "replace") - } + setHook(packageEvent(pkg, "onLoad"), hook) } - hooks_packages(package) <- character() - invisible() + + list(run = register, pkgs = pkgs) } -hooks_run_loaded <- function(hooks) { - is_loaded <- vlapply(names(hooks), isNamespaceLoaded) - for (hook in hooks[is_loaded]) { - hook() +# Remove all of our hooks for `package`. `hooks_packages()` records every +# package event we've added a hook to, so we don't need to re-derive them here. +hooks_remove <- function(package) { + for (pkg in hooks_packages(package)) { + hook_remove(package, pkg) } + hooks_packages(package) <- character() invisible() } +hook_remove <- function(package, pkg) { + event <- packageEvent(pkg, "onLoad") + hooks <- getHook(event) + ours <- vlapply(hooks, is_S7_hook, package = package) + if (any(ours)) { + setHook(event, hooks[!ours], action = "replace") + } +} #' @export #' @rdname S7_on_load diff --git a/R/method-register.R b/R/method-register.R index 2afd6ea3..b334f1f7 100644 --- a/R/method-register.R +++ b/R/method-register.R @@ -86,6 +86,21 @@ register_method <- function( ) } + # Delay package methods with external classes until onLoad. Outside a package + # there is no deferred methods table, so resolve them before registering. + if (signature_has_external_class(signature)) { + if (is.null(package)) { + signature <- resolve_signature(signature) + } else { + generic_ext <- as_external_generic(generic, env) + external_methods_add(package, generic_ext, signature, method) + if (!is_local_generic(generic, package)) { + return(generic_sentinel(generic_ext)) + } + return(invisible(original)) + } + } + external <- NULL if (!is.null(package) && !is_local_generic(generic, package)) { external <- as_external_generic(generic, env) diff --git a/R/union.R b/R/union.R index c6267853..86fa9b56 100644 --- a/R/union.R +++ b/R/union.R @@ -60,6 +60,7 @@ on_load_define_or_methods <- function() { registerS3method("|", "S7_union", `|.S7_class`) registerS3method("|", "S7_base_class", `|.S7_class`) registerS3method("|", "S7_S3_class", `|.S7_class`) + registerS3method("|", "S7_external_class", `|.S7_class`) registerS3method("|", "S7_any", `|.S7_class`) registerS3method("|", "S7_missing", `|.S7_class`) registerS3method("|", "classGeneratorFunction", `|.S7_class`) diff --git a/_pkgdown.yml b/_pkgdown.yml index 8f091ee1..49394977 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -44,6 +44,7 @@ reference: for more details. contents: - S7_on_load + - new_external_class - new_external_generic - title: Compatibility diff --git a/man/new_external_class.Rd b/man/new_external_class.Rd new file mode 100644 index 00000000..b3e6b616 --- /dev/null +++ b/man/new_external_class.Rd @@ -0,0 +1,62 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/external-class.R +\name{new_external_class} +\alias{new_external_class} +\title{Classes in other packages} +\usage{ +new_external_class(package, name, version = NULL) +} +\arguments{ +\item{package}{Package the class is defined in.} + +\item{name}{Name of the class, as a string.} + +\item{version}{An optional version the package must meet for the method to +be registered.} +} +\value{ +An S7 external class, i.e. a list with S3 class \code{S7_external_class}. +} +\description{ +An external class is a lightweight placeholder for an S7 class defined in +another package (or in your own package and needed before it's fully +defined). It carries only the package and class name, and is resolved to +the real S7 class when needed. + +External classes are useful in two situations: +\itemize{ +\item To register a method for a generic in your package, dispatching on a class +from a soft dependency. The method will be registered when \code{pkg} is loaded +(using the same machinery as \code{\link[=new_external_generic]{new_external_generic()}}). + +\if{html}{\out{
}}\preformatted{SomeClass <- new_external_class("pkg", "SomeClass") +method(my_generic, SomeClass) <- ... +}\if{html}{\out{
}} +\item To refer to a class that hasn't been defined yet, such as a +self-referential or mutually recursive class. + +\if{html}{\out{
}}\preformatted{tree_stub <- new_external_class("mypkg", "tree") +new_class("tree", properties = list(child = NULL | tree_stub)) +}\if{html}{\out{
}} +} + +Make sure to call \code{\link[=S7_on_load]{S7_on_load()}} in your package's \code{.onLoad()} so that +deferred method registrations fire when the relevant package is loaded. + +External classes can not currently be used as parents in \code{\link[=new_class]{new_class()}}. +We hope to relax that restriction in the near future. +} +\examples{ +# Refer to a class in another package without taking a hard dependency: +Tibble <- new_external_class("tibble", "tbl_df") +Tibble + +# Self-referential class: the `child` property can be another `tree`, +# or `NULL` to terminate the chain. +tree_stub <- new_external_class("mypkg", "tree") +tree <- new_class( + name = "tree", + package = "mypkg", + properties = list(child = NULL | tree_stub) +) +} diff --git a/tests/testthat/_snaps/class.md b/tests/testthat/_snaps/class.md index 941c207a..5e44a215 100644 --- a/tests/testthat/_snaps/class.md +++ b/tests/testthat/_snaps/class.md @@ -149,6 +149,15 @@ - @x is . - @x is . +# subclassing an external class requires its package to be loaded + + Code + new_class("Child", Parent, properties = list(x = Ext)) + Condition + Error: + ! Can't find external class : + * Package 'notloaded.pkg' is not installed. + # abstract classes can't be instantiated Code diff --git a/tests/testthat/_snaps/external-class.md b/tests/testthat/_snaps/external-class.md new file mode 100644 index 00000000..a37b1a22 --- /dev/null +++ b/tests/testthat/_snaps/external-class.md @@ -0,0 +1,54 @@ +# new_external_class() validates inputs + + Code + new_external_class(1, "x") + Condition + Error in `new_external_class()`: + ! `package` must be a string. + Code + new_external_class("pkg", 1) + Condition + Error in `new_external_class()`: + ! `name` must be a string. + +# print method works + + Code + print(new_external_class("foo", "Bar")) + Output + foo::Bar + Code + print(new_external_class("foo", "Bar", version = "1.0")) + Output + foo::Bar (>= 1.0) + +# resolve_external_class_req() errors per failure mode + + Code + resolve_external_class_req(new_external_class("not_a_pkg", "X")) + Condition + Error: + ! Can't find external class : + * Package 'not_a_pkg' is not installed. + Code + resolve_external_class_req(new_external_class("S7", "S7_object", "2.0.0")) + Condition + Error: + ! Can't find external class : + * Package 'S7' needs version 2.0.0, but only 1.0.0 is available. + Code + resolve_external_class_req(new_external_class("S7", "not_a_class")) + Condition + Error: + ! Can't find external class : + * Packages 'S7' doesn't contain 'not_a_class'. + +# external class works as a property type for self-reference + + Code + Tree(label = "bad", child = 1) + Condition + Error in `Tree()`: + ! object properties are invalid: + - @child must be or , not + diff --git a/tests/testthat/_snaps/method-introspect.md b/tests/testthat/_snaps/method-introspect.md index 2307f793..72924a45 100644 --- a/tests/testthat/_snaps/method-introspect.md +++ b/tests/testthat/_snaps/method-introspect.md @@ -60,6 +60,21 @@ - x: - y: +# method introspection requires external class's package to be loaded + + Code + method(foo, class = ext) + Condition + Error: + ! Can't find external class : + * Package 'not_a_package' is not installed. + Code + method_explain(foo, class = ext) + Condition + Error: + ! Can't find external class : + * Package 'not_a_package' is not installed. + # method explanation shows all possible methods along with matches add([foo2], [foo2]) diff --git a/tests/testthat/test-class.R b/tests/testthat/test-class.R index e31814cd..8ae0672a 100644 --- a/tests/testthat/test-class.R +++ b/tests/testthat/test-class.R @@ -193,6 +193,17 @@ test_that("inheritance doesn't let child properties widen or change the parent's }) }) + +test_that("subclassing an external class requires its package to be loaded", { + Ext <- new_external_class("notloaded.pkg", "Cls") + Parent := new_class(properties = list(x = NULL | Ext), package = NULL) + + expect_snapshot( + new_class("Child", Parent, properties = list(x = Ext)), + error = TRUE + ) +}) + test_that("inheritance lets dynamic child properties override any parent type", { foo1 <- new_class("foo1", properties = list(x = class_integer)) readonly <- new_property(class_character, getter = function(self) "x") diff --git a/tests/testthat/test-external-class.R b/tests/testthat/test-external-class.R new file mode 100644 index 00000000..d25fd0dc --- /dev/null +++ b/tests/testthat/test-external-class.R @@ -0,0 +1,120 @@ +test_that("new_external_class() validates inputs", { + expect_snapshot(error = TRUE, { + new_external_class(1, "x") + new_external_class("pkg", 1) + }) +}) + +test_that("print method works", { + expect_snapshot({ + print(new_external_class("foo", "Bar")) + print(new_external_class("foo", "Bar", version = "1.0")) + }) +}) + +test_that("external class is a valid class spec", { + ec <- new_external_class("foo", "Bar") + + expect_identical(as_class(ec), ec) + expect_equal(class_type(ec), "S7_external") + expect_equal(class_register(ec), "foo::Bar") + expect_equal(class_desc(ec), "") + expect_equal(S7_class_desc(ec), "") +}) + +test_that("external class resolution uses the S7 class name", { + # The class is bound to `class_renamed`, but its S7 name is "renamed", so + # resolution must find it by scanning for a matching S7 class name. + pkg := local_package( + renamed <- new_class("named") + ) + named := new_external_class("pkg") + resolved <- resolve_external_class_req(named) + + expect_s3_class(resolved, "S7_class") + expect_equal(S7_class_name(resolved), "pkg::named") +}) + +test_that("resolve_external_class_req() errors per failure mode", { + local_mocked_bindings(getNamespaceVersion = function(package) "1.0.0") + expect_snapshot(error = TRUE, { + resolve_external_class_req(new_external_class("not_a_pkg", "X")) + resolve_external_class_req(new_external_class("S7", "S7_object", "2.0.0")) + resolve_external_class_req(new_external_class("S7", "not_a_class")) + }) +}) + +test_that("external class can be used as a union arm", { + ec <- new_external_class("foo", "Bar") + u <- NULL | ec + expect_s3_class(u, "S7_union") + expect_length(u$classes, 2) +}) + +test_that("external class works as a property type for self-reference", { + Tree := new_class( + package = "mypkg", + properties = list( + label = class_character, + child = NULL | new_external_class("mypkg", "Tree") + ) + ) + + leaf <- Tree(label = "leaf") + expect_null(leaf@child) + + root <- Tree(label = "root", child = leaf) + expect_equal(root@child@label, "leaf") + + # type checking still rejects wrong types + expect_snapshot(error = TRUE, Tree(label = "bad", child = 1)) +}) + +test_that("external class works for mutually recursive classes", { + ClassOne := new_class( + package = "mypkg", + properties = list(x = NULL | new_external_class("mypkg", "ClassTwo")) + ) + ClassTwo := new_class( + package = "mypkg", + properties = list(y = NULL | new_external_class("mypkg", "ClassOne")) + ) + + obj <- ClassOne(x = ClassTwo(y = ClassOne())) + expect_s3_class(obj@x, "mypkg::ClassTwo") + expect_s3_class(obj@x@y, "mypkg::ClassOne") +}) + +test_that("class_inherits() works for external class", { + Tree := new_class( + package = "mypkg", + properties = list(child = NULL | new_external_class("mypkg", "Tree")) + ) + ec <- new_external_class("mypkg", "Tree") + expect_true(class_inherits(Tree(), ec)) + expect_false(class_inherits(1, ec)) + expect_false(class_inherits(NULL, ec)) +}) + +test_that("method_deps() collects the generic and external classes", { + gen <- new_external_generic("foo", "bar", "x") + sig <- list( + new_external_class("baz", "X"), + class_character, + NULL | new_external_class("qux", "Y", version = "1.0") + ) + deps <- method_deps(gen, sig) + expect_equal(vcapply(deps, `[[`, "package"), c("foo", "baz", "qux")) + expect_equal(deps[[3]]$version, "1.0") +}) + +test_that("dep_available() respects loaded + version", { + # S7 is loaded, so this dep is available + expect_true(dep_available(new_external_generic("S7", "S7_inherits", "x"))) + # version too high → not available + expect_false(dep_available( + new_external_generic("S7", "S7_inherits", "x", version = "999.0") + )) + # unloaded package → not available + expect_false(dep_available(new_external_class("not_a_package", "X"))) +}) diff --git a/tests/testthat/test-hooks.R b/tests/testthat/test-hooks.R index e48f1306..1f8a62ce 100644 --- a/tests/testthat/test-hooks.R +++ b/tests/testthat/test-hooks.R @@ -14,6 +14,22 @@ test_that("S7_on_load() doesn't accumulate hooks across repeated loads", { expect_length(package_hooks("upstream"), 1) }) +test_that("S7_on_load() registers methods dispatching on an external class", { + upstream := local_package( + Foo := new_class() + ) + downstream := local_package( + own_generic := new_generic("x"), + Foo := new_external_class("upstream"), + method(own_generic, Foo) <- \(x) "from external class" + ) + # The method is deferred (its signature has an external class), not yet live + expect_length(methods(downstream$own_generic), 0) + + S7_on_load_(downstream) + expect_equal(downstream$own_generic(upstream$Foo()), "from external class") +}) + test_that("S7_on_unload() unregisters methods and removes hooks", { upstream <- local_package("upstream", gen := new_generic("x")) downstream <- local_package( diff --git a/tests/testthat/test-method-introspect.R b/tests/testthat/test-method-introspect.R index 21affc22..f1a68067 100644 --- a/tests/testthat/test-method-introspect.R +++ b/tests/testthat/test-method-introspect.R @@ -35,6 +35,17 @@ test_that("method introspection errors if no method found", { }) }) +test_that("method introspection requires external class's package to be loaded", { + foo := new_generic("x") + method(foo, NULL) <- function(x) "null" + + ext <- new_external_class("not_a_package", "X") + expect_snapshot(error = TRUE, { + method(foo, class = ext) + method_explain(foo, class = ext) + }) +}) + test_that("method explanation shows all possible methods along with matches", { foo1 := new_class(package = NULL) foo2 := new_class(foo1, package = NULL) diff --git a/tests/testthat/test-method-register.R b/tests/testthat/test-method-register.R index d198c1d3..2fe06880 100644 --- a/tests/testthat/test-method-register.R +++ b/tests/testthat/test-method-register.R @@ -64,35 +64,59 @@ test_that("method registration returns the generic unchanged when not in a packa expect_identical(out, sum) }) +test_that("method registration resolves external classes outside packages", { + env <- new.env(parent = baseenv()) + env[["method<-"]] <- `method<-` + env$g <- new_generic("g", "x") + env$ext <- new_external_class("S7", "S7_object") + env$f <- function(x) "external" + + expect_null(packageName(env)) + evalq(method(g, ext) <- f, env) + + expect_equal(env$g(S7_object()), "external") +}) + test_that("method registration returns a strippable sentinel for foreign generics in a package (#364)", { - external_methods_reset("S7") - on.exit(external_methods_reset("S7"), add = TRUE) - - foo := new_class(package = NULL) - ext <- new_external_generic("notloaded.pkg", "ext_gen", "x") - - out <- register_method( - ext, - foo, - function(x) "x", - env = asNamespace("S7"), - package = "S7" + pkg := local_package( + ext := new_external_generic("notloaded.pkg", "x"), + foo := new_class(), + foo2 := new_class() ) - expect_s3_class(out, "S7_generic_sentinel") - expect_s3_class(out, "S7_external_generic") + + # In a package, `method<-` writes a sentinel back into the binding + evalq(method(ext, foo) <- function(x) "x", pkg) + expect_s3_class(pkg$ext, "S7_generic_sentinel") + expect_s3_class(pkg$ext, "S7_external_generic") # the sentinel is still a usable generic, so further methods can be # registered through the same binding (as in the t2 test package) - foo2 := new_class(package = NULL) - out <- register_method( - out, - foo2, - function(x) "y", - env = asNamespace("S7"), - package = "S7" + evalq(method(ext, foo2) <- function(x) "y", pkg) + expect_s3_class(pkg$ext, "S7_generic_sentinel") + expect_length(S7_methods_table("pkg"), 2) +}) + +test_that("deferred external-class methods preserve sentinel for foreign generics", { + pkg := local_package( + ext := new_external_class("notloaded.pkg") + ) + + # In a package, `method<-` writes a sentinel back into the binding + evalq(method(sum, ext) <- function(x, ...) "x", pkg) + expect_s3_class(pkg$sum, "S7_generic_sentinel") + expect_s3_class(pkg$sum, "S7_external_generic") + expect_length(S7_methods_table("pkg"), 1) +}) + +test_that("method registration defers external classes in union signatures", { + pkg := local_package( + foo := new_generic("x"), + ext := new_external_class("notloaded.pkg"), + method(foo, NULL | ext) <- function(x) "x" ) - expect_s3_class(out, "S7_generic_sentinel") - expect_length(S7_methods_table("S7"), 2) + + expect_length(methods(pkg$foo), 0) + expect_length(S7_methods_table("pkg"), 1) }) test_that("method unregistration removes an S7 method via NULL assignment", { diff --git a/vignettes/packages.Rmd b/vignettes/packages.Rmd index 8988fcc9..c58d198d 100644 --- a/vignettes/packages.Rmd +++ b/vignettes/packages.Rmd @@ -58,16 +58,43 @@ If you export a class (i.e. its constructor), you must also set the `package` ar NB: if your package creates a hierarchy of classes, subclasses must be defined _after_ the parent classes. That means if you define the classes in separate files, you will need to use the `DESCRIPTION` Collate field (or the equivalent roxygen2 `@include` tag) to ensure the files are loaded in the correct order. -## Generics and methods +## Generics You should document generics like regular functions (since they are!). If you expect others to create their own methods for your generic, you may want to include a section describing the properties that you expect all methods to have. If you want to list all methods for a generic, you can use the [doclisting](https://doclisting.r-lib.org) package. +## Methods + If you use roxygen2, you can document S7 generics and methods by following the advice in [`vignette("rd-S7", package = "roxygen2")`](https://roxygen2.r-lib.org/articles/rd-S7.html). Note that methods can only be defined after both the class and generic have been defined. If generics/methods/classes live in different files, you will need to use the `DESCRIPTION` Collate field (or the equivalent roxygen2 `@include` tag) to ensure the files are loaded in the correct order. +### Methods for generics in suggested packages + +If you want to register a method for a generic defined in a package that you only suggest (rather than import), use `new_external_generic()` to refer to the generic without taking a hard dependency: + +```{r, eval = FALSE} +# In your package +median <- new_external_generic("stats", "median", "x") +method(median, MyClass) <- function(x, ...) { ... } +``` + +When the suggested package is loaded, S7 will register the method automatically (via `S7_on_load()` as described above). + +### Methods for classes in suggested packages + +Conversely, you may want to register a method for one of your own generics, dispatching on a class from a suggested package. Use `new_external_class()` to refer to the class by name: + +```{r, eval = FALSE} +# In your package +my_generic <- new_generic("my_generic", "x") +ggplot <- new_external_class("ggplot2", "ggplot") +method(my_generic, ggplot) <- function(x) { ... } +``` + +When the suggested package is loaded, S7 will register the method automatically (via `S7_on_load()` as described above). + ## Backward compatibility ### S3