r-lib / devtools

Tools to make an R developer's life easier
https://devtools.r-lib.org
Other
2.37k stars 755 forks source link

devtools::load_all() will run a function that is named "search" in the target package #2458

Closed oganm closed 1 year ago

oganm commented 1 year ago

Is your feature request related to a problem? Please describe.

I have an example package. This package contains only two functions. search and something_else. When working on this package if I use

devtools::load_all()

I will see that the function named search will be executed twice. When debugging what is going on under the hood, I have tried using

pkgload::load_all()

and

# this is a copy of load_all from devtools with explicit function names
copy_devtools_load_all = function (path = ".", reset = TRUE, recompile = FALSE, export_all = TRUE, 
                 helpers = TRUE, quiet = FALSE, ...) 
{
    if (inherits(path, "package")) {
        path <- path$path
    }
    devtools:::save_all()
    ellipsis:::check_dots_used(action = getOption("devtools.ellipsis_action", 
                                                  rlang::warn))
    pkgload::load_all(path = path, reset = reset, recompile = recompile, 
                      export_all = export_all, helpers = helpers, quiet = quiet, 
                      ...)
}
copy_devtools_load_all()

neither seem to have the same issue so I am unsure what is causing this. Similarly, running devtools::load_all() from outside Rstudio doesn't seem to cause this issue. However it seems that defining asearch function in the global environment will also cause the issue to be replicated so non-explicit function call seems to be a likely culprit

# in any package with or a function named `search`
search = function(){
    print('why me')
}
devtools::load_all()

Describe the solution you'd like Functions should not be executed when loading a package based on what their names are.

Describe alternatives you've considered Avoiding the name "search" or using pkgload fixes this issue but neither is ideal

jennybc commented 1 year ago

base::search() is a pretty fundamental function, so my first quick reaction is that it seems like asking for trouble to mask it by defining a function named search() in your package.

(I'll have to look into whether there's something special about masking search() and devtools.)

jennybc commented 1 year ago

Using your package, I can replicate this in RStudio and it does not replicate outside of RStudio. So I think it's the IDE that's calling search(), not devtools.

Has a similar vibe to https://github.com/rstudio/rstudio/issues/10089, https://github.com/rstudio/rstudio/issues/8031

@kevinushey Should @oganm open an issue in https://github.com/rstudio/rstudio?