]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/vet: add directive analyzer
authorRuss Cox <rsc@golang.org>
Sat, 14 Jan 2023 18:57:08 +0000 (13:57 -0500)
committerGopher Robot <gobot@golang.org>
Mon, 30 Jan 2023 19:25:52 +0000 (19:25 +0000)
For #56986, add the new directive analyzer that catches
misplaced //go:debug lines.

Ran 'go mod vendor' after adding the import in vet
to bring in the vendored files.

A followup CL will enable it by default in 'go test'.

Change-Id: I12c46e292b31bdbf5ceb86ba4474545e78a83a47
Reviewed-on: https://go-review.googlesource.com/c/go/+/462201
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>

src/cmd/vendor/golang.org/x/tools/go/analysis/passes/directive/directive.go [new file with mode: 0644]
src/cmd/vendor/modules.txt
src/cmd/vet/main.go
src/cmd/vet/testdata/directive/directive.go [new file with mode: 0644]
src/cmd/vet/vet_test.go

diff --git a/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/directive/directive.go b/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/directive/directive.go
new file mode 100644 (file)
index 0000000..76d852c
--- /dev/null
@@ -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
+}
index f983c30a000c5ba4c1ef9743c73878ad30c49c53..c6492ca300bdf24f7e07e77d1819593f05e4012b 100644 (file)
@@ -60,6 +60,7 @@ golang.org/x/tools/go/analysis/passes/cgocall
 golang.org/x/tools/go/analysis/passes/composite
 golang.org/x/tools/go/analysis/passes/copylock
 golang.org/x/tools/go/analysis/passes/ctrlflow
+golang.org/x/tools/go/analysis/passes/directive
 golang.org/x/tools/go/analysis/passes/errorsas
 golang.org/x/tools/go/analysis/passes/framepointer
 golang.org/x/tools/go/analysis/passes/httpresponse
index 334179c19457417b98e294ae43a5bea9090d7f62..0bcee78b973153cc21c42e57123898a7502bbd1a 100644 (file)
@@ -17,6 +17,7 @@ import (
        "golang.org/x/tools/go/analysis/passes/cgocall"
        "golang.org/x/tools/go/analysis/passes/composite"
        "golang.org/x/tools/go/analysis/passes/copylock"
+       "golang.org/x/tools/go/analysis/passes/directive"
        "golang.org/x/tools/go/analysis/passes/errorsas"
        "golang.org/x/tools/go/analysis/passes/framepointer"
        "golang.org/x/tools/go/analysis/passes/httpresponse"
@@ -51,6 +52,7 @@ func main() {
                cgocall.Analyzer,
                composite.Analyzer,
                copylock.Analyzer,
+               directive.Analyzer,
                errorsas.Analyzer,
                framepointer.Analyzer,
                httpresponse.Analyzer,
diff --git a/src/cmd/vet/testdata/directive/directive.go b/src/cmd/vet/testdata/directive/directive.go
new file mode 100644 (file)
index 0000000..91ccb1b
--- /dev/null
@@ -0,0 +1,4 @@
+package p
+
+// ERRORNEXT "//go:debug directive only valid in package main or test"
+//go:debug panicnil=1
index 280ed8d7ae5fe9a2e04fc55757ff258de5f68378..fca9cac8c2232bf9dc2a661a9791c61a19f029b5 100644 (file)
@@ -71,6 +71,7 @@ func TestVet(t *testing.T) {
                "composite",
                "copylock",
                "deadcode",
+               "directive",
                "httpresponse",
                "lostcancel",
                "method",