golangci / golangci-lint

Fast linters runner for Go
https://golangci-lint.run
GNU General Public License v3.0
15.35k stars 1.37k forks source link

Incorrect reporting for uncheckedInlineErr (gocritic) #3628

Open ennc0d3 opened 1 year ago

ennc0d3 commented 1 year ago

Welcome

Description of the problem

When running golangci-lint with gocritic with uncheckedInlineErr for a simple go program with cgo enabled, golangci-lint is producing incorrect reporting,

Example,

main.go:21:8: uncheckedInlineErr:  er error is unchecked, maybe intended to check it instead of  // (gocritic)
    if err = doIt(); err2 != nil { // expect to complain
       ^
main.go:25:8: uncheckedInlineErr:  val error is unchecked, maybe intended to check it instead of trin (gocritic)
    if err2 = doIt(); err2 != nil { // expect  no issue
       ^
main.go:32:8: uncheckedInlineErr: err2 error is unchecked, maybe intended to check it instead of err (gocritic)
    if err2 := doIt(); err != nil { // expect to complain

The err, err2 variables are declared earlier and I have marked the statements where I expect gocritic to complain, but looking report we can see golangci-lint/gocritic seems to be confused.

When I tried the same with the gocritic standalone binary it doesn't report anything at all which is also weird.

go install -v github.com/go-critic/go-critic/cmd/gocritic@v0.6.7
cd uncheckedInlineErr
gocritic check -v  --enable="uncheckedInlineErr" ./...

Maybe this is not the right way to invoke?

But I think anyway golangci-lint report is not working as expected or could it be because of some weird issues in my setup?

Thanks,

Version of golangci-lint

```console $ golangci-lint --version golangci-lint has version 1.51.2 built from 3e8facb4 on 2023-02-19T21:43:54Z ```

Configuration file

```console $ cat .golangci.yml linters: disable-all: true enable: - gocritic linters-settings: gocritic: enabled-checks: - uncheckedInlineErr ```

Go environment

```console $ go version && go env go version go1.20.1 linux/amd64 GO111MODULE="" GOARCH="amd64" GOBIN="/home/enaggan/go/bin" GOCACHE="/home/enaggan/.cache/go-build" GOENV="/home/enaggan/.config/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="linux" GOINSECURE="" GOMODCACHE="/home/enaggan/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="linux" GOPATH="/home/enaggan/go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local/go" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64" GOVCS="" GOVERSION="go1.20.1" GCCGO="gccgo" GOAMD64="v1" AR="ar" CC="gcc" CXX="g++" CGO_ENABLED="1" GOMOD="/home/enaggan/uncheckedInlineErr/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 -fdebug-prefix-map=/tmp/go-build3755390846=/tmp/go-build -gno-record-gcc-switches" ```

Verbose output of running

```console $ golangci-lint cache clean INFO [config_reader] Config search paths: [./ /home/enn/uncheckedInlineErr /home/enn /home /] INFO [config_reader] Used config file .golangci.yml $ golangci-lint run -v IINFO [config_reader] Config search paths: [./ /home/enn/uncheckedInlineErr /home/enn /home /] INFO [config_reader] Used config file .golangci.yml INFO [lintersdb] Active 1 linters: [gocritic] INFO [loader] Go packages loading at mode 575 (compiled_files|exports_file|files|imports|name|deps|types_sizes) took 45.368767ms INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 213.543µs INFO [linters_context/goanalysis] analyzers took 673.896µs with top 10 stages: gocritic: 673.896µs INFO [runner] Processors filtering stat (out/in): skip_files: 3/3, exclude: 3/3, max_from_linter: 3/3, path_shortener: 3/3, sort_results: 3/3, autogenerated_exclude: 3/3, uniq_by_line: 3/3, diff: 3/3, max_per_file_from_linter: 3/3, max_same_issues: 3/3, severity-rules: 3/3, path_prefixer: 3/3, filename_unadjuster: 3/3, path_prettifier: 3/3, skip_dirs: 3/3, exclude-rules: 3/3, nolint: 3/3, source_code: 3/3, cgo: 3/3, identifier_marker: 3/3 INFO [runner] processing took 327.237µs with stages: exclude-rules: 108.287µs, nolint: 86.012µs, identifier_marker: 71.285µs, autogenerated_exclude: 27.977µs, source_code: 10.968µs, path_prettifier: 9.537µs, skip_dirs: 3.962µs, max_same_issues: 2.211µs, uniq_by_line: 1.894µs, cgo: 1.098µs, max_from_linter: 1.04µs, skip_files: 691ns, path_shortener: 638ns, filename_unadjuster: 523ns, max_per_file_from_linter: 277ns, exclude: 234ns, diff: 214ns, severity-rules: 202ns, sort_results: 116ns, path_prefixer: 71ns INFO [runner] linters took 247.49308ms with stages: gocritic: 247.116063ms main.go:21:8: uncheckedInlineErr: er error is unchecked, maybe intended to check it instead of // (gocritic) if err = doIt(); err2 != nil { // expect to complain ^ main.go:25:8: uncheckedInlineErr: val error is unchecked, maybe intended to check it instead of trin (gocritic) if err2 = doIt(); err2 != nil { // expect no issue ^ main.go:32:8: uncheckedInlineErr: err2 error is unchecked, maybe intended to check it instead of err (gocritic) if err2 := doIt(); err != nil { // expect to complain ^ INFO File cache stats: 1 entries of total size 584B INFO Memory: 4 samples, avg is 48.9MB, max is 60.3MB INFO Execution took 297.273113ms ```

Code example or link to a public repository

```go cat go.mod module gocheck go 1.20 cat main.go [uncheckedInlineErr.zip](https://github.com/golangci/golangci-lint/files/10813251/uncheckedInlineErr.zip) package main /* char *Test() { char *msg = "HelloTest"; return msg; } */ import "C" import ( "fmt" "errors" ) func doIt() error { return errors.New("error case") } func main() { var err, err2 error if err = doIt(); err2 != nil { // expect to complain fmt.Println(err) } if err2 = doIt(); err2 != nil { // expect no issue fmt.Println(err2) } cs := C.Test() value := C.GoString(cs) fmt.Printf("Cgo string: %s\n", value) if err2 := doIt(); err != nil { // expect to complain fmt.Println(err2) } } ```
boring-cyborg[bot] commented 1 year ago

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

ennc0d3 commented 1 year ago

Is there something that I can do to analyze, of course, I need help. Thanks

homme commented 1 year ago

I'm experiencing this bug in cgo code as well.

alexaandru commented 1 month ago

Same here, also using cgo: https://github.com/alexaandru/go-tree-sitter-bare/blob/v1.3.4/query.go#L216-L218

FWIW, before the last refactoring, I had the if ...; err != nil { return } if clause in each case in the switch above and it errored out there as well.