Add a pre-commit config

[Goldziher]

Hi, using this tool via pre-commit would be awesome.

This requires adding a yaml entry point to the repository and an install script - it can be in Python or a shell script.

[chavacava]

Hi @Goldziher thank you for the proposal. I've checked the documentation of pre-commit and the yaml entry point like

-   id: revive
    name: revive
    description: Beautiful Go linter
    entry: revive
    language: golang

seems enough (i.e., no install script required) because the tool will do a go install on the project. Is my understanding correct?

[Goldziher]

Hi @Goldziher thank you for the proposal. I've checked the documentation of pre-commit and the yaml entry point like

-   id: revive
    name: revive
    description: Beautiful Go linter
    entry: revive
    language: golang

seems enough (i.e., no install script required) because the tool will do a go install on the project. Is my understanding correct?

I'm not sure, but I can test this

[mfederowicz]

Hmm I also read article about pre-commit : https://goangle.medium.com/golang-improving-your-go-project-with-pre-commit-hooks-a265fad0e02f and of course it depends :) My opinion is that I like keep all tools and script for project in my repo. I dont like depends on external repos (linters and basic hooks we can configure in git without external tools. What if author of original repo change his mind? )

[chavacava]

@mfederowicz adding the yaml entry point to revive repo will just make it available to pre-commit users; up to them to hook revive or not.

[mfederowicz]

hmm in that case ok :)

[mfederowicz]

hi @chavacava i made some basic files for config and hooks, but basic hook need some tweaks to work properly, can you take a look on that?

[chavacava]

I did some tests and a .pre-commit-hooks.yaml like the following makes the job

-   id: revive
    name: revive
    description: Beautiful Go linter
    entry: revive
    args: [./...]
    language: golang
    types: [go]

On user side, with a .pre-commit-config.yaml like the following

repos:
-   repo: https://github.com/mgechev/revive
    rev: feature/precommit
    hooks:
    -   id: revive
        language: golang

When running:

pre-commit run --all-files

I get

[WARNING] The 'rev' field of repo 'https://github.com/mgechev/revive' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
revive...................................................................Passed

The warn is about pointing to a branch (feature/precommit) instead to a tagged release.

Anyway, the .pre-commit-hooks.yaml is opinionated because it sets revive arguments thus it will certainly not satisfy everybody's expectations on how the hook must behave. A more flexible approach seems to be to declare hooks out of linter's repos. For example, this repo makes available a large set of hooks and provides configuration entry points to fine tune the behavior of the linters. IMO this approach it's way more convenient than providing a .pre-commit-hooks.yaml at the linter repository.

[mfederowicz]

ok with something like:

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.4.0
    hooks:
      - id: end-of-file-fixer
      - id: trailing-whitespace
  - repo: https://github.com/TekWizely/pre-commit-golang
    rev: v1.0.0-rc.1
    hooks:
      -   id: go-revive
      -   id: go-revive-mod
      -   id: go-revive-repo-mod

i have results like:

git commit -am 'pre-commit: test'                                                                                                                                         
fix end of files.........................................................Passed                                                                                              
trim trailing whitespace.................................................Passed                                                                                              
go-revive................................................................Passed                                                                                              
go-revive-mod............................................................Passed                                                                                              
go-revive-repo-mod.......................................................Passed                                                                                              
[pre-commit-config cd4b14b] pre-commit: test                                                                                                                                 
 1 file changed, 1 insertion(+), 1 deletion(-)

i think all above hooks passes because i dont modify any files

of course if run command: pre-commit run --all-files not all hooks passes

pre-commit run --all-files                                                                                                                                                
fix end of files.........................................................Passed                                                                                              
trim trailing whitespace.................................................Passed                                                                                              
go-revive................................................................Failed                                                                                              
- hook id: go-revive                                                                                                                                                         
- exit code: 1                                                                                                                                                               

testdata/confusing-naming1.go:7:7: method receiver 't' is not referenced in method's body, consider removing or renaming it as _                                             
testdata/confusing-naming1.go:11:7: method receiver 't' is not referenced in method's body, consider removing or renaming it as _                                            
testdata/confusing-naming1.go:17:7: method receiver 't' is not referenced in method's body, consider removing or renaming it as _

of course I understand that not in every commit we commit all files from repo, so content of .pre-commit-config.yaml mentioned at the begining of this comment looks ok for this issue, what you think @chavacava ?

[chavacava]

1. I do not see the utility of using pre-commit hooks in revive
2. It could be possible to add a .pre-commit-hooks.yaml to revive github repository but, as I mentioned in my previous comment, the hook definition will be necessarily too opinionated thus using ad-hoc repositories to declare hooks (like this repo) seems to me a better approach.

To resume: my feeling is that revive repository should not include any kind of pre-commit content.

[mfederowicz]

ok, but we can leave basic config:

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.4.0
    hooks:
      - id: end-of-file-fixer
      - id: trailing-whitespace

after run pre-commit run --all-files

I have few modified files:

        modified:   Makefile
        modified:   README.md
        modified:   RULES_DESCRIPTIONS.md
        modified:   assets/demo.svg
        modified:   config/testdata/enable2.toml
        modified:   config/testdata/enableAllBut2.toml
        modified:   config/testdata/rule-level-exclude-850.toml
        modified:   testdata/confusing-results.go
        modified:   testdata/duplicated-imports.go
        modified:   testdata/golint/blank-import-with-embed.go
        modified:   testdata/golint/error-strings.go
        modified:   testdata/imports-blacklist-original.go
        modified:   testdata/redundant-import-alias.go

of course the same result we can achieve with other tools (by the way we can add other PR to fix end of file) than pre-commit so decision is depends on you @chavacava :)

[Goldziher]

Pre-commit is a very powerful orchestrator. Especially when you have multi-language repositories, as I do. A lot of go tools support it, e.g. golangci-lint for example. Or other things, such as hadolint, shellcheck, buf etc.

Here for example is my current .pre-commit-config.yaml:

default_stages: [commit]
repos:
    - repo: https://github.com/alessandrojcm/commitlint-pre-commit-hook
      rev: 'v9.5.0'
      hooks:
          - id: commitlint
            stages: [commit-msg]
    - repo: https://github.com/pre-commit/pre-commit-hooks
      rev: 'v4.4.0'
      hooks:
          - id: trailing-whitespace
          - id: end-of-file-fixer
            exclude: '.idea/modules'
          - id: check-yaml
          - id: check-added-large-files
    - repo: https://github.com/koalaman/shellcheck-precommit
      rev: v0.9.0
      hooks:
          - id: shellcheck
            exclude: 'gradle*'
    - repo: https://github.com/hadolint/hadolint
      rev: v2.12.1-beta
      hooks:
          - id: hadolint-docker
    - repo: https://github.com/pre-commit/mirrors-prettier
      rev: 'v3.0.3'
      hooks:
          - id: prettier
            exclude: 'go.mod|gen/ts|.idea/modules'
    - repo: https://github.com/pre-commit/mirrors-eslint
      rev: 'v8.51.0'
      hooks:
          - id: eslint
            files: \.tsx?$
            types: [file]
            args: [--fix]
            exclude: 'gen/ts'
    - repo: https://github.com/golangci/golangci-lint
      rev: 'v1.54.2'
      hooks:
          - id: golangci-lint
            args: ['--timeout', '3m0s']
            exclude: 'gen/go'
    - repo: https://github.com/segmentio/golines
      rev: 'v0.11.0'
      hooks:
          - id: golines
    - repo: https://github.com/bufbuild/buf
      rev: v1.27.0
      hooks:
          - id: buf-format
          - id: buf-lint
    - repo: https://github.com/sqlfluff/sqlfluff
      rev: 2.3.2
      hooks:
          - id: sqlfluff-fix
            exclude: 'sql/migrations'
    - repo: local
      hooks:
          - id: detekt
            name: detekt
            description: Runs `detekt` on modified .kt files.
            language: script
            entry: .bin/detekt.sh
            files: \.kt