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

x/tools/go/analysis/passes/nilness: no position information with error #31008

Open
reillywatson opened this issue Mar 22, 2019 · 7 comments
Open
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@reillywatson
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
1.12

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/reilly/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/reilly/league/services:/Users/reilly/go:"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gz/4ytqr0r12q1fbx8ct_w3k_880000gn/T/go-build175674008=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Ran the nilness analyzer on this (simplified) package:

package foo

import (
	"fmt"
)

func Foo() {
	var x []int
	for _, a := range x {
		fmt.Println(a)
	}
}

What did you expect to see?

Ideally: a different error about how you're iterating over a nil slice
Less ideally: the same error, but with information telling me where the error is

What did you see instead?

-: nil dereference in index operation

This kind of error is pretty hard to track down if I'm running the analyzer over a large codebase, because it doesn't say which file or package is failing.

@julieqiu
Copy link
Member

/cc @matloob

@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 28, 2019
@acestronautical
Copy link

acestronautical commented Jul 12, 2019

This is a major problem that deserves attention. I work for a medium sized tech company looking to adopt golangci-lint and this issue is holding me back.

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
vovapi added a commit to vovapi/tools that referenced this issue Nov 23, 2019
Indexed range loops are lacking position data
in their SSA representation, so add the position data
into an index operation

This improves the problem described in golang/go#31008
by providing position data with the nilness message
@gopherbot
Copy link

Change https://golang.org/cl/208599 mentions this issue: go/ssa: add position data in indexed range loops

gopherbot pushed a commit to golang/tools that referenced this issue Jan 9, 2020
Indexed range loops are lacking position data
in their SSA representation, so add the position data
into an index operation.

This improves the nilness output by providing position data
with the message.

Updates golang/go#31008

Change-Id: I664c8668bc74207e770657e21129d20fd70e7af6
GitHub-Last-Rev: 75831d2
GitHub-Pull-Request: #190
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208599
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@findleyr
Copy link
Contributor

findleyr commented Jan 9, 2020

@vovapi's CL above adds position information to the error message, but I think to improve this further we'd have alter the way go/ssa tracks position information. Something similar was done for staticcheck: dominikh/go-tools@c14c261

@uhthomas
Copy link

Thought it might be worth adding a case where the nilness analyzer fails to provide position information where it seemingly should do so.

func do(a interface{}) {
	switch a.(type) {
	case nil:
		return
	}
	switch a.(type) {
	case nil:
		return
	}
}

https://go.dev/play/p/3K_9FFGOWzd

❯ go run golang.org/x/tools/go/analysis/passes/nilness/cmd/nilness@latest test.go
-: impossible condition: non-nil == nil
exit status 3

@gopherbot
Copy link

Change https://go.dev/cl/394694 mentions this issue: go/ssa: add position information for switch case conditions

@zpavlinovic zpavlinovic added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Mar 22, 2022
gopherbot pushed a commit to golang/tools that referenced this issue Mar 23, 2022
This allows better reporting in passes, such as nilness.

Updates golang/go#31008

Change-Id: Ie188844b550bf2b924747f3ac476dd6feeda6d6c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/394694
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
@zpavlinovic
Copy link
Contributor

Thought it might be worth adding a case where the nilness analyzer fails to provide position information where it seemingly should do so.

...

Change https://go.dev/cl/394694 should have now addressed the issue with positions in case conditions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

7 participants