go-tools icon indicating copy to clipboard operation
go-tools copied to clipboard

U1000 false positive when passing to reflect.TypeFor

Open tkw1536 opened this issue 11 months ago • 3 comments

In the following code ( Playground Link ):

package main

import (
	"fmt"
	"reflect"
)

type something struct {
	x string
}

func main() {
	typ := reflect.TypeFor[something]()
	for i := range typ.NumField() {
		fmt.Println(typ.Field(i).Name)
	}
}

The x field of the something struct is marked as unused by U1000:

$ staticcheck .                                                                                                                                                                              
example.go:9:2: field x is unused (U1000)

However, it is clearly being used: The name appears in the output of the program. Removing the field results in a blank output.

Version Details:

$ staticcheck -version                                                                                                                                                                      
staticcheck 2025.1.1 (0.6.1)
$ staticcheck -debug.version
staticcheck 2025.1.1 (0.6.1)

Compiled with Go version: go1.24.2
Main module:
        honnef.co/go/[email protected] (sum: h1:R094WgE8K4JirYjBaOpz/AvTyUu/3wbmAoskKN/pxTI=)
Dependencies:
        github.com/BurntSushi/[email protected] (sum: h1:pxW6RcqyfI9/kWtOwnv/G+AzdKuy2ZrqINhenH4HyNs=)
        golang.org/x/exp/[email protected] (sum: h1:1P7xPZEwZMoBoz0Yze5Nx2/4pxj6nw9ZqHWXqP0iRgQ=)
        golang.org/x/[email protected] (sum: h1:Zb7khfcRGKk+kqfxFaP5tZqCnDZMjC5VtUBs87Hr6QM=)
        golang.org/x/[email protected] (sum: h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w=)
        golang.org/x/[email protected] (sum: h1:BgcpHewrV5AUp2G9MebG4XPFI1E2W41zU1SaqVA9vJY=)
$ go version
go version go1.24.2 darwin/arm64
$ go env
AR='ar'
CC='cc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='c++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/username/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/username/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/lx/0flh25sd5xv0mnjwvv6m7xd80000gn/T/go-build1801652558=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/username/tmp/example/go.mod'
GOMODCACHE='/Users/username/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/username/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.24.2/libexec'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/username/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.24.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.2'
GOWORK=''
PKG_CONFIG='pkg-config'

tkw1536 avatar May 06 '25 20:05 tkw1536

In a strict sense you are right. But how realistic is this exact constellation of code, where you're (dynamically) interested in the field

  • existing
  • being unexported
  • having a name
  • never being assigned to?

Under any other circumstance, the field would be considered used.

dominikh avatar May 06 '25 23:05 dominikh

I definitely see your point. For the issue I attempted to reduce it to a minimal test case that showed only the concrete problem I encountered.

In my actual use case, I encountered this related to a real world in a library function that violated the third law of reflection with code like:

func UpdateX(s any /* typed as any, but always has a x field */) {
	r := reflect.ValueOf(s).Elem().FieldByName("x")
        // "forget" it was obtained from an unsettable field
	r = reflect.NewAt(r.Type(), r.Addr().UnsafePointer()).Elem()

        // we can now set the field
	r.SetString("some computed value")
}

The real code actually involved reading the "x" field elsewhere, so it was not flagged.

However, in my test code I did:

func ExampleUpdateX() {
	type something struct {
		x string // stuff_test.go:10:3: field x is unused (U1000)
	}

	var s something
	example.UpdateX(&s)

	// Output: {some computed value}
	fmt.Println(s)
}

Which resulted in the U1000.

tkw1536 avatar May 07 '25 09:05 tkw1536

That's fair, but probably not something we can fix without either missing the majority of unused, unexported fields--by assuming that all use of reflection can lead to violations of the third law--or by still having false positives and playing whack-a-mole, by trying to conservatively detect patterns of this happening. And of course this is just a special case of unsafe code in general, which can do anything it wants and completely defy U1000.

I think the best we can do is acknowledge that this false positive exists and encourage working around it.

dominikh avatar May 07 '25 15:05 dominikh