Appsilon / box.linters

lintr-compatible linters for box modules in R
https://appsilon.github.io/box.linters/
4 stars 0 forks source link

[LINT_BUG]: Error when searching for module with relative path #110

Open caldwellst opened 1 week ago

caldwellst commented 1 week ago

box.linters version

0.9.1

Sample source code to lint

Setup a simple directly with 3 files, a/x.R, which is exported using {box} and two files to import it b/y.R and z.R.

a/x.R

#' @export
x <- function() {
  print("X!")
}

b/y.R

box::use(../a/x)

x$x()

z.R

box::use(a/x)

x$x()

Lint command used

box.linters::use_box_lintr()
lintr::lint_dir()

Lint result

Error: Linter 'box_unused_attached_mod_linter' failed in .../box-lintr-test/b/y.R: unable to load module “../a/x”; not found in “..../box-lintr-test”

Expected result

Linting should have succeeded. If you delete b/y.R, lintr::lint_dir() succeeds. Reading the error message, it seems that it is not following the relevant pathing of ../path/to/module when linting. However, nested imports below the calling file that don't require going up a directory with ../ succeed.

The error is discussed in #108, but is a separate issue because this shouldn't create a lint flag, but should succeed.

radbasa commented 1 week ago

Thank you for submitting a bug report.

This looks like a fundamental difference in how {lintr} and {box} work.

Running lintr::lint_dir() at the project root uses the project root as the current working directory. When it reads b/y.R it sees ../a/x and thus {box.linters} looks for the module in ../a/x from the current working directory. That would be one up from the current working directory (the project root) then to an a folder/directory, a sibling of the project root.

Running lintr::lint_file("b/y.R") from the project root produces the same error result: unable to load module "../a/x"; not found in "...../box-lintr-test". Changing the current working directory to ./b then running lintr::lint("y.R") "fixes" the problem, but it shows how {lintr} is handling directories.

This is different from how {box} works when it sees an explicit relative path. It skips the import search path defined in box.path, instead it uses the source file's (b/y.R) location as the current directory.

For now, we would recommend explicitly setting {box}'s import search path as described in the documentation.

options(box.path = getwd())

box.path == NULL signals {box} to always use the current file's location as the module path root.

Then all module paths become relative to box.path:

b/y.R

box::use(a/x)

x$x()

Meanwhile, we will investigate providing {box.linters} support for relative module paths in box::use().