Skip to content

ginkgolinter: false positive with Eventually(func(g Gomega){}) #5398

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

Closed
6 of 7 tasks
Luap99 opened this issue Feb 12, 2025 · 9 comments
Closed
6 of 7 tasks

ginkgolinter: false positive with Eventually(func(g Gomega){}) #5398

Luap99 opened this issue Feb 12, 2025 · 9 comments
Assignees
Labels
bug Something isn't working dependencies Relates to an upstream dependency

Comments

@Luap99
Copy link

Luap99 commented Feb 12, 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

After the update to v1.64.2 I See false positives from the ginkgolinter about something like this

Eventually(func(g Gomega) {
	...
}).WithTimeout(time.Second).Should(Succeed())

per ginkgo docs this is valid and looking at ginkgolinter upstream this should be allowed
https://github.com/nunnatsa/ginkgolinter?tab=readme-ov-file#prevent-wrong-actual-values-with-the-succeed-matcher-bug

Running ginkgolinter directly with ginkgolinter ./... does not produce this error so I think this is something with the golangci-lint integration here that is triggering the issue.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.64.2 built with go1.24.0 from 1dae906b on 2025-02-11T21:44:55Z

Configuration

$ golangci-lint run -E ginkgolinter

Go environment

$ go version && go env
go version go1.22.10 linux/amd64
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/pholzing/.cache/go-build'
GOENV='/home/pholzing/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/pholzing/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/pholzing/go'
GOPRIVATE=''
GOPROXY='direct'
GOROOT='/usr/lib/golang'
GOSUMDB='off'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.10'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/pholzing/CODE/test/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 -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1629350348=/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.64.2 built with go1.24.0 from 1dae906b on 2025-02-11T21:44:55Z 
INFO [config_reader] Config search paths: [./ /home/pholzing/CODE/test /home/pholzing/CODE /home/pholzing /home /] 
INFO [goenv] Read go env for 1.797459ms: map[string]string{"GOCACHE":"/home/pholzing/.cache/go-build", "GOROOT":"/usr/lib/golang"} 
INFO [lintersdb] Active 7 linters: [errcheck ginkgolinter gosimple govet ineffassign staticcheck unused] 
INFO [loader] Go packages loading at mode 8767 (types_sizes|deps|imports|name|compiled_files|exports_file|files) took 81.159661ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 157.205µs 
INFO [linters_context/goanalysis] analyzers took 10.89424182s with top 10 stages: buildir: 9.182147313s, inspect: 566.134956ms, printf: 246.549697ms, fact_deprecated: 210.963565ms, ctrlflow: 199.736962ms, fact_purity: 134.388019ms, SA5012: 120.647631ms, nilness: 113.740309ms, typedness: 84.086307ms, tokenfileanalyzer: 30.456012ms 
INFO [runner] Processors filtering stat (in/out): path_shortener: 1/1, nolint_filter: 1/1, filename_unadjuster: 1/1, path_relativity: 1/1, skip_files: 1/1, identifier_marker: 1/1, uniq_by_line: 1/1, max_from_linter: 1/1, source_code: 1/1, skip_dirs: 1/1, path_prettifier: 1/1, sort_results: 1/1, diff: 1/1, cgo: 1/1, exclusion_paths: 1/1, generated_file_filter: 1/1, exclusion_rules: 1/1, max_per_file_from_linter: 1/1, max_same_issues: 1/1, severity-rules: 1/1, path_absoluter: 1/1, invalid_issue: 1/1, fixer: 1/1 
INFO [runner] processing took 180.764µs with stages: nolint_filter: 85.037µs, exclusion_rules: 49.85µs, generated_file_filter: 17.652µs, source_code: 12.949µs, skip_dirs: 6.063µs, uniq_by_line: 1.241µs, path_relativity: 1.122µs, max_same_issues: 807ns, max_from_linter: 688ns, cgo: 687ns, path_shortener: 658ns, sort_results: 570ns, invalid_issue: 463ns, identifier_marker: 447ns, path_absoluter: 417ns, exclusion_paths: 387ns, filename_unadjuster: 378ns, fixer: 345ns, max_per_file_from_linter: 285ns, path_prettifier: 274ns, diff: 201ns, skip_files: 167ns, severity-rules: 76ns 
INFO [runner] linters took 2.527715867s with stages: goanalysis_metalinter: 2.527488805s 
main_test.go:19:3: ginkgo-linter: Success matcher only support a single error value, or function with Gomega as its first parameter (ginkgolinter)
		Eventually(func(g Gomega) {
		^
INFO File cache stats: 1 entries of total size 360B 
INFO Memory: 28 samples, avg is 669.4MB, max is 1159.0MB 
INFO Execution took 2.611434894s 

A minimal reproducible example or link to a public repository

package main_test

import (
	"testing"
	"time"

	. "github.com/onsi/ginkgo/v2"
	. "github.com/onsi/gomega"
)

func TestBooks(t *testing.T) {
	RegisterFailHandler(Fail)
	RunSpecs(t, "t")
}

var _ = Describe("t", func() {

	It("t", func() {
		Eventually(func(g Gomega) {
			g.Expect("abc").To(Equal("abc"))
		}).WithTimeout(time.Second).Should(Succeed())
	})

})

Validation

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

Supporter

@Luap99 Luap99 added the bug Something isn't working label Feb 12, 2025
Copy link

boring-cyborg bot commented Feb 12, 2025

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

@danail-branekov
Copy link

danail-branekov commented Feb 12, 2025

We are seeing the same failure with our codebase: https://github.com/cloudfoundry/korifi/tree/55b4d9219d221491330a9292ed6f31cc152bb89f

We have seen no issues with the same code with golangci-lint 1.63.4. As @Luap99 observed, just running the ginkgo linter succeeds.

Luap99 added a commit to Luap99/libpod that referenced this issue Feb 12, 2025
This is valid and the upstream linter allows it but somehow with
golangci-lint it produces an error:

Success matcher only support a single error value, or function with Gomega as its first parameter

I reported a bug upstream[1] but for now let's just ignore it so we can
update the linter.

[1] golangci/golangci-lint#5398

Signed-off-by: Paul Holzinger <[email protected]>
@ldez ldez self-assigned this Feb 12, 2025
danail-branekov added a commit to cloudfoundry/korifi that referenced this issue Feb 12, 2025
golangci-lint 1.64.2 reports false positives with the ginkgolinter, see golangci/golangci-lint#5398

Co-authored-by: Georgi Sabev <[email protected]>
@ldez
Copy link
Member

ldez commented Feb 12, 2025

I'm investigating but I observed that modifying the function signature seems to "fix" the problem

package main_test

import (
	"testing"
	"time"

	. "github.com/onsi/ginkgo/v2"
	. "github.com/onsi/gomega"
)

func TestBooks(t *testing.T) {
	RegisterFailHandler(Fail)
	RunSpecs(t, "t")
}

var _ = Describe("t", func() {

	It("t", func() {
		Eventually(func(g Gomega) error {
			g.Expect("abc").To(Equal("abc"))
			return nil
		}).WithTimeout(time.Second).Should(Succeed())
	})

})

georgethebeatle added a commit to cloudfoundry/korifi that referenced this issue Feb 12, 2025
golangci-lint 1.64.2 reports false positives with the ginkgolinter, see golangci/golangci-lint#5398

Co-authored-by: Georgi Sabev <[email protected]>
@ldez
Copy link
Member

ldez commented Feb 12, 2025

This is ginkgolinter problem: if I modify the min go version of ginkgolinter to go1.23.0 I have the same behavior:

$ ginkgolinter ./...                  
/home/ldez/sources/golangci/sandbox/foo_test.go:19:3: ginkgo-linter: Success matcher only support a single error value, or function with Gomega as its first parameter

ping @nunnatsa

@ldez ldez added the dependencies Relates to an upstream dependency label Feb 12, 2025
@nunnatsa
Copy link
Contributor

Thanks you all! I noticed the same issue. It doesn't happen when running ginkgolinter cli directly, or with the previous version of golangci-lint, even though the ginkgolinter version was not changed.

@ldez - not sure what you mean by modify the min go version of ginkgolinter to go1.23.0

@nunnatsa
Copy link
Contributor

OK, I locally bumped golang in ginkgolinter to 1.23 - and I can see the error in my tests. looking

@nunnatsa
Copy link
Contributor

Just released ginkgolinter v0.19.0 with a fix to this issue: https://github.com/nunnatsa/ginkgolinter/releases/tag/v0.19.0

@ldez
Copy link
Member

ldez commented Feb 12, 2025

Fixed by #5404

But I think the fix can have side effects, so I opened a complementary PR nunnatsa/ginkgolinter#187.

@ldez ldez closed this as completed Feb 12, 2025
@Luap99
Copy link
Author

Luap99 commented Feb 12, 2025

Thanks @ldez and @nunnatsa

danail-branekov added a commit to cloudfoundry/korifi that referenced this issue Feb 13, 2025
The [issue](golangci/golangci-lint#5398) with false positives from the ginkgolinter has been resolved

This reverts commit cd5f925.
danail-branekov added a commit to cloudfoundry/korifi that referenced this issue Feb 13, 2025
The [issue](golangci/golangci-lint#5398) with false positives from the ginkgolinter has been resolved

This reverts commit cd5f925.

fixes #3794
danail-branekov added a commit to cloudfoundry/korifi that referenced this issue Feb 13, 2025
The [issue](golangci/golangci-lint#5398) with false positives from the ginkgolinter has been resolved

This reverts commit cd5f925.

fixes #3794
Luap99 added a commit to Luap99/libpod that referenced this issue Feb 13, 2025
Luap99 added a commit to Luap99/libpod that referenced this issue Feb 20, 2025
This is valid and the upstream linter allows it but somehow with
golangci-lint it produces an error:

Success matcher only support a single error value, or function with Gomega as its first parameter

I reported a bug upstream[1] but for now let's just ignore it so we can
update the linter.

[1] golangci/golangci-lint#5398

Signed-off-by: Paul Holzinger <[email protected]>
(cherry picked from commit 8b6f14f)
Signed-off-by: Paul Holzinger <[email protected]>
Luap99 added a commit to Luap99/libpod that referenced this issue Feb 20, 2025
This reverts commit 8b6f14f.

golangci/golangci-lint#5398 is fixed

Signed-off-by: Paul Holzinger <[email protected]>
(cherry picked from commit 7773713)
Signed-off-by: Paul Holzinger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Relates to an upstream dependency
Projects
None yet
Development

No branches or pull requests

4 participants