r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.19k stars 184 forks source link

Handling imports from roxygen comment #1429

Open Wazarr opened 2 years ago

Wazarr commented 2 years ago

The linter indicates an object_user_linter issue for functions that have been imported using roxygen import statements. This doesn't happen on RStudio, only on VSCode. For context, I am working on a golem project.

image

AshesITR commented 2 years ago

The lints should disappear if lintr is running in a context where pkgload::load_all() has been run. What we could do to improve this would be to automatically process a NAMESPACE file if it exists.

IndrajeetPatil commented 2 years ago

in a context where pkgload::load_all() has been run

How can one detect this?

Here is one way we can list exported objects from the package root directory:

  # `{desc}` uses the `DESCRIPTION` file of the package in the working directory
  package_name <- desc::desc_get("Package")

  # to bring the library on the search path
  library(package_name, character.only = TRUE, quietly = TRUE)
  on.exit(detach(), add = TRUE)

  # exported
  package_env <- rlang::pkg_env(package_name)
  exported_objects <- rlang::env_names(package_env)
AshesITR commented 2 years ago

pkgload::load_all() would attach an environment named "package:mypkg" if the package name was mypkg. I don't think lintr should run this automatically, though. Imported objects can be obtained using base R via getNamespaceImports("mypkg").

If we don't load the package, we could parse the NAMESPACE file to generate a list of namespace imports.

MichaelChirico commented 6 months ago

How can one detect this?

we can use devtools::dev_packages(), which is a wrapper on whether pkgload::dev_meta(nm) returns NULL. So if we go this route, we can stick to "only" a dependency on {pkgload} (maybe as Suggests) by using pkgload::load_all() and pkgload::dev_meta().

AshesITR commented 6 months ago

How would we implement this? Opt-in or Opt-out?

MichaelChirico commented 6 months ago

I guess we don't have to worry as much about side effects if #912 is fulfilled

AshesITR commented 6 months ago

So object_usage_linter would check

If running inside a package and the package is not loaded and new argument auto_load is TRUE, run pkgload::load_all()?

Or is this necessary as a global feature?

NB users might want to disable linters that run potentially untrusted code, which is why we mention this fact in the docs. load_all() would also do this.