Unexport unused s3 methods (#147)

* Initial progress wrapping unused exported S3 methods with a warning * Warn of future un-export of many S3 methods * nocov all the new code
r-lib · Jan 31, 2025 · 1da901a · 1da901a
1 parent 99aea44
commit 1da901a
Show file tree

Hide file tree

Showing 2 changed files with 226 additions and 0 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -16,6 +16,14 @@
   available or (2) namespace-qualify all {bit} calls with `bit::`; adding {bit} to `Imports:` or
   `Suggests:` will also be necessary.
 
+2. The following S3 methods now generate a warning if called directly (but not if called properly,
+   i.e. _via_ S3 dispatch on the generic):
+
+   `:.default`, `:.integer64`, `[.integer64`, `[[.integer64`, `[[<-.integer64`, `%in%.default`, `%in%.integer64`, `length<-.integer64`, `all.equal.integer64`, `as.bitstring.integer64`, `as.integer64.factor`, `as.integer64.integer64`, `as.integer64.NULL`, `as.list.integer64`, `as.logical.integer64`, `cbind.integer64`, `diff.integer64`, `duplicated.integer64`, `hashdup.cache_integer64`, `hashfin.cache_integer64`, `hashfun.integer64`, `hashmap.integer64`, `hashmaptab.integer64`, `hashmapuni.integer64`, `hashmapupo.integer64`, `hashpos.cache_integer64`, `hashrev.cache_integer64`, `hashrin.cache_integer64`, `hashtab.cache_integer64`, `hashuni.cache_integer64`, `hashupo.cache_integer64`, `is.double.default`, `is.double.integer64`, `is.finite.integer64`, `is.infinite.integer64`, `is.nan.integer64`, `is.sorted.integer64`, `is.vector.integer64`, `keypos.integer64`, `match.default`, `match.integer64`, `mean.integer64`, `median.integer64`, `mergeorder.integer64`, `mergesort.integer64`, `mergesortorder.integer64`, `na.count.integer64`, `nties.integer64`, `nunique.integer64`, `nvalid.integer64`, `order.default`, `order.integer64`, `orderdup.integer64`, `orderfin.integer64`, `orderkey.integer64`, `ordernut.integer64`, `orderpos.integer64`, `orderqtl.integer64`, `orderrnk.integer64`, `ordertab.integer64`, `ordertie.integer64`, `orderuni.integer64`, `orderupo.integer64`, `prank.integer64`, `print.bitstring`, `qtile.integer64`, `quantile.integer64`, `quickorder.integer64`, `quicksort.integer64`, `quicksortorder.integer64`, `radixorder.integer64`, `radixsort.integer64`, `radixsortorder.integer64`, `ramorder.integer64`, `ramsort.integer64`, `ramsortorder.integer64`, `rank.default`, `rbind.integer64`, `scale.integer64`, `shellorder.integer64`, `shellsort.integer64`, `shellsortorder.integer64`, `sort.integer64`, `sortfin.integer64`, `sortnut.integer64`, `sortorderdup.integer64`, `sortorderkey.integer64`, `sortorderpos.integer64`, `sortorderrnk.integer64`, `sortordertab.integer64`, `sortordertie.integer64`, `sortorderuni.integer64`, `sortorderupo.integer64`, `sortqtl.integer64`, `sorttab.integer64`, `sortuni.integer64`, `summary.integer64`, `table.integer64`, `tiepos.integer64`, `unipos.integer64`
+
+   As noted in the notes for 4.6.0-1, there was no recorded instance of users calling these methods
+   directly on GitHub; in the next release, they will be removed from the NAMESPACE.
+
 ## NOTES
 
 1. {bit64} no longer prints any start-up messages through an `.onAttach()` hook (#106). Thanks @hadley for the request.

diff --git a/R/zzz.R b/R/zzz.R
@@ -11,4 +11,222 @@
 .onUnload <- function(libpath) {
   library.dynam.unload("bit64", libpath)
 }
+
+generic_call_in_stack <- function(generic_name) {
+  calls = sys.calls()
+  # we define `:.default` --> avoid infinite loop
+  for (jj in base::`:`(length(calls), 1L)) {
+    if (identical(calls[[jj]][[1L]], as.name(generic_name))) return(TRUE)
+  }
+  FALSE
+}
+
+# some, but not all, primitives are totally absent from sys.calls(). try
+#   and separate these two classes of is.primitive functions
+primitive_generic_appears_in_stack  = function(generic_name) {
+  generic = get(generic_name)
+  generic_nm = as.name(generic_name)
+  if (!is.primitive(generic)) return(TRUE)
+
+  fake_method = paste0(generic_name, ".xxx")
+  eval(bquote({
+    .(fake_method) = function(...) {
+      sc = sys.calls()
+      would_be_generic_call = sc[[length(sc) - 1L]]
+      identical(would_be_generic_call[[1L]], .(generic_nm))
+    }
+    generic_args = args(generic)
+    n_args = if (is.null(generic_args)) 1L else length(formals(generic_args))
+    call_args = vector("list", n_args)
+    d = 1L
+    class(d) = "xxx"
+    call_args[[1L]] = d
+    do.call(.(generic_nm), call_args)
+  }))
+}
+
+deprecate_exported_s3_methods <- function(..., verbose=FALSE) {
+  methods <- list(...)
+  method_names <- vapply(substitute(list(...))[-1L], deparse, character(1L))
+  # this happens to work here -- no affected classes use '.', so the class is the last part
+  generic_method <- vapply(
+    strsplit(method_names, ".", fixed=TRUE),
+    function(parts) c(paste(head(parts, -1L), collapse="."), tail(parts, 1L)),
+    character(2L)
+  )
+
+  if (verbose) primitives = character()
+
+  for (ii in seq_along(methods)) {
+    method <- methods[[ii]]
+    method_name <- method_names[ii]
+    generic_name <- generic_method[1L, ii]
+
+    # call stack may/may not work correctly for primitives. optionally report what's skipped.
+    if (!primitive_generic_appears_in_stack(generic_name)) {
+      if (verbose) primitives = c(primitives, generic_name)
+      next
+    }
+
+    # Prepend the warning check to the function body
+    warning_expr <- bquote(if (!generic_call_in_stack(.(generic_name))) {
+      warning(
+        "Don't call '", .(method_name), "' directly. ",
+        "Instead only call '", .(generic_name), "' and rely on S3 dispatch. ",
+        "In the next version, this symbol will stop being exported.",
+        domain=NA
+      )
+    })
+
+    # in 'function(x) x', body() is a name --> subsetting breaks
+    # if un-braced, as.list() will break up the first (and only) expression
+    if (!is.name(body(method)) && identical(body(method)[[1L]], as.name("{"))) {
+      exprs <- as.list(body(method)[-1L])
+    } else {
+      exprs <- body(method)
+    }
+    body(method) <- as.call(c(as.name("{"), warning_expr, exprs))
+
+    assign(method_name, method, envir = parent.frame())
+  }
+
+  if (verbose && length(primitives))
+    cat(sprintf("Skipped these primitive functions: %s\n", toString(sort(unique(primitives)))))
+  invisible()
+}
+
+# commented out: those primitives which don't appear in the call stack
+deprecate_exported_s3_methods(
+  # `-.integer64`,
+  `:.default`,
+  `:.integer64`,
+  # `!.integer64`,
+  # `!=.integer64`,
+  `[.integer64`,
+  `[[.integer64`,
+  `[[<-.integer64`,
+  # `*.integer64`,
+  # `/.integer64`,
+  # `&.integer64`,
+  # `%/%.integer64`,
+  # `%%.integer64`,
+  `%in%.default`,
+  `%in%.integer64`,
+  # `^.integer64`,
+  # `+.integer64`,
+  # `<.integer64`,
+  # `<=.integer64`,
+  # `==.integer64`,
+  # `>.integer64`,
+  # `>=.integer64`,
+  # `|.integer64`,
+  `length<-.integer64`,
+  all.equal.integer64,
+  as.bitstring.integer64,
+  as.integer64.factor,
+  as.integer64.integer64,
+  as.integer64.NULL,
+  as.list.integer64,
+  as.logical.integer64,
+  cbind.integer64,
+  # ceiling.integer64,
+  # cummax.integer64,
+  # cummin.integer64,
+  # cumprod.integer64,
+  # cumsum.integer64,
+  diff.integer64,
+  duplicated.integer64,
+  # floor.integer64,
+  hashdup.cache_integer64,
+  hashfin.cache_integer64,
+  hashfun.integer64,
+  hashmap.integer64,
+  hashmaptab.integer64,
+  hashmapuni.integer64,
+  hashmapupo.integer64,
+  hashpos.cache_integer64,
+  hashrev.cache_integer64,
+  hashrin.cache_integer64,
+  hashtab.cache_integer64,
+  hashuni.cache_integer64,
+  hashupo.cache_integer64,
+  is.double.default,
+  is.double.integer64,
+  is.finite.integer64,
+  is.infinite.integer64,
+  is.nan.integer64,
+  is.sorted.integer64,
+  is.vector.integer64,
+  keypos.integer64,
+  # log10.integer64,
+  # log2.integer64,
+  match.default,
+  match.integer64,
+  mean.integer64,
+  median.integer64,
+  mergeorder.integer64,
+  mergesort.integer64,
+  mergesortorder.integer64,
+  na.count.integer64,
+  nties.integer64,
+  nunique.integer64,
+  nvalid.integer64,
+  order.default,
+  order.integer64,
+  orderdup.integer64,
+  orderfin.integer64,
+  orderkey.integer64,
+  ordernut.integer64,
+  orderpos.integer64,
+  orderqtl.integer64,
+  orderrnk.integer64,
+  ordertab.integer64,
+  ordertie.integer64,
+  orderuni.integer64,
+  orderupo.integer64,
+  prank.integer64,
+  print.bitstring,
+  # prod.integer64,
+  qtile.integer64,
+  quantile.integer64,
+  quickorder.integer64,
+  quicksort.integer64,
+  quicksortorder.integer64,
+  radixorder.integer64,
+  radixsort.integer64,
+  radixsortorder.integer64,
+  ramorder.integer64,
+  ramsort.integer64,
+  ramsortorder.integer64,
+  # range.integer64,
+  rank.default,
+  rbind.integer64,
+  # round.integer64,
+  scale.integer64,
+  shellorder.integer64,
+  shellsort.integer64,
+  shellsortorder.integer64,
+  # sign.integer64,
+  # signif.integer64,
+  sort.integer64,
+  sortfin.integer64,
+  sortnut.integer64,
+  sortorderdup.integer64,
+  sortorderkey.integer64,
+  sortorderpos.integer64,
+  sortorderrnk.integer64,
+  sortordertab.integer64,
+  sortordertie.integer64,
+  sortorderuni.integer64,
+  sortorderupo.integer64,
+  sortqtl.integer64,
+  sorttab.integer64,
+  sortuni.integer64,
+  # sqrt.integer64,
+  summary.integer64,
+  table.integer64,
+  tiepos.integer64,
+  # trunc.integer64,
+  unipos.integer64
+)
 # nocov end