From e9f95fb66f371a4714704a32605592cf06a88291 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 25 May 2026 15:07:20 -0500 Subject: [PATCH 1/8] Complete S3 method registration Part of #455 --- NEWS.md | 1 + R/method-register.R | 34 +++++++++----- man/method-set.Rd | 4 +- tests/testthat/_snaps/method-register.md | 6 +-- tests/testthat/test-method-register.R | 56 ++++++++++++++++++++++-- 5 files changed, 83 insertions(+), 18 deletions(-) diff --git a/NEWS.md b/NEWS.md index 795ec1fd..46b7de59 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # S7 (development version) +* `method<-` can now register methods on S3 generics with base types (e.g. `class_character`), S3 classes (`new_S3_class()`, `class_factor`, etc.), S7 unions (expanded to one registration per class), `class_any` (registered as the `default` method), and `NULL` (registered as the `NULL` method). `class_missing` is explicitly rejected since S3 dispatches on a single, always-present argument (#455). * `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). * Internal changes to support R-devel (4.6) (#592, #593, #598, #600). * `S7_error_method_not_found` now has a correct class vector without a duplicate `"error"` entry (@jjjermiah, #604) diff --git a/R/method-register.R b/R/method-register.R index d2664c2a..0cd7d28a 100644 --- a/R/method-register.R +++ b/R/method-register.R @@ -34,7 +34,9 @@ #' For S7 generics that use multiple dispatch, this must be a list of any of #' the above types. #' -#' For S3 generics, this must be a single S7 class. +#' For S3 generics, this can be any of the above types. There's one exception: +#' you can only use [class_missing] with S3 operators that support double +#' dispatch (e.g. `+` and `-`). #' #' For S4 generics, this must either be an S7 class, or a list that includes #' at least one S7 class. @@ -78,7 +80,10 @@ register_method <- function( check_method(method, generic, name = method_name(generic, signature)) register_S7_method(generic, signature, method) } else if (is_S3_generic(generic)) { - register_S3_method(generic, signature, method, env) + signatures <- flatten_signature(signature) + for (signature in signatures) { + register_S3_method(generic, signature, method, env) + } } else if (is_S4_generic(generic)) { register_S4_method(generic, signature, method, env) } @@ -99,14 +104,22 @@ register_S3_method <- function( method, envir = parent.frame() ) { - if (class_type(signature[[1]]) != "S7") { - msg <- sprintf( - "When registering methods for S3 generic %s(), signature must be an S7 class, not %s.", - generic$name, - class_friendly(signature[[1]]) - ) - stop(msg, call. = FALSE) - } + sig <- signature[[1]] + + class <- switch( + class_type(sig), + `NULL` = "NULL", + missing = stop( + "`class_missing` not supported for non-operator S3 generics.", + call. = FALSE + ), + any = "default", + S7_base = sig$class, + S7 = S7_class_name(sig), + S7_union = , # Handled above + S7_S3 = sig$class[[1]], + S4 = sig@className + ) if ( is_external_generic(external_generic <- get0(generic$name, envir = envir)) @@ -114,7 +127,6 @@ register_S3_method <- function( envir <- asNamespace(external_generic$package) } - class <- S7_class_name(signature[[1]]) registerS3method(generic$name, class, method, envir) } diff --git a/man/method-set.Rd b/man/method-set.Rd index 102855db..a5920665 100644 --- a/man/method-set.Rd +++ b/man/method-set.Rd @@ -27,7 +27,9 @@ following: For S7 generics that use multiple dispatch, this must be a list of any of the above types. -For S3 generics, this must be a single S7 class. +For S3 generics, this can be any of the above types. There's one exception: +you can only use \link{class_missing} with S3 operators that support double +dispatch (e.g. \code{+} and \code{-}). For S4 generics, this must either be an S7 class, or a list that includes at least one S7 class.} diff --git a/tests/testthat/_snaps/method-register.md b/tests/testthat/_snaps/method-register.md index 7c7310ef..354ca29e 100644 --- a/tests/testthat/_snaps/method-register.md +++ b/tests/testthat/_snaps/method-register.md @@ -6,13 +6,13 @@ Message Overwriting method foo() -# method registration / S3 registration requires a S7 class +# method registration / rejects class_missing on S3 generics Code - method(sum, new_S3_class("foo")) <- (function(x, ...) "foo") + method(s3_gen, class_missing) <- (function(x) "missing") Condition Error: - ! When registering methods for S3 generic sum(), signature must be an S7 class, not an S3 class. + ! `class_missing` not supported for non-operator S3 generics. # method registration / can register S7 method for S4 generic diff --git a/tests/testthat/test-method-register.R b/tests/testthat/test-method-register.R index 398fce0b..e234a30c 100644 --- a/tests/testthat/test-method-register.R +++ b/tests/testthat/test-method-register.R @@ -64,10 +64,60 @@ describe("method registration", { } }) - it("S3 registration requires a S7 class", { - foo <- new_class("foo") + it("can register S7 method for S3 generic with base type signature", { + s3_gen <- function(x) UseMethod("s3_gen") + method(s3_gen, class_character) <- function(x) "char" + method(s3_gen, class_integer) <- function(x) "int" + + tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] + expect_contains(ls(tbl), c("s3_gen.character", "s3_gen.integer")) + }) + + it("can register S7 method for S3 generic with S3 class signature", { + s3_gen <- function(x) UseMethod("s3_gen") + method(s3_gen, new_S3_class("foo")) <- function(x) "foo" + method(s3_gen, class_factor) <- function(x) "factor" + + tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] + expect_contains(ls(tbl), c("s3_gen.foo", "s3_gen.factor")) + }) + + it("S3 registration for a multi-class S3 class uses only the first class", { + s3_gen <- function(x) UseMethod("s3_gen") + method(s3_gen, new_S3_class(c("ordered", "factor"))) <- function(x) "ord" + + tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] + expect_setequal(ls(tbl), "s3_gen.ordered") + }) + + it("can register S7 method for S3 generic with class_any and NULL", { + s3_gen <- function(x) UseMethod("s3_gen") + method(s3_gen, class_any) <- function(x) "any" + method(s3_gen, NULL) <- function(x) "null" + + tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] + expect_contains(ls(tbl), c("s3_gen.default", "s3_gen.NULL")) + }) + + it("S3 method registration expands unions to one method per class", { + s3_gen <- function(x) UseMethod("s3_gen") + method(s3_gen, class_numeric) <- function(x) "num" + + tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] + expect_contains(ls(tbl), c("s3_gen.integer", "s3_gen.double")) + + # Custom union mixing a base type and an S3 class + s3_gen2 <- function(x) UseMethod("s3_gen2") + method(s3_gen2, class_character | new_S3_class("foo")) <- function(x) "x" + + tbl2 <- environment(s3_gen2)[[".__S3MethodsTable__."]] + expect_contains(ls(tbl2), c("s3_gen2.character", "s3_gen2.foo")) + }) + + it("rejects class_missing on S3 generics", { + s3_gen <- function(x) UseMethod("s3_gen") expect_snapshot(error = TRUE, { - method(sum, new_S3_class("foo")) <- function(x, ...) "foo" + method(s3_gen, class_missing) <- function(x) "missing" }) }) From 33533cdb987dc944eb7c4f70dff7eb41d3bda015 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 25 May 2026 16:01:36 -0500 Subject: [PATCH 2/8] Better temp name --- R/method-register.R | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/R/method-register.R b/R/method-register.R index 0cd7d28a..d02d4a38 100644 --- a/R/method-register.R +++ b/R/method-register.R @@ -80,9 +80,8 @@ register_method <- function( check_method(method, generic, name = method_name(generic, signature)) register_S7_method(generic, signature, method) } else if (is_S3_generic(generic)) { - signatures <- flatten_signature(signature) - for (signature in signatures) { - register_S3_method(generic, signature, method, env) + for (sig in flatten_signature(signature)) { + register_S3_method(generic, sig, method, env) } } else if (is_S4_generic(generic)) { register_S4_method(generic, signature, method, env) From 8e4c86ddd6d2e526c57bf3ef04f4defc83b6dab1 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 26 May 2026 13:45:52 -0500 Subject: [PATCH 3/8] Switch to testing dispatch --- tests/testthat/helper.R | 23 ++++++++++++++++ tests/testthat/test-method-register.R | 39 ++++++++++++++------------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index 2e22cec7..08c5ef66 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -60,6 +60,29 @@ local_S4_class <- function(name, ..., env = parent.frame()) { out } +# Create an S3 generic in globalenv() so that `UseMethod()` can find methods +# registered by S7 (which writes to the generic's environment's methods table). +# Cleans up the generic and any registered methods on exit. +local_s3_generic <- function(name, frame = parent.frame()) { + generic <- eval(bquote(function(x) UseMethod(.(name)))) + environment(generic) <- globalenv() + assign(name, generic, envir = globalenv()) + defer( + { + rm(list = name, envir = globalenv()) + tbl <- globalenv()[[".__S3MethodsTable__."]] + if (!is.null(tbl)) { + methods <- ls(tbl, pattern = paste0("^", name, "\\.")) + if (length(methods)) { + rm(list = methods, envir = tbl) + } + } + }, + frame = frame + ) + invisible() +} + # Lightweight equivalent of withr::defer() defer <- function(expr, frame = parent.frame(), after = FALSE) { thunk <- as.call(list(function() expr)) diff --git a/tests/testthat/test-method-register.R b/tests/testthat/test-method-register.R index e234a30c..958980ae 100644 --- a/tests/testthat/test-method-register.R +++ b/tests/testthat/test-method-register.R @@ -65,57 +65,58 @@ describe("method registration", { }) it("can register S7 method for S3 generic with base type signature", { - s3_gen <- function(x) UseMethod("s3_gen") + local_s3_generic("s3_gen") method(s3_gen, class_character) <- function(x) "char" method(s3_gen, class_integer) <- function(x) "int" - tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] - expect_contains(ls(tbl), c("s3_gen.character", "s3_gen.integer")) + expect_equal(s3_gen("a"), "char") + expect_equal(s3_gen(1L), "int") }) it("can register S7 method for S3 generic with S3 class signature", { - s3_gen <- function(x) UseMethod("s3_gen") + local_s3_generic("s3_gen") method(s3_gen, new_S3_class("foo")) <- function(x) "foo" method(s3_gen, class_factor) <- function(x) "factor" - tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] - expect_contains(ls(tbl), c("s3_gen.foo", "s3_gen.factor")) + expect_equal(s3_gen(structure(list(), class = "foo")), "foo") + expect_equal(s3_gen(factor("a")), "factor") }) it("S3 registration for a multi-class S3 class uses only the first class", { - s3_gen <- function(x) UseMethod("s3_gen") + local_s3_generic("s3_gen") method(s3_gen, new_S3_class(c("ordered", "factor"))) <- function(x) "ord" - tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] - expect_setequal(ls(tbl), "s3_gen.ordered") + expect_equal(s3_gen(ordered("a")), "ord") + # plain factors don't match because only `ordered` was registered + expect_error(s3_gen(factor("a")), "no applicable method") }) it("can register S7 method for S3 generic with class_any and NULL", { - s3_gen <- function(x) UseMethod("s3_gen") + local_s3_generic("s3_gen") method(s3_gen, class_any) <- function(x) "any" method(s3_gen, NULL) <- function(x) "null" - tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] - expect_contains(ls(tbl), c("s3_gen.default", "s3_gen.NULL")) + expect_equal(s3_gen(1L), "any") + expect_equal(s3_gen(NULL), "null") }) it("S3 method registration expands unions to one method per class", { - s3_gen <- function(x) UseMethod("s3_gen") + local_s3_generic("s3_gen") method(s3_gen, class_numeric) <- function(x) "num" - tbl <- environment(s3_gen)[[".__S3MethodsTable__."]] - expect_contains(ls(tbl), c("s3_gen.integer", "s3_gen.double")) + expect_equal(s3_gen(1L), "num") + expect_equal(s3_gen(1.5), "num") # Custom union mixing a base type and an S3 class - s3_gen2 <- function(x) UseMethod("s3_gen2") + local_s3_generic("s3_gen2") method(s3_gen2, class_character | new_S3_class("foo")) <- function(x) "x" - tbl2 <- environment(s3_gen2)[[".__S3MethodsTable__."]] - expect_contains(ls(tbl2), c("s3_gen2.character", "s3_gen2.foo")) + expect_equal(s3_gen2("a"), "x") + expect_equal(s3_gen2(structure(list(), class = "foo")), "x") }) it("rejects class_missing on S3 generics", { - s3_gen <- function(x) UseMethod("s3_gen") + local_s3_generic("s3_gen") expect_snapshot(error = TRUE, { method(s3_gen, class_missing) <- function(x) "missing" }) From 586554dff237f456bfa8b0318c688df78ae1813f Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 26 May 2026 13:49:10 -0500 Subject: [PATCH 4/8] Simplify helper a bit --- tests/testthat/helper.R | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index 08c5ef66..4b99d9eb 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -70,19 +70,25 @@ local_s3_generic <- function(name, frame = parent.frame()) { defer( { rm(list = name, envir = globalenv()) - tbl <- globalenv()[[".__S3MethodsTable__."]] - if (!is.null(tbl)) { - methods <- ls(tbl, pattern = paste0("^", name, "\\.")) - if (length(methods)) { - rm(list = methods, envir = tbl) - } - } + unregister_s3_methods(globalenv(), name) }, frame = frame ) invisible() } +unregister_s3_methods <- function(envir, generic) { + tbl <- envir[[".__S3MethodsTable__."]] + if (is.null(tbl)) { + return(invisible()) + } + methods <- ls(tbl, pattern = paste0("^", generic, "\\.")) + if (length(methods)) { + rm(list = methods, envir = tbl) + } + invisible() +} + # Lightweight equivalent of withr::defer() defer <- function(expr, frame = parent.frame(), after = FALSE) { thunk <- as.call(list(function() expr)) From 4b65a505bb4a68ce8c3c3dc63e2c164da522c63f Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 26 May 2026 13:52:17 -0500 Subject: [PATCH 5/8] Simplify helper further --- tests/testthat/helper.R | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index 4b99d9eb..578c551d 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -64,9 +64,10 @@ local_S4_class <- function(name, ..., env = parent.frame()) { # registered by S7 (which writes to the generic's environment's methods table). # Cleans up the generic and any registered methods on exit. local_s3_generic <- function(name, frame = parent.frame()) { - generic <- eval(bquote(function(x) UseMethod(.(name)))) - environment(generic) <- globalenv() - assign(name, generic, envir = globalenv()) + eval( + bquote(.(name) <- function(x) UseMethod(.(name))), + globalenv() + ) defer( { rm(list = name, envir = globalenv()) @@ -79,13 +80,11 @@ local_s3_generic <- function(name, frame = parent.frame()) { unregister_s3_methods <- function(envir, generic) { tbl <- envir[[".__S3MethodsTable__."]] - if (is.null(tbl)) { - return(invisible()) - } - methods <- ls(tbl, pattern = paste0("^", generic, "\\.")) - if (length(methods)) { + if (!is.null(tbl)) { + methods <- ls(tbl, pattern = paste0("^", generic, "\\.")) rm(list = methods, envir = tbl) } + invisible() } From 5daa3b31fa4d2af1aef793a379cffd3b4d04ba3c Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 26 May 2026 13:53:24 -0500 Subject: [PATCH 6/8] Feedback from review --- R/method-register.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/method-register.R b/R/method-register.R index d02d4a38..22d18620 100644 --- a/R/method-register.R +++ b/R/method-register.R @@ -115,7 +115,7 @@ register_S3_method <- function( any = "default", S7_base = sig$class, S7 = S7_class_name(sig), - S7_union = , # Handled above + S7_union = stop("Unreachable"), S7_S3 = sig$class[[1]], S4 = sig@className ) From 35e9db8f326c1b4540243cd9eb4446abe0f6bd38 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 29 May 2026 14:29:09 -0500 Subject: [PATCH 7/8] Register local methods --- R/method-register.R | 32 ++++++++++++++++++++++----- tests/testthat/test-method-register.R | 13 +++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/R/method-register.R b/R/method-register.R index 4cc38d80..663013e2 100644 --- a/R/method-register.R +++ b/R/method-register.R @@ -176,13 +176,35 @@ register_S3_method <- function( S4 = sig@className ) - if ( - is_external_generic(external_generic <- get0(generic$name, envir = envir)) - ) { - envir <- asNamespace(external_generic$package) + if (is_local_s3_generic(generic)) { + register_local_s3_method(generic, class, method) + } else { + # Register external generics in their own namespace + external_generic <- get0(generic$name, envir = envir) + if (is_external_generic(external_generic)) { + envir <- asNamespace(external_generic$package) + } + registerS3method(generic$name, class, method, envir) } +} - registerS3method(generic$name, class, method, envir) +# `registerS3method()` registers into the S3 methods table of +# `environment(generic)`, but `UseMethod()` dispatches using the table of +# `topenv(environment(generic))`. These are the same for package and global +# generics, but differ for a generic defined in a local environment. +is_local_s3_generic <- function(generic) { + env <- environment(generic$generic) + !is.null(env) && !identical(env, topenv(env)) +} +register_local_s3_method <- function(generic, class, method) { + dispatch_env <- topenv(environment(generic$generic)) + table <- dispatch_env[[".__S3MethodsTable__."]] + if (is.null(table)) { + table <- new.env(parent = baseenv()) + dispatch_env[[".__S3MethodsTable__."]] <- table + } + assign(paste(generic$name, class, sep = "."), method, envir = table) + invisible(method) } register_S7_method <- function(generic, signature, method) { diff --git a/tests/testthat/test-method-register.R b/tests/testthat/test-method-register.R index 616d662d..bd3fcc37 100644 --- a/tests/testthat/test-method-register.R +++ b/tests/testthat/test-method-register.R @@ -64,6 +64,19 @@ describe("method registration", { } }) + it("can register S7 method for S3 generic defined in a local environment", { + s3_gen <- local(function(x) UseMethod("s3_gen")) + defer(unregister_s3_methods(topenv(environment(s3_gen)), "s3_gen")) + + local({ + method(s3_gen, class_character) <- function(x) "char" + method(s3_gen, class_integer) <- function(x) "int" + }) + + expect_equal(s3_gen("a"), "char") + expect_equal(s3_gen(1L), "int") + }) + it("can register S7 method for S3 generic with base type signature", { local_s3_generic("s3_gen") method(s3_gen, class_character) <- function(x) "char" From b59cf92c3fcfd28ae2b95868f423a363cda57f8c Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 29 May 2026 15:05:18 -0500 Subject: [PATCH 8/8] Move news bullet to correct place --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 8dd3c4a3..6b42f35f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,5 @@ # S7 (development version) -* `method<-` can now register methods on S3 generics with base types (e.g. `class_character`), S3 classes (`new_S3_class()`, `class_factor`, etc.), S7 unions (expanded to one registration per class), `class_any` (registered as the `default` method), and `NULL` (registered as the `NULL` method). `class_missing` is explicitly rejected since S3 dispatches on a single, always-present argument (#455). * Errors thrown by S7 now report the function where they occurred, making it easier to track down the source of a problem (#646). * `method<-` now accepts `NULL` to unregister an existing method, e.g. `method(foo, class_character) <- NULL` (#613). * `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). @@ -10,6 +9,7 @@ * `convert()` no longer errors when `from` is a base or S3 object and `to` is an S7 class that inherits from `from`'s class. The base/S3 value is now passed as `.data` to the `to` constructor (#537). * `method<-` now gives a clear error when assigning a primitive function (e.g. `log`) as a method (#608). * `method<-` and `method()` now accept a length-1 list as `signature` for single-dispatch generics, matching the list-of-classes form required for multi-dispatch (#555). +* `method<-` can now register methods on S3 generics with base types (e.g. `class_character`), S3 classes (`new_S3_class()`, `class_factor`, etc.), S7 unions (expanded to one registration per class), `class_any` (registered as the `default` method), and `NULL` (registered as the `NULL` method). `class_missing` is explicitly rejected since S3 dispatches on a single, always-present argument (#455). * `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_object()` now gives an informative error when `.parent` is a class specification rather than an instance of the parent class (#409). * `S7_inherits()` and `check_is_S7()` now accept any class specification (S7 class, S7 union, S3 class, S4 class, or base type wrapper like `class_integer`), not just S7 classes (#556).