--new-from-rev main doesn't work

[facchettos]

Welcome

• Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
• Yes, I've searched similar issues on GitHub and didn't find any.
• Yes, I've read the typecheck section of the FAQ (https://golangci-lint.run/usage/faq/#why-do-you-have-typecheck-errors).
• Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.). (https://golangci-lint.run/usage/linters/)

Description of the problem

after trying to enable only-new-errors and also --new-from-rev (separately) on the GitHub action in our repo, golangci-lint does find errors even though there should be one. version in the action is 1.56, doesn't work either on my machine with version
golangci-lint has version 1.56.1 built with go1.22.0 from a25592b5 on 2024-02-08T18:03:33Z

You can find the run here https://github.com/loft-sh/vcluster/actions/runs/7854299455/job/21434870993?pr=1534

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.56.1 built with go1.22.0 from a25592b5 on 2024-02-08T18:03:33Z

Configuration

run:
  timeout: 10m

linters:
  disable-all: true
  enable:
    - asasalint
    - asciicheck
    - bidichk
    - decorder
    - dupl
    - durationcheck
    - errcheck
    - errname
    - errorlint
    - exhaustive
    - exportloopref
    - ginkgolinter
    - gocheckcompilerdirectives
    - gofmt
    - goimports
    - gosimple
    - govet
    - grouper
    - importas
    - ineffassign
    - makezero
    - misspell
    - nakedret
    - promlinter
    - revive
    - staticcheck
    - stylecheck
    - tagalign
    - typecheck
    - unconvert
    - unused
    - usestdlibvars
    - whitespace

    # next linters to be enabled:
    # - prealloc
    # - nilnil
    # - errchkjson
    # - gocritic

    # linters to be enabled in the distant future:
    # - cyclop
    # - dupl
    # - funlen
    # - interfacebloat
    # - predeclared
    # - stylecheck
    # - wrapcheck

linters-settings:
  gofmt:
    simplify: true
  dupl:
    threshold: 400

  exhaustive:
    check:
      - switch
      - map
    ignore-enum-types: "ResourceName|Atom"
    default-signifies-exhaustive: true

  importas:
    no-unaliased: true
    alias:
      # Kubernetes
      - pkg: k8s\.io/api/(\w+)/(v[\w\d]+)
        alias: $1$2
      - pkg: k8s\.io/apimachinery/pkg/api/(\w+)/(v[\w\d]+)
        alias: $1$2
      - pkg: k8s.io/apimachinery/pkg/api/errors
        alias: kerrors
      - pkg: k8s.io/apimachinery/pkg/apis/meta/internalversion
        alias: metainternalversion

  tagalign:
    order:
      - json
      - yaml
      - xml
      - form

issues:
  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
  max-issues-per-linter: 0
  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
  max-same-issues: 0

name: Lint

on:
  release:
    types: [created]
  pull_request:
    branches:
      - main
    paths:
      - "**.go"

concurrency:
  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
  cancel-in-progress: true

jobs:
  golangci:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - uses: actions/setup-go@v5
        with:
          go-version: "1.21"
          cache: false
      - name: Generate Embedded Helm Charts
        run: |
          go generate ./...
      - name: Run golangci-lint
        uses: golangci/golangci-lint-action@v3
        with:
          args: -v --config=.golangci.yml
          only-new-issues: true

Go environment

$ go version && go env
go version go1.22.0 darwin/arm64
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/jeremyfacchetti/Library/Caches/go-build'
GOENV='/Users/jeremyfacchetti/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/jeremyfacchetti/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/jeremyfacchetti/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/jeremyfacchetti/go/src/vcluster/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/k1/ckrxs_qd66s8kn1ldprb4qqc0000gn/T/go-build1931624603=/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/jeremyfacchetti/go/src/vcluster /Users/jeremyfacchetti/go/src /Users/jeremyfacchetti/go /Users/jeremyfacchetti /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 32 linters: [asasalint asciicheck bidichk decorder dupl durationcheck errcheck errname errorlint exhaustive exportloopref ginkgolinter gocheckcompilerdirectives gofmt goimports gosimple govet grouper importas ineffassign makezero misspell nakedret promlinter revive staticcheck stylecheck tagalign unconvert unused usestdlibvars whitespace]
INFO [loader] Go packages loading at mode 575 (exports_file|imports|types_sizes|compiled_files|deps|files|name) took 839.944167ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 11.494375ms
INFO [linters_context/goanalysis] analyzers took 3m55.626973773s with top 10 stages: buildir: 2m10.410232697s, the_only_name: 12.01607704s, dupl: 7.578001212s, unconvert: 6.209480824s, exhaustive: 5.758219159s, nilness: 5.006622591s, fact_deprecated: 4.517471187s, printf: 3.622372413s, ctrlflow: 3.516414075s, inspect: 3.504887153s
INFO [runner] Issues before processing: 1195, after processing: 0
INFO [runner] Processors filtering stat (out/in): filename_unadjuster: 1195/1195, exclude: 1155/1155, nolint: 30/35, autogenerated_exclude: 1155/1195, exclude-rules: 35/1155, uniq_by_line: 23/30, skip_files: 1195/1195, diff: 0/23, cgo: 1195/1195, path_prettifier: 1195/1195, skip_dirs: 1195/1195, identifier_marker: 1155/1155
INFO [runner] processing took 274.340752ms with stages: diff: 218.288833ms, autogenerated_exclude: 17.777583ms, exclude-rules: 17.589751ms, identifier_marker: 11.033542ms, path_prettifier: 6.261875ms, nolint: 2.223875ms, skip_dirs: 1.0175ms, cgo: 93.125µs, filename_unadjuster: 42.125µs, uniq_by_line: 8.417µs, max_same_issues: 1.709µs, skip_files: 958ns, severity-rules: 291ns, max_per_file_from_linter: 251ns, sort_results: 250ns, fixer: 250ns, exclude: 208ns, path_prefixer: 84ns, max_from_linter: 84ns, source_code: 41ns, path_shortener: 0s
INFO [runner] linters took 24.550464041s with stages: goanalysis_metalinter: 24.275932125s
INFO File cache stats: 372 entries of total size 1.8MiB
INFO Memory: 248 samples, avg is 2528.8MB, max is 4410.0MB
INFO Execution took 25.409005s

A minimal reproducible example or link to a public repository

https://github.com/loft-sh/vcluster/actions/runs/7854299455/job/21434870993?pr=1534

➜  vcluster git:(lint-fix) golangci-lint run
pkg/leaderelection/leaderelection.go:72:27: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
			OnStartedLeading: func(ctx context.Context) {
			                       ^
			                     ^
pkg/setup/initialize.go:302:1: unnecessary trailing newline (whitespace)

^
➜  vcluster git:(lint-fix) golangci-lint run  --new-from-rev main
➜  vcluster git:(lint-fix)

loft-sh/vcluster#1534 should fail linting, it does without --new-from-rev but not with it, which is not expected

Validation

• Yes, I've included all information above (version, config, etc.).

[ldez]

related to #4375

[ldez]

$ git clone git@github.com:facchettos/vcluster.git
Cloning into 'vcluster'...
remote: Enumerating objects: 49966, done.
remote: Counting objects: 100% (4122/4122), done.
remote: Compressing objects: 100% (2249/2249), done.
remote: Total 49966 (delta 1736), reused 3844 (delta 1627), pack-reused 45844
Receiving objects: 100% (49966/49966), 61.95 MiB | 7.20 MiB/s, done.
Resolving deltas: 100% (27395/27395), done.

$ cd vcluster 

$ git switch lint-fix 
branch 'lint-fix' set up to track 'origin/lint-fix'.
Switched to a new branch 'lint-fix'

$ git lgr -4
* ca280767 E - (HEAD -> lint-fix, origin/lint-fix) Update initialize.go (Jeremy Facchetti 63 minutes ago)
* 7aa32cf6 E - Update initialize.go (Jeremy Facchetti 64 minutes ago)
* ce40435a N - trying a temporary fix for the CI failurs (facchettos 3 days ago)
| * ca2c8bff N - (origin/json-schema-automation) some fixes and a new action (facchettos 2 hours ago)

$ golangci-lint version                               
golangci-lint has version 1.56.1 built with go1.22.0 from a25592b5 on 2024-02-08T18:03:33Z

$ golangci-lint cache clean

$ golangci-lint run --new-from-rev main
pkg/setup/initialize.go:302:1: unnecessary trailing newline (whitespace)

^

--new-from-rev works as expected.

[facchettos]

it works on your machine, but neither on mine or when running the GitHub action

loft-sh/vcluster#1534 (last run https://github.com/loft-sh/vcluster/actions/runs/7872252998/job/21477080256?pr=1534)

➜  vcluster git:(lint-fix) golangci-lint cache clean
➜  vcluster git:(lint-fix) golangci-lint run  --new-from-rev main  -v
INFO [config_reader] Config search paths: [./ /Users/jeremyfacchetti/go/src/vcluster /Users/jeremyfacchetti/go/src /Users/jeremyfacchetti/go /Users/jeremyfacchetti /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 32 linters: [asasalint asciicheck bidichk decorder dupl durationcheck errcheck errname errorlint exhaustive exportloopref ginkgolinter gocheckcompilerdirectives gofmt goimports gosimple govet grouper importas ineffassign makezero misspell nakedret promlinter revive staticcheck stylecheck tagalign unconvert unused usestdlibvars whitespace]
INFO [loader] Go packages loading at mode 575 (deps|files|name|types_sizes|compiled_files|exports_file|imports) took 1.140434792s
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 11.743833ms
INFO [linters_context/goanalysis] analyzers took 4m29.965121631s with top 10 stages: buildir: 2m22.389086351s, the_only_name: 13.685301873s, dupl: 10.287301466s, unconvert: 6.970400327s, nilness: 6.099583101s, exhaustive: 5.316888935s, bidichk: 4.952421909s, fact_deprecated: 4.788200681s, inspect: 3.679315855s, printf: 3.463779371s
INFO [runner] Issues before processing: 1195, after processing: 0
INFO [runner] Processors filtering stat (out/in): skip_dirs: 1195/1195, diff: 0/23, cgo: 1195/1195, filename_unadjuster: 1195/1195, identifier_marker: 1155/1155, skip_files: 1195/1195, autogenerated_exclude: 1155/1195, exclude-rules: 35/1155, nolint: 30/35, path_prettifier: 1195/1195, exclude: 1155/1155, uniq_by_line: 23/30
INFO [runner] processing took 308.506168ms with stages: diff: 243.669333ms, autogenerated_exclude: 23.412791ms, exclude-rules: 17.771457ms, identifier_marker: 12.922209ms, path_prettifier: 7.390166ms, nolint: 2.132667ms, skip_dirs: 1.04475ms, cgo: 97.125µs, filename_unadjuster: 58.833µs, uniq_by_line: 4.126µs, max_same_issues: 833ns, exclude: 292ns, max_per_file_from_linter: 292ns, severity-rules: 291ns, fixer: 250ns, sort_results: 209ns, skip_files: 209ns, max_from_linter: 84ns, path_prefixer: 84ns, source_code: 84ns, path_shortener: 83ns
INFO [runner] linters took 26.798472917s with stages: goanalysis_metalinter: 26.489810916s
INFO File cache stats: 372 entries of total size 1.8MiB
INFO Memory: 270 samples, avg is 2603.9MB, max is 4587.2MB
INFO Execution took 27.958387541s
➜  vcluster git:(lint-fix)

it's skipping all the directories from what I understand in the logs

[ldez]

it works on your machine, but neither on mine or when running the GitHub action

The option of the GitHub action only-new-issues uses the GitHub diff (new-from-patch) and not --new-from-rev.

Can you test locally with a fresh clone?

[facchettos]

The option of the GitHub action only-new-errors uses the GitHub diff and not --new-from-rev.

I tried both actually and none work

[facchettos]

for the run with only-new-issues: true
https://github.com/loft-sh/vcluster/actions/runs/7872471889/job/21477732097?pr=1534

and for the fresh clone

➜  /tmp git clone git@github.com:facchettos/vcluster
Cloning into 'vcluster'...
remote: Enumerating objects: 49975, done.
remote: Counting objects: 100% (4131/4131), done.
remote: Compressing objects: 100% (2258/2258), done.
remote: Total 49975 (delta 1739), reused 3849 (delta 1627), pack-reused 45844
Receiving objects: 100% (49975/49975), 61.94 MiB | 12.54 MiB/s, done.
Resolving deltas: 100% (27398/27398), done.
➜  /tmp cd vcluster
➜  vcluster git:(main) git switch lint-fix
branch 'lint-fix' set up to track 'origin/lint-fix'.
Switched to a new branch 'lint-fix'
➜  vcluster git:(lint-fix) golangci-lint cache clean
➜  vcluster git:(lint-fix) golangci-lint run  --new-from-rev main  -v
INFO [config_reader] Config search paths: [./ /tmp/vcluster /tmp / /Users/jeremyfacchetti]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 32 linters: [asasalint asciicheck bidichk decorder dupl durationcheck errcheck errname errorlint exhaustive exportloopref ginkgolinter gocheckcompilerdirectives gofmt goimports gosimple govet grouper importas ineffassign makezero misspell nakedret promlinter revive staticcheck stylecheck tagalign unconvert unused usestdlibvars whitespace]
INFO [loader] Go packages loading at mode 575 (types_sizes|compiled_files|deps|files|exports_file|imports|name) took 1.073009583s
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 12.034292ms
INFO [linters_context/goanalysis] analyzers took 3m54.256930058s with top 10 stages: buildir: 2m5.63464844s, the_only_name: 13.09086058s, dupl: 8.67383842s, unconvert: 6.602964749s, nilness: 5.531049767s, exhaustive: 5.388250987s, fact_deprecated: 4.333431462s, bidichk: 3.334910628s, ctrlflow: 3.244165584s, printf: 3.099849548s
INFO [runner] Issues before processing: 1195, after processing: 0
INFO [runner] Processors filtering stat (out/in): diff: 0/23, skip_files: 1195/1195, uniq_by_line: 23/30, filename_unadjuster: 1195/1195, skip_dirs: 1195/1195, exclude-rules: 35/1155, nolint: 30/35, cgo: 1195/1195, autogenerated_exclude: 1155/1195, exclude: 1155/1155, path_prettifier: 1195/1195, identifier_marker: 1155/1155
INFO [runner] processing took 227.690542ms with stages: diff: 171.337166ms, exclude-rules: 21.559084ms, autogenerated_exclude: 16.309792ms, identifier_marker: 10.520834ms, path_prettifier: 4.325542ms, nolint: 2.497083ms, skip_dirs: 1.043333ms, cgo: 58.999µs, filename_unadjuster: 27.291µs, uniq_by_line: 7.459µs, max_same_issues: 1.083µs, path_shortener: 708ns, severity-rules: 667ns, fixer: 334ns, sort_results: 249ns, exclude: 209ns, source_code: 208ns, skip_files: 167ns, max_per_file_from_linter: 167ns, max_from_linter: 84ns, path_prefixer: 83ns
INFO [runner] linters took 24.021379917s with stages: goanalysis_metalinter: 23.793506083s
INFO File cache stats: 372 entries of total size 1.8MiB
INFO Memory: 250 samples, avg is 2433.4MB, max is 4247.9MB
INFO Execution took 25.113441834s
➜  vcluster git:(lint-fix)

[ldez]

First, I was able to reproduce it.

whitespace reports an issue at the "wrong" line: The change is on 301 and the report on 302, but 302 has not changed so golangci-lint skips the report.

It's not really the "wrong" line because the report is related to an empty line before the }, this empty line is only a problem because }.

We can see that as a more global limitation of the new-from-patch and new-from-rev: if a report is not directly on the line that changes then the report is skipped.

So technically, there is no possible fix for that.
The only way for those options to work, with reports that are not at the same places as the changes, is to use --whole-files

golangci-lint run --whole-files --new-from-rev main

[facchettos]

awesome, I guess we can use --whole-files ! merci chef :)