x/tools/go/analysis/passes/directive: spurious requirement for blank line between //go:debug and package declaration

[deltamualpha]

Go version

go version go1.22.0 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/me/Library/Caches/go-build'
GOENV='/Users/me/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/me/go/pkg/mod'
GONOPROXY='private/*'
GONOSUMDB='private/*'
GOOS='darwin'
GOPATH='/Users/me/go'
GOPRIVATE='private/*'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.0/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/vl/k9dw37lx0x5bbtzj98plsqbh0000gp/T/go-build1408921875=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Prompted by a question from Sean Sorrell in the #general channel in the Gophers Slack about using go:debug directives in test files, I wrote the following file called main_test.go::

//go:debug panicnil=1
package main_test

import "testing"

func TestFoo(t *testing.T) {
	t.Errorf("oops!")
}

According to the documentation, this should pass go vet.

What did you see happen?

go vet . returns:

./main_test.go:1:1: //go:debug directive only valid before package declaration

What did you expect to see?

A clean go vet output.

[seankhliao]

Not just tests,

fails: https://go.dev/play/p/rM-aiRMn4rd

//go:debug panicnil=1
package main

func main() {
	panic(nil)
}

vs

passes: https://go.dev/play/p/BZY9qp1ilAI

//go:debug panicnil=1

package main

func main() {
	panic(nil)
}

cc @timothy-king

[adonovan]

I suspect the directive analyzer handles //go:debug comments this way for consistency with //go:build comments, which are "ignored adjoining the package declaration":

		// A +build comment is ignored after or adjoining the package declaration.
		if group.End()+1 >= f.Package {
			check.inHeader = false
		}

Assigning to @rsc to adjudicate the desired behavior.

[gopherbot]

Change https://go.dev/cl/569360 mentions this issue: go/analysis/passes/directive: do not report adjoining //go:debug

[ianlancetaylor]

//go:debug comments are parsed by the same code that parses //go:build comments. So the vet warning seems consistent.

That said, unless my testing is broken the parsing does currently permit the //go:build or //go:debug comment to appear as part of the package comment. The only requirement is that the comment appear before the package clause. That may be a bug.

[deltamualpha]

@ianlancetaylor I don't interpret "(preceding the package statement)" on the documentation page on go:debug as requiring the comment having a newline between it and package main, but perhaps I'm misunderstanding what "package clause"/"package statement"/"package comment" means in this context? The spec makes no reference to requiring white space between any comment and the package clause, but I suppose this is a toolchain-specific question and not a spec one.

[rsc]

// +build comments need a blank line, because otherwise a package doc comment talking about the +build feature might
accidentally turn into a semantically important comment after an unfortunate line wrap.

Newer directives of the //tool:thing form do not need that blank line and do not have the wrapping problem. As part of the richer doc comment syntax, we updated gofmt and friends to move such directives to the end of doc comments and not to print them as documentation.

Empirically, new-style directives, both //go:debug and //go:build, are recognized in package doc comments:

% cat x.go
// Hello world
//
//go:debug panicnil=2
package main
% go list -f '{{.DefaultGODEBUG}}' x.go
panicnil=2
% 

% cat x.go
// Hello world
//go:build linux
package main
% gofmt x.go | gofmt
//go:build linux

// Hello world
//
package main
% 

Since the standard tools recognize them in the package doc comment, x/tools/go/analysis should recognize them there too. But be sure not to start recognizing // +build in the package doc comment.

It is possible that the code has always been wrong but it didn't matter because gofmt moves //go:build lines out of package doc comments into stanzas of their own. So it goes.