search

lorenzwalthert / precommit

pre-commit hooks for R projects
https://lorenzwalthert.github.io/precommit/
GNU General Public License v3.0
250 stars 49 forks source link

Conflicting indentation between {styler} and {lintr} #596

Open philiporlando opened 1 week ago

philiporlando commented 1 week ago

Before filing a bug

Describe the bug

I am trying to incorporate {styler} as a pre-commit hook within my R project. I have already been using {lintr} with some custom config. This is what my .lintr looks like:

linters: linters_with_defaults(
  infix_spaces_linter = NULL,
  object_length_linter = NULL,
  object_name_linter = NULL,
  object_usage_linter = NULL,
  indentation_linter = indentation_linter(hanging_indent_style = "never"))
exclusions: list("renv")

Prior to using {precommit} and {styler} calling lintr::lint_dir() was running successfully without any linting errors. However, now that I'm trying to auto-style on commit, I'm seeing the below linting errors:

/home/user/my-project/R/my-script.R:377:4: style: [indentation_linter] Indentation should be 2 spaces but is 4 spaces.
    var1, var2, var3
  ~^

Here is what my .pre-commit-config.yaml looks like:

# All available hooks: https://pre-commit.com/hooks.html
# R specific hooks: https://github.com/lorenzwalthert/precommit
repos:
-   repo: https://github.com/lorenzwalthert/precommit
    rev: v0.4.3.9001
    hooks: 
    -   id: style-files
        args: [--style_pkg=styler, --style_fun=tidyverse_style, --indent_by=2]
        files: '^.*\.[rR]$'
    -   id: spell-check
        exclude: >
          (?x)^(
          .*\.[rR]|
          .*\.feather|
          .*\.jpeg|
          .*\.pdf|
          .*\.png|
          .*\.py|
          .*\.RData|
          .*\.rds|
          .*\.Rds|
          .*\.Rproj|
          .*\.sh|
          (.*/|)\.gitignore|
          (.*/|)\.gitlab-ci\.yml|
          (.*/|)\.lintr|
          (.*/|)\.pre-commit-.*|
          (.*/|)\.Rbuildignore|
          (.*/|)\.Renviron|
          (.*/|)\.Rprofile|
          (.*/|)\.travis\.yml|
          (.*/|)appveyor\.yml|
          (.*/|)NAMESPACE|
          (.*/|)renv/settings\.dcf|
          (.*/|)renv\.lock|
          (.*/|)WORDLIST|
          \.github/workflows/.*|
          data/.*|
          )$
    -   id: lintr
        files: '^.*\.[rR]$'
    -   id: readme-rmd-rendered
        files: '^.*\.[rR]$'
    -   id: parsable-R
        files: '^.*\.[rR]$'
    -   id: no-browser-statement
        files: '^.*\.[rR]$'
    -   id: no-debug-statement
        files: '^.*\.[rR]$'
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks: 
    -   id: check-added-large-files
        args: ['--maxkb=200']
    -   id: end-of-file-fixer
        exclude: '\.Rd'
-   repo: https://github.com/pre-commit-ci/pre-commit-ci-config
    rev: v1.6.1
    hooks:
    # Only required when https://pre-commit.ci is used for config validation
    -   id: check-pre-commit-ci-config
-   repo: local
    hooks:
    -   id: forbid-to-commit
        name: Don't commit common R artifacts
        entry: Cannot commit .Rhistory, .RData, .Rds or .rds.
        language: fail
        files: '^.*\.(Rhistory|RData|Rds|rds)$'
        # `exclude: <regex>` to allow committing specific files

ci:
    autoupdate_schedule: monthly

To Reproduce

Still working on a full reprex. Will update soon.

Expected behavior

I would expect {styler} to respect my indent_by=2 argument, and to not use four spaces. While the {styler} pre-commit hook passes, the {lintr} hook fails... This is telling me that {styler} is continuing to use four spaces, instead of the expected two...

Any insight here is appreciated!

Additional context

lorenzwalthert commented 1 week ago

Thanks. This is likely unrelated to pre-commit, but a conflict between styler and lintr. Please see renv.lock of this repo for the version used for each package. That way you should be able to figure out the problem. Also note I pushed a hook release < 1 hour ago, maybe the problem resolves itself if you precommit::autoupdate().

lorenzwalthert commented 1 week ago

Note that both styler and lintr run in a separate renv (invisible to you), i.e. they don't match your global (or renv project specific) version most likely.

philiporlando commented 1 week ago 
packageVersion("styler")
# [1] ‘1.10.3’

packageVersion("lintr")
# [1] ‘3.1.2’
philiporlando commented 1 week ago

I'm able to run styler::style_dir() interactively, and it uses the indent_by=2L as expected. Only the pre-commit hook is giving me trouble.

I'm currently experiencing this error:

git commit -m 'test commit'
style-files....................................................Failed
- hook id: style-files
- exit code: 1

- One or more packages recorded in the lockfile are not installed.
- Use `renv::status()` for more details.
- One or more packages recorded in the lockfile are not installed.
- Use `renv::status()` for more details.
Error in packageVersion("precommit") : 
  there is no package called ‘precommit’
Execution halted

But I do not see this issue when loading {precommit} interactively:

renv::status()
# No issues found -- the project is in a consistent state.

packageVersion("precommit")
# [1] ‘0.4.3’

precommit::version_precommit()
# [1] "3.4.0"

precommit::autoupdate()
# ✔ Ran `pre-commit autoupdate ()` to get the latest version of the hooks.

It seems like my RStudio session and my git terminal are using separate pre-commit environments? I'm not sure where to go from here. Any troubleshooting support here is greatly appreciated!

lorenzwalthert commented 1 week ago

Note that both styler and lintr run in a separate renv (invisible to you), i.e. they don't match your global (or renv project specific) version most likely.

Again, the renv in your project is unrelated to the renv precommit uses. These are two differen renvs. The one pre-commit uses is stored in a caching directory so you don't see it. If you use the latest version of the hooks, you can check here what versions of {styler} and {lintr} are used:

They match. So you should be able to reproduce your problem with these versions.

Prior to using {precommit} and {styler} calling lintr::lint_dir() was running successfully without any linting errors. However, now that I'm trying to auto-style on commit, I'm seeing the below linting errors: [...] This is telling me that {styler} is continuing to use four spaces, instead of the expected two...

So you say that calling styler thorugh pre-commit created additional indention? Did you verify that by looking at the file contents? Maybe create a reprex, it's hard to help otherwise. Also, if styler makes changes, you need to stage them and try to commit again for the hook to pass, otherwise the hook will repeatedly fail.