staticcheck: SA5011 false positive

[fhs]

$ staticcheck -debug.version
staticcheck (devel, v0.0.0-20191028234905-f51cd49dbadb)

Compiled with Go version: devel +6453337494 Tue Oct 29 17:38:23 2019 +0000
Main module:
	honnef.co/go/[email protected] (sum: h1:zwVuj3NBk70CHiI+u8OAjE2u05Xq1M/8845ac9+uwMY=)
Dependencies:
	github.com/BurntSushi/[email protected] (sum: h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=)
	golang.org/x/[email protected] (sum: h1:ZLYwrbapYXNQ3n6e8thW6VN7mHoao8ig10ydUi8VO4Q=)

Running staticcheck on this program:

package main

type T struct {
	w *int
}

func main() {
	var t *T

	if t == nil {
		return
	}
	w := t.w
	if t == nil {
		return
	}
	println(w)
}

Gives this output:

main.go:13:9: possible nil pointer dereference (SA5011)
	main.go:14:5: this check suggests that the pointer can be nil

t will not be nil because of the check before it's being used. Staticcheck is being fooled by the second check.

[dominikh]

It is a false positive, and we could make an effort to avoid it, but it's not a high priority to me. I'd prefer if people took it as a suggestion to remove the unnecessary nil check instead.

Is this false positive occurring in a context where an unnecessary check cannot be removed?

[fhs]

I think the unnecessary check can be removed. This is the context: edwood/acme.go (some really old C code that's been ported to Go).

[dominikh]

I am going to close this issue as "working as designed". The check flags interactions of pointer dereferences and nil pointer checks, and it is correct in stating that the check suggests the possible presence of a nil pointer. Removing the impossible nil pointer check is the correct fix here.

[mpictor]

Just got bit by something related - staticcheck complained of a possible nil dereference, so I added a check at the top of the function. It kept on complaining, which made me think it somehow didn't understand the nil check. Only after reading this issue did I notice there was a second nil check later in the function, below the lines it'd complained about.

Would it be possible to update the message? Maybe something like

-possible nil pointer dereference (SA5011)
+possible nil pointer dereference (or extra nil check below) (SA5011)

I'm using staticcheck as bundled in

golangci-lint has version 1.27.0 built from fb74c2e on 2020-05-13T18:48:26Z

[dominikh]

@mpictor when running staticcheck on this function

func fn(x *byte) {
	println(*x)
	if x != nil {
		return
	}
}

it outputs the following diagnostic

foo.go:4:10: possible nil pointer dereference (SA5011)
	foo.go:5:5: this check suggests that the pointer can be nil

pointing at both the dereference, as well as the relevant nil pointer check.

It seems that golangci-lint neglects to point at the actual check:

foo.go:4:10: SA5011: possible nil pointer dereference (staticcheck)
	println(*x)

This is an issue in golangci-lint, not staticcheck. In go/analysis terms, golangci-lint doesn't seem to make use of RelatedInformation.

[mpictor]

thanks for looking @dominikh, I'll ask golangci-lint to fix