Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --root argument to lintr hook #600

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TymekDev
Copy link
Contributor

@TymekDev TymekDev commented Oct 18, 2024

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
Copy link
Owner

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
Copy link
Contributor Author

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/box.linters@v0.10.5

.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) {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants