Add --root argument to lintr hook

[TymekDev]

trafficstars

I have stumbled upon {box.linters} failing to run its checks as it relies on loading relative paths. Specifying a root directory with the newly-added --root flag solves this.

Edit: I just realized that it helped in a synthetic test—when invoked by hand. I cannot figure out how to pass absolute path to the repo via pre-commit's args. I tried $PWD, pwd, $(git rev-parse --show-top-leell), but each was treated as a string and not expanded at all 🤔

[lorenzwalthert]

Edit: I just realized that it helped in a synthetic test—when invoked by hand. I cannot figure out how to pass absolute path to the repo via pre-commit's args. I tried $PWD, pwd, $(git rev-parse --show-top-leell), but each was treated as a string and not expanded at all 🤔

Sorry I don't understand what you mean. Can you provide more context?

[TymekDev]

My bad! Hopefully laying everything out will make it clear.

Context

{box.linters} relies on importing {box} modules during linting. These imports are relative—typically to the root of a repo. Running lintr::lint("app.R") in the root of a repo works without issues.

However, pre-commit or {precommit} (?) runs the hook scripts in a cache directory. This leads to the following error:

Error: Linter 'box_unused_attached_mod_linter' failed in <snip>/app.R:
unable to load module “./main”; not found in “<snip>/.cache/pre-commit/repo93s66d2m/inst/hooks/exported”

I am looking for a way fix that. What came to my mind was adding the --root argument. However, I couldn't figure out how to pass the path to the repo via the args array to --root.

Example for Testing

1. Create a directory with the files listed below
2. Install install.packages("box.linters")
3. Run Rscript -e 'lintr::lint("app.R")' - notice that it works
4. Run git init (otherwise pre-commit will complain)
5. Run pre-commit run --files app.R lintr - notice that it fails (with the above error)

.pre-commit-config.yaml

repos:
  - repo: https://github.com/lorenzwalthert/precommit
    rev: v0.4.3
    hooks:
      - id: lintr
        additional_dependencies:
          - box
          - Appsilon/[email protected]

.lintr

linters:
  linters_with_defaults(
    defaults = box.linters::rhino_default_linters
  )

app.R

box::use(
  shiny[shinyApp],
)

box::use(
  ./main,
)

shinyApp(main$ui, main$server)

main.R

box::use(
  shiny[fluidPage],
)

#' @export
ui <- fluidPage("Hello World!")

#' @export
server <- function(input, output, session) {}

[lorenzwalthert]

Sorry @TymekDev I took so long to answer. I can reproduce your problem, thanks for the detailed report. In case this was not evident already, here's how I'd adapt the .pre-commit-config.yaml file to pass the naviest form of a root to the hook

repos:
  - repo: https://github.com/lorenzwalthert/precommit
    rev: 10bd189ec3421926bdbc32aba94b873a7a597e53
    hooks:
      - id: lintr
        additional_dependencies:
          - box
          - Appsilon/[email protected]

        args: [--root=/Users/lorenz/git/precommit.box.linter]

But the error is the one you described above.

In addition, we can avoid using renv under the hood and just use the hook script with our global R package library:

repos:
  - repo: https://github.com/lorenzwalthert/precommit
    rev: 10bd189ec3421926bdbc32aba94b873a7a597e53
    hooks:
      - id: lintr
        language: script
        entry: inst/hooks/exported/lintr.R

This won't solve the problem either. The working directory at the time of hook execution is guaranteed to be the git root. You can go one level deeper and just execute the hook script that lives in the pre-commit cache transparently from the project root, and you get the same error:

$ /Users/lorenz/.cache/pre-commit/repouv8mdctt/inst/hooks/exported/lintr.R main.R 
Error: Linter 'box_unused_attached_mod_linter' failed in /Users/lorenz/git/precommit.box.linter/main.R: unable to load module “./main”; not found in “/Users/lorenz/.cache/pre-commit/repouv8mdctt/inst/hooks/exported”
Execution halted

The --root argument for other hooks is intended for the case where the R package / project of yours lives in a sub-directory instead of the git root.

Having said that, I am not sure what the problem is. It seems like box.linters::rhino_default_linters makes some kind of assumption about the file system and gets confused if the code that runs the linter lives outside of the current working directory? Another source of trouble might be that box.linters::rhino_default_linters() requires the package to be linted to be pkgload::load_all()ed. There were previous issues in this repo regarding this, e.g. #lintr.

So overall, I can't help further with this, as it seems a problem on the side of {box.linters}... 😩

[TymekDev]

@lorenzwalthert thanks for taking time to look into this!

Ultimately, I have settled for running a local script hook:

repos:
  - repo: local
    hooks:
      - id: lint-r-app
        name: run rhino::lint_r() on app/ files
        language: script
        entry: /usr/bin/env Rscript -e 'rhino::lint_r(commandArgs(trailingOnly = TRUE))'
        files: |
          (?x)^(
            \.Rprofile|
            app/.*\.R
          )$

It might not be ideal, as part of the idea of having pre-commit is separating the dev tooling from the project, but for the time being... it just works. :^) {box.linters} doesn't complain, because the hook runs in the repo and the project being a {rhino}-based application means renv.lock already has the necessary dependencies to run the hook.

I am closing this PR as it's not really solving what I thought it would. Feel free to re-open if these changes are still needed.