Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

goheader false positive (caused by golangci-lint, not goheader itself) #5284

Closed
6 of 7 tasks
skitt opened this issue Jan 2, 2025 · 4 comments · Fixed by #5286
Closed
6 of 7 tasks

goheader false positive (caused by golangci-lint, not goheader itself) #5284

skitt opened this issue Jan 2, 2025 · 4 comments · Fixed by #5286
Assignees
Labels
bug Something isn't working

Comments

@skitt
Copy link

skitt commented Jan 2, 2025

Welcome

  • Yes, I'm using a binary release within 2 latest 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.
  • Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.).
  • I agree to follow this project's Code of Conduct

Description of the problem

v1.63.2 reports this on https://github.com/submariner-io/subctl/blob/devel/pkg/join/options.go even though the file has a correct header:

pkg/join/options.go:54:3: Missed header for check (goheader)
}

This worked fine in v1.62.2, and works find with go-header invoked directly.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.63.2 built with go1.23.4 from 15412b30 on 2025-01-02T12:43:20Z

Configuration

---
linters-settings:
  depguard:
    rules:
      main:
        deny:
          - pkg: github.com/mcuadros/go-version
            desc: Use coreos/go-semver instead
  gocritic:
    enabled-tags:
      - diagnostic
      - opinionated
      - performance
      - style
    disabled-checks:
      - ifElseChain
      - unnamedResult
  gocyclo:
    min-complexity: 15
  godot:
    exclude:
      - ^\s*\+
  goheader:
    template: |-
      SPDX-License-Identifier: Apache-2.0

      Copyright Contributors to the Submariner project.

      Licensed under the Apache License, Version 2.0 (the "License");
      you may not use this file except in compliance with the License.
      You may obtain a copy of the License at

          http://www.apache.org/licenses/LICENSE-2.0

      Unless required by applicable law or agreed to in writing, software
      distributed under the License is distributed on an "AS IS" BASIS,
      WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
      See the License for the specific language governing permissions and
      limitations under the License.
  govet:
    enable:
      - fieldalignment
  lll:
    line-length: 140
  wrapcheck:
    ignoreSigs:
      - .Error(
      - .Errorf(
      - errors.New(
      - errors.Unwrap(
      - .Wrap(
      - .Wrapf(
      - .WithMessage(
      - .WithMessagef(
      - .WithStack(
  wsl:
    # Separating explicit var declarations by blank lines seems excessive.
    allow-cuddle-declarations: true
linters:
  disable-all: true
  enable:
    - asciicheck
    - bidichk
    - bodyclose
    - contextcheck
    - copyloopvar
    # - cyclop # This is equivalent to gocyclo
    - depguard
    - dogsled
    - dupl
    - durationcheck
    - errcheck
    - errorlint
    - errname
    - exhaustive
    # - exhaustivestruct # Not recommended for general use - meant to be used only for special cases
    # - forbidigo # We don't forbid any statements
    # - forcetypeassert # There are many unchecked type assertions that would be the result of a programming error so the
    #                     reasonable recourse would be to panic anyway if checked so this doesn't seem useful
    # - funlen # gocyclo is enabled which is generally a better metric than simply LOC.
    - gci
    - ginkgolinter
    # - gochecknoglobals # We don't want to forbid global variable constants
    # - gochecknoinits # We use init functions for valid reasons
    - gocognit
    - goconst
    - gocritic
    - gocyclo
    - godot
    # - godox #  Let's not forbid inline TODOs, FIXMEs et al
    - err113
    - gofmt
    - gofumpt
    - goheader
    - goimports
    # - golint # Deprecated since v1.41.0
    # - gomnd # It doesn't seem useful in general to enforce constants for all numeric values
    # - gomoddirectives # We don't want to forbid the 'replace' directive
    # - gomodguard # We don't block any modules
    # - goprintffuncname # This doesn't seem useful at all
    - gosec
    - gosimple
    - gosmopolitan
    - govet
    # - ifshort # This is a style preference and doesn't seem compelling
    - importas
    - ineffassign
    # - ireturn # The argument to always "Return Concrete Types" doesn't seem compelling. It is perfectly valid to return
    #             an interface to avoid exposing the entire underlying struct
    # - interfacer # Deprecated since v1.38.0
    - lll
    - makezero
    # - maligned # Deprecated since v1.38.0
    - mirror
    - misspell
    - nakedret
    # - nestif # This calculates cognitive complexity but we're doing that elsewhere
    - nilerr
    - nilnil
    # - nlreturn # This is reasonable with a block-size of 2 but setting it above isn't honored
    # - noctx # We don't send HTTP requests
    - nolintlint
    # - paralleltest # Not relevant for Ginkgo UTs
    - prealloc
    - predeclared
    - promlinter
    - revive
    # - rowserrcheck # We don't use SQL
    # - scopelint # Deprecated since v1.39.0
    # - sqlclosecheck # We don't use SQL
    - staticcheck
    - stylecheck
    - tagalign
    # - tagliatelle # Inconsistent with stylecheck and not as good
    # - tenv # Not relevant for our Ginkgo UTs
    - testpackage
    # - thelper # Not relevant for our Ginkgo UTs
    # - tparallel # Not relevant for our Ginkgo UTs
    - typecheck
    - unconvert
    - unparam
    - unused
    # - varnamelen # It doesn't seem necessary to enforce a minimum variable name length
    - wastedassign
    - whitespace
    - wrapcheck
    - wsl
    - zerologlint
issues:
  exclude-rules:
    # Allow dot-imports for Gomega BDD directives per idiomatic Gomega
    - linters:
        - revive
        - stylecheck
      text: "dot imports"
      source: "gomega"

    # Allow dot-imports for Ginkgo BDD directives per idiomatic Ginkgo
    - linters:
        - revive
        - stylecheck
      text: "dot imports"
      source: "ginkgo"

    # Ignore long line and variable name non-compliance warnings in auto-generated file
    - linters:
        - lll
        - stylecheck
        - revive
      path: "pkg/embeddedyamls/yamls.go"

    # BrokerK8sApiServer parameter is used by other projects, like ACM,
    # so not changing it to BrokerK8sAPIServer as suggested by stylecheck
    - linters:
        - revive
        - stylecheck
      text: "struct field BrokerK8sApiServer"

    # Ignore pointer bytes in struct alignment tests (this is a very
    # minor optimisation)
    - linters:
        - govet
      text: "pointer bytes could be"

    # Full text of the error is "do not define dynamic errors, use wrapped static errors instead". See
    # https://github.com/Djarvur/go-err113/issues/10 for an interesting discussion of this error. While there are cases
    # where wrapped sentinel errors are useful, it seems a bit pedantic to force that pattern in all cases.
    - linters:
        - err113
      text: "do not define dynamic errors"

    # Ignore certain linters for test files
    - path: _test\.go|test/|fake/
      linters:
        - errname
        - gochecknoinits
        - err113
        - wrapcheck

    # Ignore header linting for internal files copied from Kubernetes
    - path: internal/(cli|env|log)/.*\.go
      linters:
        - goheader

    # Ignore header linting for build constraint
    # See https://github.com/denis-tingaikin/go-header/issues/18
    - path: cmd/subctl/.*\.go
      linters:
        - goheader

Go environment

$ go version && go env
go version go1.23.1 linux/amd64
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/skitt/.cache/go-build'
GOENV='/home/skitt/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/skitt/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/skitt/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/skitt/sdk/go1.23.1'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/skitt/sdk/go1.23.1/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.1'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/skitt/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/skitt/go/src/github.com/submariner-io/subctl/go.mod'
GOWORK='/home/skitt/go/src/github.com/submariner-io/go.work'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4163584049=/tmp/go-build -gno-record-gcc-switches'

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO golangci-lint has version 1.63.2 built with go1.23.4 from 15412b30 on 2025-01-02T12:43:20Z 
INFO [config_reader] Config search paths: [./ /home/skitt/go/src/github.com/submariner-io/subctl /home/skitt/go/src/github.com/submariner-io /home/skitt/go/src/github.com /home/skitt/go/src /home/skitt/go /home/skitt /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 55 linters: [asciicheck bidichk bodyclose contextcheck copyloopvar depguard dogsled dupl durationcheck err113 errcheck errname errorlint exhaustive gci ginkgolinter gocognit goconst gocritic gocyclo godot gofmt gofumpt goheader goimports gosec gosimple gosmopolitan govet importas ineffassign lll makezero mirror misspell nakedret nilerr nilnil nolintlint prealloc predeclared promlinter revive staticcheck stylecheck tagalign testpackage unconvert unparam unused wastedassign whitespace wrapcheck wsl zerologlint] 
INFO [loader] Go packages loading at mode 8767 (deps|exports_file|imports|compiled_files|files|name|types_sizes) took 384.791345ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 5.144859ms 
INFO [linters_context] importas settings found, but no aliases listed. List aliases under alias: key. 
INFO [linters_context/goanalysis] analyzers took 8m47.717880922s with top 10 stages: buildir: 3m37.324911839s, gocritic: 2m34.22654751s, buildssa: 32.516904055s, wastedassign: 11.229288237s, unparam: 11.007917064s, the_only_name: 8.766081663s, inspect: 7.722154244s, exhaustive: 6.857988273s, goimports: 6.403315168s, fact_deprecated: 5.946952341s 
INFO [runner] Issues before processing: 657, after processing: 1 
INFO [runner] Processors filtering stat (in/out): invalid_issue: 657/657, skip_files: 657/657, autogenerated_exclude: 657/657, uniq_by_line: 1/1, max_per_file_from_linter: 1/1, sort_results: 1/1, filename_unadjuster: 657/657, nolint: 199/1, source_code: 1/1, fixer: 1/1, path_prettifier: 657/657, max_same_issues: 1/1, severity-rules: 1/1, path_prefixer: 1/1, cgo: 657/657, skip_dirs: 657/657, identifier_marker: 657/657, exclude: 657/657, exclude-rules: 657/199, diff: 1/1, max_from_linter: 1/1, path_shortener: 1/1 
INFO [runner] processing took 68.968876ms with stages: exclude-rules: 32.877173ms, nolint: 15.55953ms, identifier_marker: 14.17857ms, path_prettifier: 2.856007ms, autogenerated_exclude: 2.832131ms, skip_dirs: 471.532µs, invalid_issue: 66.395µs, cgo: 64.53µs, filename_unadjuster: 36.091µs, source_code: 19.508µs, max_same_issues: 2.069µs, uniq_by_line: 1.516µs, path_shortener: 1.312µs, max_from_linter: 692ns, max_per_file_from_linter: 387ns, skip_files: 362ns, sort_results: 252ns, fixer: 233ns, exclude: 212ns, diff: 186ns, path_prefixer: 100ns, severity-rules: 88ns 
INFO [runner] linters took 41.12116656s with stages: goanalysis_metalinter: 41.052082246s 
pkg/join/options.go:54:3: Missed header for check (goheader)
}
 ^
INFO File cache stats: 158 entries of total size 589.1KiB 
INFO Memory: 345 samples, avg is 3927.8MB, max is 6374.5MB 
INFO Execution took 41.521437447s

A minimal reproducible example or link to a public repository

See https://github.com/submariner-io/subctl/blob/devel/pkg/join/options.go

Validation

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

Supporter

@skitt skitt added the bug Something isn't working label Jan 2, 2025
Copy link

boring-cyborg bot commented Jan 2, 2025

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

@skitt skitt changed the title goheader false negative (caused by golangci-lint, not goheader itself) goheader false positive (caused by golangci-lint, not goheader itself) Jan 2, 2025
@ldez ldez self-assigned this Jan 2, 2025
@ldez
Copy link
Member

ldez commented Jan 2, 2025

Fixed by #5286

@ldez ldez closed this as completed Jan 2, 2025
@ccoVeille

This comment was marked as outdated.

@skitt
Copy link
Author

skitt commented Jan 3, 2025

Thanks for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants