go/analysis/passes/buildtag: update check for //go:build

Part of //go:build change (#41184).
See https://golang.org/design/draft-gobuild

Report misplaced and malformed //go:build lines,
like we've always done for // +build lines.

Report when the // +build lines do not say the same thing as the //go:build line.
(Running gofmt will always bring them back in sync.)

For Go 1.17 vet.

Change-Id: Ifd22c6e8edcfeedc37851ba514712c5c057ad4b8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/293834
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This commit is contained in:
Russ Cox 2021-02-18 14:24:10 -05:00
parent 06713c25cb
commit f47cb783b1
12 changed files with 545 additions and 83 deletions

View File

@ -354,13 +354,13 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis
// Any comment starting with "want" is treated
// as an expectation, even without following whitespace.
if rest := strings.TrimPrefix(text, "want"); rest != text {
expects, err := parseExpectations(rest)
lineDelta, expects, err := parseExpectations(rest)
if err != nil {
t.Errorf("%s:%d: in 'want' comment: %s", filename, linenum, err)
return
}
if expects != nil {
want[key{filename, linenum}] = expects
want[key{filename, linenum + lineDelta}] = expects
}
}
}
@ -526,13 +526,13 @@ func (ex expectation) String() string {
// parseExpectations parses the content of a "// want ..." comment
// and returns the expectations, a mixture of diagnostics ("rx") and
// facts (name:"rx").
func parseExpectations(text string) ([]expectation, error) {
func parseExpectations(text string) (lineDelta int, expects []expectation, err error) {
var scanErr string
sc := new(scanner.Scanner).Init(strings.NewReader(text))
sc.Error = func(s *scanner.Scanner, msg string) {
scanErr = msg // e.g. bad string escape
}
sc.Mode = scanner.ScanIdents | scanner.ScanStrings | scanner.ScanRawStrings
sc.Mode = scanner.ScanIdents | scanner.ScanStrings | scanner.ScanRawStrings | scanner.ScanInts
scanRegexp := func(tok rune) (*regexp.Regexp, error) {
if tok != scanner.String && tok != scanner.RawString {
@ -543,14 +543,19 @@ func parseExpectations(text string) ([]expectation, error) {
return regexp.Compile(pattern)
}
var expects []expectation
for {
tok := sc.Scan()
switch tok {
case '+':
tok = sc.Scan()
if tok != scanner.Int {
return 0, nil, fmt.Errorf("got +%s, want +Int", scanner.TokenString(tok))
}
lineDelta, _ = strconv.Atoi(sc.TokenText())
case scanner.String, scanner.RawString:
rx, err := scanRegexp(tok)
if err != nil {
return nil, err
return 0, nil, err
}
expects = append(expects, expectation{"diagnostic", "", rx})
@ -558,24 +563,24 @@ func parseExpectations(text string) ([]expectation, error) {
name := sc.TokenText()
tok = sc.Scan()
if tok != ':' {
return nil, fmt.Errorf("got %s after %s, want ':'",
return 0, nil, fmt.Errorf("got %s after %s, want ':'",
scanner.TokenString(tok), name)
}
tok = sc.Scan()
rx, err := scanRegexp(tok)
if err != nil {
return nil, err
return 0, nil, err
}
expects = append(expects, expectation{"fact", name, rx})
case scanner.EOF:
if scanErr != "" {
return nil, fmt.Errorf("%s", scanErr)
return 0, nil, fmt.Errorf("%s", scanErr)
}
return expects, nil
return lineDelta, expects, nil
default:
return nil, fmt.Errorf("unexpected %s", scanner.TokenString(tok))
return 0, nil, fmt.Errorf("unexpected %s", scanner.TokenString(tok))
}
}
}

View File

@ -2,14 +2,17 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
//go:build go1.16
// +build go1.16
// Package buildtag defines an Analyzer that checks build tags.
package buildtag
import (
"bytes"
"fmt"
"go/ast"
"go/build/constraint"
"go/parser"
"go/token"
"strings"
"unicode"
@ -52,118 +55,313 @@ func runBuildTag(pass *analysis.Pass) (interface{}, error) {
}
func checkGoFile(pass *analysis.Pass, f *ast.File) {
pastCutoff := false
var check checker
check.init(pass)
defer check.finish()
for _, group := range f.Comments {
// A +build comment is ignored after or adjoining the package declaration.
if group.End()+1 >= f.Package {
pastCutoff = true
check.plusBuildOK = false
}
// "+build" is ignored within or after a /*...*/ comment.
if !strings.HasPrefix(group.List[0].Text, "//") {
pastCutoff = true
continue
// 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.goBuildOK = false
}
// Check each line of a //-comment.
for _, c := range group.List {
if !strings.Contains(c.Text, "+build") {
continue
}
if err := checkLine(c.Text, pastCutoff); err != nil {
pass.Reportf(c.Pos(), "%s", err)
// "+build" is ignored within or after a /*...*/ comment.
if !strings.HasPrefix(c.Text, "//") {
check.plusBuildOK = false
}
check.comment(c.Slash, c.Text)
}
}
}
func checkOtherFile(pass *analysis.Pass, filename string) error {
var check checker
check.init(pass)
defer check.finish()
// We cannot use the Go parser, since this may not be a Go source file.
// Read the raw bytes instead.
content, tf, err := analysisutil.ReadFile(pass.Fset, filename)
if err != nil {
return err
}
// We must look at the raw lines, as build tags may appear in non-Go
// files such as assembly files.
lines := bytes.SplitAfter(content, nl)
check.file(token.Pos(tf.Base()), string(content))
return nil
}
type checker struct {
pass *analysis.Pass
plusBuildOK bool // "+build" lines still OK
goBuildOK bool // "go:build" lines still OK
crossCheck bool // cross-check go:build and +build lines when done reading file
inStar bool // currently in a /* */ comment
goBuildPos token.Pos // position of first go:build line found
plusBuildPos token.Pos // position of first "+build" line found
goBuild constraint.Expr // go:build constraint found
plusBuild constraint.Expr // AND of +build constraints found
}
func (check *checker) init(pass *analysis.Pass) {
check.pass = pass
check.goBuildOK = true
check.plusBuildOK = true
check.crossCheck = true
}
func (check *checker) file(pos token.Pos, text string) {
// Determine cutpoint where +build comments are no longer valid.
// They are valid in leading // comments in the file followed by
// a blank line.
//
// This must be done as a separate pass because of the
// requirement that the comment be followed by a blank line.
var cutoff int
for i, line := range lines {
line = bytes.TrimSpace(line)
if !bytes.HasPrefix(line, slashSlash) {
if len(line) > 0 {
break
var plusBuildCutoff int
fullText := text
for text != "" {
i := strings.Index(text, "\n")
if i < 0 {
i = len(text)
} else {
i++
}
offset := len(fullText) - len(text)
line := text[:i]
text = text[i:]
line = strings.TrimSpace(line)
if !strings.HasPrefix(line, "//") && line != "" {
break
}
if line == "" {
plusBuildCutoff = offset
}
}
// Process each line.
// Must stop once we hit goBuildOK == false
text = fullText
check.inStar = false
for text != "" {
i := strings.Index(text, "\n")
if i < 0 {
i = len(text)
} else {
i++
}
offset := len(fullText) - len(text)
line := text[:i]
text = text[i:]
check.plusBuildOK = offset < plusBuildCutoff
if strings.HasPrefix(line, "//") {
check.comment(pos+token.Pos(offset), line)
continue
}
// Keep looking for the point at which //go:build comments
// stop being allowed. Skip over, cut out any /* */ comments.
for {
line = strings.TrimSpace(line)
if check.inStar {
i := strings.Index(line, "*/")
if i < 0 {
line = ""
break
}
line = line[i+len("*/"):]
check.inStar = false
continue
}
cutoff = i
if strings.HasPrefix(line, "/*") {
check.inStar = true
line = line[len("/*"):]
continue
}
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
}
}
for i, line := range lines {
line = bytes.TrimSpace(line)
if !bytes.HasPrefix(line, slashSlash) {
continue
}
if !bytes.Contains(line, []byte("+build")) {
continue
}
if err := checkLine(string(line), i >= cutoff); err != nil {
pass.Reportf(analysisutil.LineStart(tf, i+1), "%s", err)
continue
}
}
return nil
}
// checkLine checks a line that starts with "//" and contains "+build".
func checkLine(line string, pastCutoff bool) error {
line = strings.TrimPrefix(line, "//")
line = strings.TrimSpace(line)
func (check *checker) comment(pos token.Pos, text string) {
if strings.HasPrefix(text, "//") {
if strings.Contains(text, "+build") {
check.plusBuildLine(pos, text)
}
if strings.Contains(text, "//go:build") {
check.goBuildLine(pos, text)
}
}
if strings.HasPrefix(text, "/*") {
if i := strings.Index(text, "\n"); i >= 0 {
// multiline /* */ comment - process interior lines
check.inStar = true
i++
pos += token.Pos(i)
text = text[i:]
for text != "" {
i := strings.Index(text, "\n")
if i < 0 {
i = len(text)
} else {
i++
}
line := text[:i]
if strings.HasPrefix(line, "//") {
check.comment(pos, line)
}
pos += token.Pos(i)
text = text[i:]
}
check.inStar = false
}
}
}
if strings.HasPrefix(line, "+build") {
fields := strings.Fields(line)
if fields[0] != "+build" {
// Comment is something like +buildasdf not +build.
return fmt.Errorf("possible malformed +build comment")
}
if pastCutoff {
return fmt.Errorf("+build comment must appear before package clause and be followed by a blank line")
}
if err := checkArguments(fields); err != nil {
return err
func (check *checker) goBuildLine(pos token.Pos, line string) {
if !constraint.IsGoBuild(line) {
if !strings.HasPrefix(line, "//go:build") && constraint.IsGoBuild("//"+strings.TrimSpace(line[len("//"):])) {
check.pass.Reportf(pos, "malformed //go:build line (space between // and go:build)")
}
return
}
if !check.goBuildOK || check.inStar {
check.pass.Reportf(pos, "misplaced //go:build comment")
check.crossCheck = false
return
}
if check.goBuildPos == token.NoPos {
check.goBuildPos = pos
} else {
// Comment with +build but not at beginning.
if !pastCutoff {
return fmt.Errorf("possible malformed +build comment")
}
check.pass.Reportf(pos, "unexpected extra //go:build line")
check.crossCheck = false
}
// testing hack: stop at // ERROR
if i := strings.Index(line, " // ERROR "); i >= 0 {
line = line[:i]
}
x, err := constraint.Parse(line)
if err != nil {
check.pass.Reportf(pos, "%v", err)
check.crossCheck = false
return
}
if check.goBuild == nil {
check.goBuild = x
}
return nil
}
func checkArguments(fields []string) error {
func (check *checker) plusBuildLine(pos token.Pos, line string) {
line = strings.TrimSpace(line)
if !constraint.IsPlusBuild(line) {
// Comment with +build but not at beginning.
// Only report early in file.
if check.plusBuildOK && !strings.HasPrefix(line, "// want") {
check.pass.Reportf(pos, "possible malformed +build comment")
}
return
}
if !check.plusBuildOK { // inStar implies !plusBuildOK
check.pass.Reportf(pos, "misplaced +build comment")
check.crossCheck = false
}
if check.plusBuildPos == token.NoPos {
check.plusBuildPos = pos
}
// testing hack: stop at // ERROR
if i := strings.Index(line, " // ERROR "); i >= 0 {
line = line[:i]
}
fields := strings.Fields(line[len("//"):])
// IsPlusBuildConstraint check above implies fields[0] == "+build"
for _, arg := range fields[1:] {
for _, elem := range strings.Split(arg, ",") {
if strings.HasPrefix(elem, "!!") {
return fmt.Errorf("invalid double negative in build constraint: %s", arg)
check.pass.Reportf(pos, "invalid double negative in build constraint: %s", arg)
check.crossCheck = false
continue
}
elem = strings.TrimPrefix(elem, "!")
for _, c := range elem {
if !unicode.IsLetter(c) && !unicode.IsDigit(c) && c != '_' && c != '.' {
return fmt.Errorf("invalid non-alphanumeric build constraint: %s", arg)
check.pass.Reportf(pos, "invalid non-alphanumeric build constraint: %s", arg)
check.crossCheck = false
break
}
}
}
}
return nil
if check.crossCheck {
y, err := constraint.Parse(line)
if err != nil {
// Should never happen - constraint.Parse never rejects a // +build line.
// Also, we just checked the syntax above.
// Even so, report.
check.pass.Reportf(pos, "%v", err)
check.crossCheck = false
return
}
if check.plusBuild == nil {
check.plusBuild = y
} else {
check.plusBuild = &constraint.AndExpr{X: check.plusBuild, Y: y}
}
}
}
var (
nl = []byte("\n")
slashSlash = []byte("//")
)
func (check *checker) finish() {
if !check.crossCheck || check.plusBuildPos == token.NoPos || check.goBuildPos == token.NoPos {
return
}
// Have both //go:build and // +build,
// with no errors found (crossCheck still true).
// Check they match.
var want constraint.Expr
lines, err := constraint.PlusBuildLines(check.goBuild)
if err != nil {
check.pass.Reportf(check.goBuildPos, "%v", err)
return
}
for _, line := range lines {
y, err := constraint.Parse(line)
if err != nil {
// Definitely should not happen, but not the user's fault.
// Do not report.
return
}
if want == nil {
want = y
} else {
want = &constraint.AndExpr{X: want, Y: y}
}
}
if want.String() != check.plusBuild.String() {
check.pass.Reportf(check.plusBuildPos, "+build lines do not match //go:build condition")
return
}
}

View File

@ -0,0 +1,174 @@
// Copyright 2013 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.
// TODO(rsc): Delete this file once Go 1.17 comes out and we can retire Go 1.15 support.
//go:build !go1.16
// +build !go1.16
// Package buildtag defines an Analyzer that checks build tags.
package buildtag
import (
"bytes"
"fmt"
"go/ast"
"go/parser"
"strings"
"unicode"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
)
const Doc = "check that +build tags are well-formed and correctly located"
var Analyzer = &analysis.Analyzer{
Name: "buildtag",
Doc: Doc,
Run: runBuildTag,
}
func runBuildTag(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.
return nil, nil
}
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) {
pastCutoff := false
for _, group := range f.Comments {
// A +build comment is ignored after or adjoining the package declaration.
if group.End()+1 >= f.Package {
pastCutoff = true
}
// "+build" is ignored within or after a /*...*/ comment.
if !strings.HasPrefix(group.List[0].Text, "//") {
pastCutoff = true
continue
}
// Check each line of a //-comment.
for _, c := range group.List {
if !strings.Contains(c.Text, "+build") {
continue
}
if err := checkLine(c.Text, pastCutoff); err != nil {
pass.Reportf(c.Pos(), "%s", err)
}
}
}
}
func checkOtherFile(pass *analysis.Pass, filename string) error {
content, tf, err := analysisutil.ReadFile(pass.Fset, filename)
if err != nil {
return err
}
// We must look at the raw lines, as build tags may appear in non-Go
// files such as assembly files.
lines := bytes.SplitAfter(content, nl)
// Determine cutpoint where +build comments are no longer valid.
// They are valid in leading // comments in the file followed by
// a blank line.
//
// This must be done as a separate pass because of the
// requirement that the comment be followed by a blank line.
var cutoff int
for i, line := range lines {
line = bytes.TrimSpace(line)
if !bytes.HasPrefix(line, slashSlash) {
if len(line) > 0 {
break
}
cutoff = i
}
}
for i, line := range lines {
line = bytes.TrimSpace(line)
if !bytes.HasPrefix(line, slashSlash) {
continue
}
if !bytes.Contains(line, []byte("+build")) {
continue
}
if err := checkLine(string(line), i >= cutoff); err != nil {
pass.Reportf(analysisutil.LineStart(tf, i+1), "%s", err)
continue
}
}
return nil
}
// checkLine checks a line that starts with "//" and contains "+build".
func checkLine(line string, pastCutoff bool) error {
line = strings.TrimPrefix(line, "//")
line = strings.TrimSpace(line)
if strings.HasPrefix(line, "+build") {
fields := strings.Fields(line)
if fields[0] != "+build" {
// Comment is something like +buildasdf not +build.
return fmt.Errorf("possible malformed +build comment")
}
if pastCutoff {
return fmt.Errorf("+build comment must appear before package clause and be followed by a blank line")
}
if err := checkArguments(fields); err != nil {
return err
}
} else {
// Comment with +build but not at beginning.
if !pastCutoff {
return fmt.Errorf("possible malformed +build comment")
}
}
return nil
}
func checkArguments(fields []string) error {
for _, arg := range fields[1:] {
for _, elem := range strings.Split(arg, ",") {
if strings.HasPrefix(elem, "!!") {
return fmt.Errorf("invalid double negative in build constraint: %s", arg)
}
elem = strings.TrimPrefix(elem, "!")
for _, c := range elem {
if !unicode.IsLetter(c) && !unicode.IsDigit(c) && c != '_' && c != '.' {
return fmt.Errorf("invalid non-alphanumeric build constraint: %s", arg)
}
}
}
}
return nil
}
var (
nl = []byte("\n")
slashSlash = []byte("//")
)

View File

@ -5,6 +5,8 @@
package buildtag_test
import (
"runtime"
"strings"
"testing"
"golang.org/x/tools/go/analysis"
@ -13,6 +15,9 @@ import (
)
func Test(t *testing.T) {
if strings.HasPrefix(runtime.Version(), "go1.") && runtime.Version() < "go1.16" {
t.Skipf("skipping on %v", runtime.Version())
}
analyzer := *buildtag.Analyzer
analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
defer func() {

View File

@ -4,15 +4,19 @@
// This file contains tests for the buildtag checker.
// +builder // want `possible malformed \+build comment`
// +build !ignore
// want +1 `possible malformed \+build comment`
// +builder
// +build ignore
// Mention +build // want `possible malformed \+build comment`
// +build nospace // want "build comment must appear before package clause and be followed by a blank line"
// want +1 `misplaced \+build comment`
// +build nospace
//go:build ok
package a
// +build toolate // want "build comment must appear before package clause and be followed by a blank line$"
// want +1 `misplaced \+build comment`
// +build toolate
var _ = 3

View File

@ -6,4 +6,12 @@
package a
// +build toolate // want "build comment must appear before package clause and be followed by a blank line$"
// want +1 `misplaced \+build comment`
// +build toolate
// want +1 `misplaced //go:build comment`
//go:build toolate
var _ = `
// +build notacomment
`

View File

@ -0,0 +1,14 @@
// Copyright 2020 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 +3 `[+]build lines do not match //go:build condition`
//go:build good
// +build bad
package a
var _ = `
// +build notacomment
`

View File

@ -0,0 +1,9 @@
// Copyright 2020 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 !(bad || worse)
// +build !bad
// +build !worse
package a

View File

@ -0,0 +1,11 @@
// Copyright 2020 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 !(bad || worse)
// +build !bad,!worse
package a
//want +1 `misplaced \+build comment`
// +build other

View File

@ -0,0 +1,9 @@
// Copyright 2020 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.
#include "go_asm.h"
// ok because we cannot parse assembly files.
// +build no

View File

@ -0,0 +1,11 @@
// Copyright 2020 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.
// +build ignore
#include "go_asm.h"
// ok because we cannot parse assembly files
// the assembler would complain if we did assemble this file.
//go:build no

View File

@ -0,0 +1,14 @@
// Copyright 2020 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 +3 `\+build lines do not match //go:build condition`
//go:build something
// +build ignore
#include "go_asm.h"
// ok because we cannot parse assembly files
// the assembler would complain if we did assemble this file.
//go:build no