pre-commit hook runs on all files instead of just changed files in the repo

[coderavels]

I want to run the tool only on the changed files since I am integrating it to an existing repo which is huge. Fixing all the errors at once is not practical.
I am using the run: new config in the .golangci.yml but it still runs on all the files not just the changed files.

I have created this small repo to mock the issue: https://github.com/heisfullstacked/golintci-test
If I uncomment the comment in the file service/should_be_lint.go and try to commit, I am expecting the pre-commit to only show errors in that file.
Instead I see errors from both main.go and service/should_be_lint.go

Trim Trailing Whitespace.................................................Passed
Fix End of Files.........................................................Passed
Check Yaml...........................................(no files to check)Skipped
Check for added large files..............................................Passed
golangci-lint............................................................Failed
- hook id: golangci-lint
- exit code: 1

service/should_be_lint.go:7:6: `change` is unused (deadcode)
type change string
     ^
main.go:14:1: commentFormatting: put a space between `//` and comment text (gocritic)
//type change string
^

Additional Information

• [x ] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
• [ x] Yes, I've searched similar issues on GitHub and didn't find any.
• [ x] Yes, I've included all information below (version, config, etc).

Version of golangci-lint

$ golangci-lint --version
# golangci-lint has version 1.27.0 built from fb74c2e on 2020-05-13T18:45:55Z

Config file

$ cat .golangci.yml
linters-settings:
  depguard:
    list-type: blacklist
    packages:
      # logging is allowed only by logutils.Log, logrus
      # is allowed to use only in logutils package
      - github.com/sirupsen/logrus
    packages-with-error-message:
      - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
  dupl:
    threshold: 100
  funlen:
    lines: 100
    statements: 50
  goconst:
    min-len: 2
    min-occurrences: 2
  gocritic:
    enabled-tags:
      - diagnostic
      - experimental
      - opinionated
      - performance
      - style
    disabled-checks:
      - dupImport # https://github.com/go-critic/go-critic/issues/845
      - ifElseChain
      - octalLiteral
      - whyNoLint
      - wrapperFunc
  gocyclo:
    min-complexity: 15
  goimports:
    local-prefixes: github.com/golangci/golangci-lint
  golint:
    min-confidence: 0
  gomnd:
    settings:
      mnd:
        # don't include the "operation" and "assign"
        checks: argument,case,condition,return
  govet:
    check-shadowing: true
    settings:
      printf:
        funcs:
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
  lll:
    line-length: 140
  maligned:
    suggest-new: true
  misspell:
    locale: US
  nolintlint:
    allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space)
    allow-unused: false # report any unused nolint directives
    require-explanation: false # don't require an explanation for nolint directives
    require-specific: false # don't require nolint directives to be specific about which linter is being skipped

linters:
  # please, do not use `enable-all`: it's deprecated and will be removed soon.
  # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
  disable-all: true
  enable:
    - bodyclose
    - deadcode
    - depguard
    - dogsled
    - dupl
    - errcheck
    - funlen
    - gochecknoinits
    - goconst
    - gocritic
    - gocyclo
    - gofmt
    - goimports
    - golint
    - gomnd
    - goprintffuncname
    - gosec
    - gosimple
    - govet
    - ineffassign
    - interfacer
    - lll
    - misspell
    - nakedret
    - nolintlint
    - rowserrcheck
    - scopelint
    - staticcheck
    - structcheck
    - stylecheck
    - typecheck
    - unconvert
    - unparam
    - unused
    - varcheck
    - whitespace

  # don't enable:
  # - asciicheck
  # - gochecknoglobals
  # - gocognit
  # - godot
  # - godox
  # - goerr113
  # - maligned
  # - nestif
  # - prealloc
  # - testpackage
  # - wsl

issues:
  # Excluding configuration per-path, per-linter, per-text and per-source
  exclude-rules:
    - path: _test\.go
      linters:
        - gomnd

    # https://github.com/go-critic/go-critic/issues/926
    - linters:
        - gocritic
      text: "unnecessaryDefer:"

run:
  tests: false
  new: true

# golangci.com configuration
# https://github.com/golangci/golangci/wiki/Configuration
service:
  golangci-lint-version: 1.23.x # use the fixed version to not introduce new linters unexpectedly
  prepare:
    - echo "here I can run custom commands, but no preparation needed for this repo"

Go environment

$ go version && go env
go version go1.14.2 darwin/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/himanshu.tanwar/Library/Caches/go-build"
GOENV="/Users/himanshu.tanwar/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY="source.golabs.io"
GONOSUMDB="source.golabs.io"
GOOS="darwin"
GOPATH="/Users/himanshu.tanwar/go"
GOPRIVATE="source.golabs.io"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.14.2_1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.14.2_1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/himanshu.tanwar/Projects/go/golintci-test/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/hv/w2lzfbyx1mn5ms4ft9v22l980000gn/T/go-build355002043=/tmp/go-build -gno-record-gcc-switches -fno-common"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /Users/himanshu.tanwar/Projects/go/golintci-test /Users/himanshu.tanwar/Projects/go /Users/himanshu.tanwar/Projects /Users/himanshu.tanwar /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 36 linters: [bodyclose deadcode depguard dogsled dupl errcheck funlen gochecknoinits goconst gocritic gocyclo gofmt goimports golint gomnd goprintffuncname gosec gosimple govet ineffassign interfacer lll misspell nakedret nolintlint rowserrcheck scopelint staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace]
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|exports_file|files|imports|name|types_sizes) took 250.694773ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 4.2135ms
INFO [linters context/goanalysis] analyzers took 68.950879ms with top 10 stages: buildir: 4.317008ms, depguard: 3.415568ms, gocritic: 2.861624ms, buildssa: 2.790626ms, fact_deprecated: 2.569307ms, golint: 2.101327ms, gofmt: 2.082654ms, goimports: 2.055769ms, printf: 1.98278ms, ineffassign: 1.957666ms
INFO [linters context/goanalysis] analyzers took 1.309422ms with top 10 stages: buildir: 783.497µs, U1000: 525.925µs
INFO [runner] Issues before processing: 6, after processing: 2
INFO [runner] Processors filtering stat (out/in): filename_unadjuster: 6/6, identifier_marker: 6/6, exclude: 2/6, source_code: 2/2, max_per_file_from_linter: 2/2, path_shortener: 2/2, path_prettifier: 6/6, skip_dirs: 6/6, autogenerated_exclude: 6/6, nolint: 2/2, uniq_by_line: 2/2, diff: 2/2, max_same_issues: 2/2, cgo: 6/6, skip_files: 6/6, exclude-rules: 2/2, max_from_linter: 2/2
INFO [runner] processing took 1.04233ms with stages: autogenerated_exclude: 257.78µs, exclude: 244.587µs, identifier_marker: 190.741µs, nolint: 119.081µs, path_prettifier: 98.312µs, source_code: 86.039µs, skip_dirs: 23.822µs, exclude-rules: 6.093µs, filename_unadjuster: 5.42µs, uniq_by_line: 2.321µs, max_same_issues: 2.089µs, cgo: 1.86µs, path_shortener: 1.484µs, max_from_linter: 1.218µs, max_per_file_from_linter: 685ns, diff: 432ns, skip_files: 366ns
INFO [runner] linters took 476.850467ms with stages: goanalysis_metalinter: 472.739384ms, unused: 2.980752ms
service/should_be_lint.go:7:1: commentFormatting: put a space between `//` and comment text (gocritic)
//type change string
^
main.go:14:1: commentFormatting: put a space between `//` and comment text (gocritic)
//type change string
^
INFO File cache stats: 4 entries of total size 516B
INFO Memory: 10 samples, avg is 71.7MB, max is 72.1MB
INFO Execution took 801.164659ms

[boring-cyborg]

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

[fermin-silva]

were you able to fix this? I believe I'm having the same issue, although our repo/modules setup is a bit more complicated

[voidus]

I've had a quick look and I'm pretty sure golangci-lint doesn't support that at the moment.

When interfacing with pre-commit, it would be best if golangci-lint took a list of filenames like golangci-lint ./path/to/some.go ./some/more.go

I've looked at the code a little am I'm pretty lost, if anybody is familiar with the codebase I'd love to hear if you think that's possible.

If not, then the only way I can see is if we added another switch that maybe runs only on staged changes?
That could be a problem with git commit -a though 🤔
Figuring out which files to check seems non-trivial, and pre-commit already does it.

[miporto]

Just to add a bit of information to this issue, in the pre-commit hook definition filenames are being ignored. You can try to set pass_filenames to true in your .pre-commit-config.yaml file. The problem is that golangci-lint does not support running on multiple individual files at the same time.

I don't know if golangci-lint will be able to support this feature in the future, but I couldn't find an issue related to this problem yet.

[mcchran]

Not sure if make sense for golangci-lint to support individual file scanning. To the best of my understanding, it invokes multiple "linters" and some of them may require to scan the entire project. A good work around is to get all linters someone need (which may be invoked for a particular list of files) and use them directly to the .pre-commit hooks. The cons here is that they should be installed manually.

[codershangfeng]

We are facing the same scenario as this issue described, and it could be quite hard for us to accept and modify all the errors given by golangci-lint at once.

But in order to improve code quality, it is really essential to integrate it. Any thoughts?

[bombsimon]

The issue here is that you have new put under the section run. If you look at the docs, you can see that new should be configured under issues like so:

issues:
  new: true

Remember that this flag has different meaning based on context. Also from the docs:

Show only new issues: if there are unstaged changes or untracked files, only those changes are analyzed, else only changes in HEAD~ are analyzed.
It's a super-useful option for integration of golangci-lint into existing large codebase.
It's not practical to fix all existing issues at the moment of integration: much better to not allow issues in new code.

This also means that (after chaning the .golangci.yaml file in your example) without any changes I'll compare to HEAD~1 but if I have any changes in my staging area the linter will only check those.

From your repository (if I amend the latest commit with the proper config):

$ golangci-lint run --print-issued-lines=false
service/should_be_lint.go:7:1: commentFormatting: put a space between `//` and comment text (gocritic)
main.go:14:1: commentFormatting: put a space between `//` and comment text (gocritic)
$ echo $?
1

$ echo "package main" > main2.go
$ golangci-lint run --print-issued-lines=false
$ echo $?
0

$ git add . && git commit -m"New rev"
$ golangci-lint run --print-issued-lines=false
$ echo $?
0

$ echo "\nvar x = 1" >> main2.go
$ git commit -am"Add failing code"
$ golangci-lint run  --print-issued-lines=false
main2.go:3:5: `x` is unused (deadcode)
$ echo $?
1

You might also be interested in using new-from-rev which does the same thing but from REV and not only HEAD~1.