Update checkS3methods to ignore deprecated/defunct functions

[hturner]

This task is to update tools::checkS3methods(), so that ignores functions where there is a call to .Deprecated() or .Defunct() in the body of a function.

This will enable a more elegant approach to fixing packages that have functions that look like S3 methods, as discussed on the CRAN Cookbook repo. This approach has the support of Kurt Hornik from the CRAN team.

tools::checkS3methods() is a pure R function, but it is a long function! So part of the task will be working through the code to understand how it could be adapted to only check functions that are not deprecated or defunct, then working on the code to implement the change.

[luciorq]

Work from today’s and yesterday’s sessions, under the guidance of @gmbecker and @lawremi, is summarized in Pull Request r-devel/r-svn#188.

Code changes were tested within the Ubuntu-based development container, while CI checks are currently in progress GH actions CI.

To the best of my knowledge, no Bugzilla issue has been opened for this yet. I am still waiting for my R Bugzilla account to be provisioned.

[luciorq]

Also find the SVN Diff file:

Index: src/library/tools/R/QC.R
===================================================================
--- src/library/tools/R/QC.R	(revision 87348)
+++ src/library/tools/R/QC.R	(working copy)
@@ -2379,6 +2379,38 @@
    res
 }
 
+### Additional functions for checkS3methods
+checkTopLevelCall <- function(expr, fun_name) {
+  if (inherits(expr, "if") || !is.call(expr)) {
+    return(FALSE)
+  }
+  fun_name <- as.name(fun_name)
+  fun_called <- expr[[1]]
+  if (is.call(fun_called)) {
+    inner_called <- fun_called[[1]]
+    if (as.character(inner_called) %in% c(":::", "::")) {
+      fun_called <- fun_called[[3]]
+    }
+  }
+  identical(fun_called, fun_name)
+}
+
+containsTopLevelCall <- function(x, fun_name) {
+  fun_body <- body(x)
+  if (inherits(fun_body, "{")) {
+    any(vapply(fun_body, checkTopLevelCall, TRUE, fun = fun_name))
+  } else {
+    checkTopLevelCall(fun_body, fun_name)
+  }
+}
+
+isDeprecated <- function(fun) {
+  containsTopLevelCall(fun, quote(.Deprecated))
+}
+isDefunct <- function(fun) {
+  containsTopLevelCall(fun, quote(.Defunct))
+}
+
 ### * checkS3methods
 
 checkS3methods <-
@@ -2603,6 +2635,11 @@
                   function(g) {
                       methods <-
                           gen_dot_cls_matches(g, functions_in_code)
+                      method_funs <-
+                          Filter(Negate(
+                              function(x) isDefunct(x) || isDeprecated(x)
+                              ), mget(methods, code_env))
+                      methods <- names(method_funs)
                       if((n <- length(methods)) > 0L) {
                           gargs <- nfg(g, code_env)
                           entries <-

[hturner]

I discussed this issue with Kurt Hornik directly - I'll ask how he would prefer to receive the patch (maybe he will just pick it up from here).

[hturner]

Feedback from Kurt:

Thanks.  I'll have a look, but I need to take a closer look to ensure
that we're get()ting functions only once, as this is slow.

(Ideally such patches would have some comments in the code explaining
what is going on.)

So I think he is happy to pick it up from here, but you / @lawremi / @gmbecker might want to follow up on his feedback.