go/analysis/passes/directive: add directive analyzer
The directive analyzer is a generalized version of
the buildtag analyzer, meant to apply checks about
any directives (//go:... lines) found in source code.
For now it only checks the placement of //go:debug lines.
For golang/go#56986.
Change-Id: I2bd3d743c44554711ada90f6ee53b6195dc55bcb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/462216
Run-TryBot: Russ Cox <rsc@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
diff --git a/go/analysis/passes/buildtag/buildtag.go b/go/analysis/passes/buildtag/buildtag.go
index c4407ad..775e507 100644
--- a/go/analysis/passes/buildtag/buildtag.go
+++ b/go/analysis/passes/buildtag/buildtag.go
@@ -20,7 +20,7 @@
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
)
-const Doc = "check that +build tags are well-formed and correctly located"
+const Doc = "check //go:build and // +build directives"
var Analyzer = &analysis.Analyzer{
Name: "buildtag",
diff --git a/go/analysis/passes/buildtag/buildtag_old.go b/go/analysis/passes/buildtag/buildtag_old.go
index e923492..0001ba5 100644
--- a/go/analysis/passes/buildtag/buildtag_old.go
+++ b/go/analysis/passes/buildtag/buildtag_old.go
@@ -22,7 +22,7 @@
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
)
-const Doc = "check that +build tags are well-formed and correctly located"
+const Doc = "check // +build directives"
var Analyzer = &analysis.Analyzer{
Name: "buildtag",
diff --git a/go/analysis/passes/directive/directive.go b/go/analysis/passes/directive/directive.go
new file mode 100644
index 0000000..76d852c
--- /dev/null
+++ b/go/analysis/passes/directive/directive.go
@@ -0,0 +1,216 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package directive defines an Analyzer that checks known Go toolchain directives.
+package directive
+
+import (
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "strings"
+ "unicode"
+ "unicode/utf8"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+)
+
+const Doc = `check Go toolchain directives such as //go:debug
+
+This analyzer checks for problems with known Go toolchain directives
+in all Go source files in a package directory, even those excluded by
+//go:build constraints, and all non-Go source files too.
+
+For //go:debug (see https://go.dev/doc/godebug), the analyzer checks
+that the directives are placed only in Go source files, only above the
+package comment, and only in package main or *_test.go files.
+
+Support for other known directives may be added in the future.
+
+This analyzer does not check //go:build, which is handled by the
+buildtag analyzer.
+`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "directive",
+ Doc: Doc,
+ Run: runDirective,
+}
+
+func runDirective(pass *analysis.Pass) (interface{}, error) {
+ for _, f := range pass.Files {
+ checkGoFile(pass, f)
+ }
+ for _, name := range pass.OtherFiles {
+ if err := checkOtherFile(pass, name); err != nil {
+ return nil, err
+ }
+ }
+ for _, name := range pass.IgnoredFiles {
+ if strings.HasSuffix(name, ".go") {
+ f, err := parser.ParseFile(pass.Fset, name, nil, parser.ParseComments)
+ if err != nil {
+ // Not valid Go source code - not our job to diagnose, so ignore.
+ continue
+ }
+ checkGoFile(pass, f)
+ } else {
+ if err := checkOtherFile(pass, name); err != nil {
+ return nil, err
+ }
+ }
+ }
+ return nil, nil
+}
+
+func checkGoFile(pass *analysis.Pass, f *ast.File) {
+ check := newChecker(pass, pass.Fset.File(f.Package).Name(), f)
+
+ for _, group := range f.Comments {
+ // A +build comment is ignored after or adjoining the package declaration.
+ if group.End()+1 >= f.Package {
+ check.inHeader = false
+ }
+ // A //go:build comment is ignored after the package declaration
+ // (but adjoining it is OK, in contrast to +build comments).
+ if group.Pos() >= f.Package {
+ check.inHeader = false
+ }
+
+ // Check each line of a //-comment.
+ for _, c := range group.List {
+ check.comment(c.Slash, c.Text)
+ }
+ }
+}
+
+func checkOtherFile(pass *analysis.Pass, filename string) error {
+ // We cannot use the Go parser, since is not a Go source file.
+ // Read the raw bytes instead.
+ content, tf, err := analysisutil.ReadFile(pass.Fset, filename)
+ if err != nil {
+ return err
+ }
+
+ check := newChecker(pass, filename, nil)
+ check.nonGoFile(token.Pos(tf.Base()), string(content))
+ return nil
+}
+
+type checker struct {
+ pass *analysis.Pass
+ filename string
+ file *ast.File // nil for non-Go file
+ inHeader bool // in file header (before package declaration)
+ inStar bool // currently in a /* */ comment
+}
+
+func newChecker(pass *analysis.Pass, filename string, file *ast.File) *checker {
+ return &checker{
+ pass: pass,
+ filename: filename,
+ file: file,
+ inHeader: true,
+ }
+}
+
+func (check *checker) nonGoFile(pos token.Pos, fullText string) {
+ // Process each line.
+ text := fullText
+ inStar := false
+ for text != "" {
+ offset := len(fullText) - len(text)
+ var line string
+ line, text, _ = stringsCut(text, "\n")
+
+ if !inStar && strings.HasPrefix(line, "//") {
+ check.comment(pos+token.Pos(offset), line)
+ continue
+ }
+
+ // Skip over, cut out any /* */ comments,
+ // to avoid being confused by a commented-out // comment.
+ for {
+ line = strings.TrimSpace(line)
+ if inStar {
+ var ok bool
+ _, line, ok = stringsCut(line, "*/")
+ if !ok {
+ break
+ }
+ inStar = false
+ continue
+ }
+ line, inStar = stringsCutPrefix(line, "/*")
+ if !inStar {
+ break
+ }
+ }
+ if line != "" {
+ // Found non-comment non-blank line.
+ // Ends space for valid //go:build comments,
+ // but also ends the fraction of the file we can
+ // reliably parse. From this point on we might
+ // incorrectly flag "comments" inside multiline
+ // string constants or anything else (this might
+ // not even be a Go program). So stop.
+ break
+ }
+ }
+}
+
+func (check *checker) comment(pos token.Pos, line string) {
+ if !strings.HasPrefix(line, "//go:") {
+ return
+ }
+ // testing hack: stop at // ERROR
+ if i := strings.Index(line, " // ERROR "); i >= 0 {
+ line = line[:i]
+ }
+
+ verb := line
+ if i := strings.IndexFunc(verb, unicode.IsSpace); i >= 0 {
+ verb = verb[:i]
+ if line[i] != ' ' && line[i] != '\t' && line[i] != '\n' {
+ r, _ := utf8.DecodeRuneInString(line[i:])
+ check.pass.Reportf(pos, "invalid space %#q in %s directive", r, verb)
+ }
+ }
+
+ switch verb {
+ default:
+ // TODO: Use the go language version for the file.
+ // If that version is not newer than us, then we can
+ // report unknown directives.
+
+ case "//go:build":
+ // Ignore. The buildtag analyzer reports misplaced comments.
+
+ case "//go:debug":
+ if check.file == nil {
+ check.pass.Reportf(pos, "//go:debug directive only valid in Go source files")
+ } else if check.file.Name.Name != "main" && !strings.HasSuffix(check.filename, "_test.go") {
+ check.pass.Reportf(pos, "//go:debug directive only valid in package main or test")
+ } else if !check.inHeader {
+ check.pass.Reportf(pos, "//go:debug directive only valid before package declaration")
+ }
+ }
+}
+
+// Go 1.18 strings.Cut.
+func stringsCut(s, sep string) (before, after string, found bool) {
+ if i := strings.Index(s, sep); i >= 0 {
+ return s[:i], s[i+len(sep):], true
+ }
+ return s, "", false
+}
+
+// Go 1.20 strings.CutPrefix.
+func stringsCutPrefix(s, prefix string) (after string, found bool) {
+ if !strings.HasPrefix(s, prefix) {
+ return s, false
+ }
+ return s[len(prefix):], true
+}
diff --git a/go/analysis/passes/directive/directive_test.go b/go/analysis/passes/directive/directive_test.go
new file mode 100644
index 0000000..a526c0d
--- /dev/null
+++ b/go/analysis/passes/directive/directive_test.go
@@ -0,0 +1,39 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package directive_test
+
+import (
+ "runtime"
+ "strings"
+ "testing"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/passes/directive"
+)
+
+func Test(t *testing.T) {
+ if strings.HasPrefix(runtime.Version(), "go1.") && runtime.Version() < "go1.16" {
+ t.Skipf("skipping on %v", runtime.Version())
+ }
+ analyzer := *directive.Analyzer
+ analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
+ defer func() {
+ // The directive pass is unusual in that it checks the IgnoredFiles.
+ // After analysis, add IgnoredFiles to OtherFiles so that
+ // the test harness checks for expected diagnostics in those.
+ // (The test harness shouldn't do this by default because most
+ // passes can't do anything with the IgnoredFiles without type
+ // information, which is unavailable because they are ignored.)
+ var files []string
+ files = append(files, pass.OtherFiles...)
+ files = append(files, pass.IgnoredFiles...)
+ pass.OtherFiles = files
+ }()
+
+ return directive.Analyzer.Run(pass)
+ }
+ analysistest.Run(t, analysistest.TestData(), &analyzer, "a")
+}
diff --git a/go/analysis/passes/directive/testdata/src/a/badspace.go b/go/analysis/passes/directive/testdata/src/a/badspace.go
new file mode 100644
index 0000000..1131399
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/badspace.go
@@ -0,0 +1,11 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+// want +1 `invalid space '\\u00a0' in //go:debug directive`
+//go:debug 00a0
+
+package main
+
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced.go b/go/analysis/passes/directive/testdata/src/a/misplaced.go
new file mode 100644
index 0000000..db30ceb
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced.go
@@ -0,0 +1,10 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+// want +1 `//go:debug directive only valid before package declaration`
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced.s b/go/analysis/passes/directive/testdata/src/a/misplaced.s
new file mode 100644
index 0000000..9e26dbc
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced.s
@@ -0,0 +1,19 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// want +1 `//go:debug directive only valid in Go source files`
+//go:debug panicnil=1
+
+/*
+can skip over comments
+//go:debug doesn't matter here
+*/
+
+// want +1 `//go:debug directive only valid in Go source files`
+//go:debug panicnil=1
+
+package a
+
+// no error here because we can't parse this far
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced_test.go b/go/analysis/passes/directive/testdata/src/a/misplaced_test.go
new file mode 100644
index 0000000..6b4527a
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced_test.go
@@ -0,0 +1,10 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:debug panicnil=1
+
+package p_test
+
+// want +1 `//go:debug directive only valid before package declaration`
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/p.go b/go/analysis/passes/directive/testdata/src/a/p.go
new file mode 100644
index 0000000..e1e3e65
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/p.go
@@ -0,0 +1,11 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// want +1 `//go:debug directive only valid in package main or test`
+//go:debug panicnil=1
+
+package p
+
+// want +1 `//go:debug directive only valid in package main or test`
+//go:debug panicnil=1
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 28bf1de..a1134be 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -48,7 +48,7 @@
## **buildtag**
-check that +build tags are well-formed and correctly located
+check //go:build and // +build directives
**Enabled by default.**
@@ -108,6 +108,26 @@
**Enabled by default.**
+## **directive**
+
+check Go toolchain directives such as //go:debug
+
+This analyzer checks for problems with known Go toolchain directives
+in all Go source files in a package directory, even those excluded by
+//go:build constraints, and all non-Go source files too.
+
+For //go:debug (see https://go.dev/doc/godebug), the analyzer checks
+that the directives are placed only in Go source files, only above the
+package comment, and only in package main or *_test.go files.
+
+Support for other known directives may be added in the future.
+
+This analyzer does not check //go:build, which is handled by the
+buildtag analyzer.
+
+
+**Enabled by default.**
+
## **embed**
check for //go:embed directive import
diff --git a/gopls/internal/lsp/source/api_json.go b/gopls/internal/lsp/source/api_json.go
index 0d24ece..e97c147 100755
--- a/gopls/internal/lsp/source/api_json.go
+++ b/gopls/internal/lsp/source/api_json.go
@@ -236,7 +236,7 @@
},
{
Name: "\"buildtag\"",
- Doc: "check that +build tags are well-formed and correctly located",
+ Doc: "check //go:build and // +build directives",
Default: "true",
},
{
@@ -260,6 +260,11 @@
Default: "true",
},
{
+ Name: "\"directive\"",
+ Doc: "check Go toolchain directives such as //go:debug\n\nThis analyzer checks for problems with known Go toolchain directives\nin all Go source files in a package directory, even those excluded by\n//go:build constraints, and all non-Go source files too.\n\nFor //go:debug (see https://go.dev/doc/godebug), the analyzer checks\nthat the directives are placed only in Go source files, only above the\npackage comment, and only in package main or *_test.go files.\n\nSupport for other known directives may be added in the future.\n\nThis analyzer does not check //go:build, which is handled by the\nbuildtag analyzer.\n",
+ Default: "true",
+ },
+ {
Name: "\"embed\"",
Doc: "check for //go:embed directive import\n\nThis analyzer checks that the embed package is imported when source code contains //go:embed comment directives.\nThe embed package must be imported for //go:embed directives to function.import _ \"embed\".",
Default: "true",
@@ -875,7 +880,7 @@
},
{
Name: "buildtag",
- Doc: "check that +build tags are well-formed and correctly located",
+ Doc: "check //go:build and // +build directives",
Default: true,
},
{
@@ -899,6 +904,11 @@
Default: true,
},
{
+ Name: "directive",
+ Doc: "check Go toolchain directives such as //go:debug\n\nThis analyzer checks for problems with known Go toolchain directives\nin all Go source files in a package directory, even those excluded by\n//go:build constraints, and all non-Go source files too.\n\nFor //go:debug (see https://go.dev/doc/godebug), the analyzer checks\nthat the directives are placed only in Go source files, only above the\npackage comment, and only in package main or *_test.go files.\n\nSupport for other known directives may be added in the future.\n\nThis analyzer does not check //go:build, which is handled by the\nbuildtag analyzer.\n",
+ Default: true,
+ },
+ {
Name: "embed",
Doc: "check for //go:embed directive import\n\nThis analyzer checks that the embed package is imported when source code contains //go:embed comment directives.\nThe embed package must be imported for //go:embed directives to function.import _ \"embed\".",
Default: true,
diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go
index 01feb21..88a3678 100644
--- a/gopls/internal/lsp/source/options.go
+++ b/gopls/internal/lsp/source/options.go
@@ -26,6 +26,7 @@
"golang.org/x/tools/go/analysis/passes/composite"
"golang.org/x/tools/go/analysis/passes/copylock"
"golang.org/x/tools/go/analysis/passes/deepequalerrors"
+ "golang.org/x/tools/go/analysis/passes/directive"
"golang.org/x/tools/go/analysis/passes/errorsas"
"golang.org/x/tools/go/analysis/passes/fieldalignment"
"golang.org/x/tools/go/analysis/passes/httpresponse"
@@ -1422,6 +1423,7 @@
cgocall.Analyzer.Name: {Analyzer: cgocall.Analyzer, Enabled: true},
composite.Analyzer.Name: {Analyzer: composite.Analyzer, Enabled: true},
copylock.Analyzer.Name: {Analyzer: copylock.Analyzer, Enabled: true},
+ directive.Analyzer.Name: {Analyzer: directive.Analyzer, Enabled: true},
errorsas.Analyzer.Name: {Analyzer: errorsas.Analyzer, Enabled: true},
httpresponse.Analyzer.Name: {Analyzer: httpresponse.Analyzer, Enabled: true},
ifaceassert.Analyzer.Name: {Analyzer: ifaceassert.Analyzer, Enabled: true},
diff --git a/internal/gcimporter/testdata/a/a.go b/internal/gcimporter/testdata/a/a.go
new file mode 100644
index 0000000..56e4292
--- /dev/null
+++ b/internal/gcimporter/testdata/a/a.go
@@ -0,0 +1,14 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Input for TestIssue13566
+
+package a
+
+import "encoding/json"
+
+type A struct {
+ a *A
+ json json.RawMessage
+}
diff --git a/internal/gcimporter/testdata/issue51836/a/a.go b/internal/gcimporter/testdata/issue51836/a/a.go
new file mode 100644
index 0000000..e9223c9
--- /dev/null
+++ b/internal/gcimporter/testdata/issue51836/a/a.go
@@ -0,0 +1,8 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package a
+
+type T[K any] struct {
+}
