Appsilon / box.linters

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

[LINT_BUG]: Box.linters should be able to load box modules when the working directory is changed #142

Open osenan opened 2 weeks ago

osenan commented 2 weeks ago

box.linters version

‘0.10.3’

Sample source code to lint

This a reproducible (toy) example that fails (uploaded files with .txt extension, please replace with .R): create a rhino app with rhino::init()

  1. Add attached file [init.R ](http://init.to/)to logic folder init.txt

  2. Add test test-two_numbers.R test-two-numbers.txt to folder tests/testthat

  3. Running rhino::test_r() passes the tests.

  4. But running rhino::lint_r()

    fails, because box.linters is not following withr::with_dir.

Lint command used

linters: linters_with_defaults( defaults = box.linters::rhino_default_linters, line_length_linter = line_length_linter(100) )

rhino::lint_r()

Lint result

Linter 'box_mod_fun_exists_linter' failed in /home/osenan/Documents/Apptests/box_issues/tests/testthat/test-two-numbers.R: unable to load module “logic/init”; not found in “/home/osenan/Documents/Apptests/box_issues”, “/home/osenan/.cache/R/renv/cache/v5/linux-ubuntu-jammy/R-4.4/x86_64-pc-linux-gnu/box/1.2.0/d94049c1d9446b0abb413fde9e82a505/box/mod”, “/home/osenan/Documents/Apptests/box_issues”

There is an error in loading because it does not follows the working directory change

Expected result

lint_r shuold have been executed (could find or not style errors)

radbasa commented 1 week ago

Linters ({box.linters} or {lintr} itself), being static code analysis tools do not execute code. They cannot follow changes made by code like withr::with_*().

Linters operate within the existing current environment.

For the following case:

with_dir("../../app/",
  box::use(
    logic/init[sum_two_numbers],
  )
)

In {rhino}, the with_dir() should be unnecessary because box.path is set in .Rprofile:

options(box.path = getwd())

This would normally be the project root.

The following should work in unit tests:

  box::use(
    app/logic/init[sum_two_numbers],
  )

@osenan

radbasa commented 1 week ago

However, dealing with non-existing box module paths should be handled more gracefully. #108