x/tools/gopls/internal/analysis/yield: reifying control decisions as booleans => false positives

[firelizzard18]

gopls version

golang.org/x/tools/gopls v0.19.0-pre.3

go env

AR='ar'
CC='x86_64-pc-linux-gnu-gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='x86_64-pc-linux-gnu-g++'
GCCGO='/usr/bin/gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/firelizzard/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/firelizzard/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2703441983=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/firelizzard/src/misc/hermes/go.mod'
GOMODCACHE='/home/firelizzard/go/pkg/mod'
GONOPROXY='REDACTED'
GONOSUMDB='REDACTED'
GOOS='linux'
GOPATH='/home/firelizzard/go'
GOPRIVATE='REDACTED'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/firelizzard/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24.2'
GOWORK='/home/firelizzard/src/misc/hermes/go.work'
PKG_CONFIG='pkg-config'

What did you do?

func (x *Transformer) getTransitiveDependencies(name string) iter.Seq[string] {
	return func(yield func(string) bool) {
		x.mu.Lock()
		defer x.mu.Unlock()
		x._getTransitiveDependencies(name, yield)
	}
}

func (x *Transformer) _getTransitiveDependencies(name string, yield func(string) bool) bool {
	for _, dep := range x.dependencies[name] {
		var ok bool
		if dep.External {
			ok = yield(dep.Name)
		} else {
			ok = x._getTransitiveDependencies(dep.Name, yield)
		}
		if !ok {
			return false
		}
	}
	return true
}

What did you see happen?

What did you expect to see?

No warning

Editor and settings

No response

Logs

No response

[adonovan]

Thanks for the report. Indeed the analysis fares poorly when control flow decisions are reified as booleans.

A simple workaround (and one that IMHO improves clarity in this particular case) is to expand out the returns, but I understand that that is unsatisfactory in general:

func (x *Transformer) _getTransitiveDependencies(name string, yield func(string) bool) bool {
	for _, dep := range x.dependencies[name] {
		if dep.External {
			if !yield(dep.Name) {
				return false
			}
		} else if !x._getTransitiveDependencies(dep.Name, yield) {
			return false
		}
	}
	return true
}