Goheader linter with only-new-issues false positives

kke commented 2 years ago

Welcome

[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).
[X] Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

Trying to enforce changing the boilerplate copyright year only for changed files in PRs using the github action.

When using the action with only-new-issues: true, the diff very rarely touches the header, mostly only when new files are added, allowing the action to pass without changing the year in the copyright, making the linter ineffective.

If using only-new-issues: false, every file is reported failing the header check even if they weren't touched.

Another problem is build directives (// +build !windows), gofmt moves them to top before the boilerplate, goheader fails for each of these because the header doesn't start from the top, so those files need to be excluded.

Version of golangci-lint

golangci-lint has version v1.43.0 built from (unknown, mod sum: "h1:SLwZFEmDgopqZpfP495zCtV9REUf551JJlJ51Ql7NZA=") on (unknown)

Configuration file

linters-settings:
  goheader:
    template-path: .go-header.txt

Go environment

go version go1.17.5 darwin/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/kimmo/Library/Caches/go-build"
GOENV="/Users/kimmo/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/kimmo/Projects/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/kimmo/Projects/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.17.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.17.5/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.5"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/kimmo/Projects/go/src/github.com/k0sproject/k0s/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 -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/t5/k06n6kss3qz1z5sf0y8pqsw40000gn/T/go-build4190391337=/tmp/go-build -gno-record-gcc-switches -fno-common"

Verbose output of running

...

Code example or link to a public repository

...

boring-cyborg[bot] commented 2 years ago

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

denis-tingaikin commented 2 years ago

@kke Will look at this, thanks for the report.

smyrman commented 1 year ago

I am seeing the same issue in GitHub Actions (false negatives with only-new-issues: true and false positives with only-new-issues: false).

If enabling GitHub Actions to run the tests on push to main (post merge), then the effect would be as if only-new-issues was set to false.

Interestingly, everything works as expected locally, meaning no false positives and no false negatives even for fresh git clones.

Local version:

$ golangci-lint --version
golangci-lint has version 1.51.2 built from 3e8facb on 2023-02-19T20:54:52Z
$ uname -a
Darwin compartes.local 22.3.0 Darwin Kernel Version 22.3.0: Mon Jan 30 20:42:11 PST 2023; root:xnu-8792.81.3~2/RELEASE_X86_64 x86_64

CI version:

  Requested golangci-lint 'v1.51', using 'v1.51.2', calculation took 56ms
  Installing golangci-lint v1.51.2...
  Downloading https://github.com/golangci/golangci-lint/releases/download/v1.51.2/golangci-lint-1.51.2-linux-amd64.tar.gz ...

Example

✅ Failure locally (correct):

% golangci-lint run                                                                                                                                                                                                                                                                                                         :(
WARN [linters_context] rowserrcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649.
query/error.go:1:14: Pattern ((20\d\d\-2023)|(2023)) doesn't match. (goheader)
// Copyright 2022 Searis AS

The linter checks that the year-range math the current year for changed files.

❌ Failures in CI with only-new-issues: true (false negatives):

None; Issue not reported because the change in the file was not located near the header.

❌ Failures in CI with only-new-issues: false, or on merge to master (false positives):

  Error: Pattern ((20\d\d\-2023)|(2023)) doesn't match. (goheader)
  Error: Pattern ((20\d\d\-2023)|(2023)) doesn't match. (goheader)
  Error: Pattern ((20\d\d\-2023)|(2023)) doesn't match. (goheader)

These are false positives; seams the file-change time from Git(?) is ignored.

smyrman commented 1 year ago

Solved the false positive case by adding fetch-depth: 0 to the Checkout step.

    - name: Checkout
        uses: actions/checkout@v3
        with:
          fetch-depth: 0 # Required for go-header check.

Without this, Git will clone only the last commit, and go-header will be unable to check the modification time of the files.

golangci / golangci-lint