[Rd] possible bug in utils::removeSource - NULL argument is silently dropped

[Martin Maechler]

>>>>> Dénes Tóth <toth.denes at kogentum.hu>
>>>>>     on Mon, 11 Dec 2017 11:39:38 +0100 writes:

    > Dear R-Core Team,

    > I found an unexpected behaviour in utils::removeSource (also present in 
    > r-devel as of today).
> ---
> 
> # create a function which accepts NULL argument
> foo <- function(x, y) {
>    if (is.null(y)) y <- "default foo"
>    attr(x, "foo") <- y
>    x
> }
> 
> # create a function which utilizes 'foo'
> testSrc <- function() {
>    x <- 1:3
>    x <- foo(x, NULL)
>    x
> }
> 
> # this works fine
> testSrc()
> 
> # this fails
> testNoSrc <- utils::removeSource(testSrc)
> testNoSrc()
> 
> # removeSource removes NULL from the 'foo' call
> print(testNoSrc)
> 
> ---
> 
> I traced back the bug to this row in removeSource:
> 
> (line 33 in sourceutils.R)
> part[[i]] <- recurse(part[[i]])
> 
> it should be (IMHO):
> part[i] <- list(recurse(part[[i]]))
> 
> ---
> 
> # create a function with the above patch
> rmSource <- function (fn) {
>    stopifnot(is.function(fn))
>    if (is.primitive(fn))
>      return(fn)
>    attr(fn, "srcref") <- NULL
>    attr(body(fn), "wholeSrcref") <- NULL
>    attr(body(fn), "srcfile") <- NULL
>    recurse <- function(part) {
>      if (is.name(part))
>        return(part)
>      attr(part, "srcref") <- NULL
>      attr(part, "wholeSrcref") <- NULL
>      attr(part, "srcfile") <- NULL
>      if (is.language(part) && is.recursive(part)) {
>        for (i in seq_along(part))
>          part[i] <- list(recurse(part[[i]]))
>      }
>      part
>    }
>    body(fn) <- recurse(body(fn))
>    fn
> }
> 
> # test
> ( testNoSrc2 <- rmSource(testSrc) )
> testNoSrc2()
> 
> 
> Regards,
> Denes

Thank you very much, Denes. This is indeed a bug 

As it happens I had wanted to improve the removeSource()
function many months ago [to make it work so it removes source attributes also
from non-function expressions] and have had a changed version of
it in my version of R-devel.

Your proposed change looks quite reasonable, and I'll try adding
it (and look at the consequences if it is ok in all use cases).

Thank you once more!

Martin Maechler
ETH Zurich