Commit 0c614671 authored by Russ Cox's avatar Russ Cox

cmd/go, go/build: implement import comment checking

See golang.org/s/go14customimport for design.

Added case to deps_test to allow go/build to import regexp.
Not a new dependency, because go/build already imports go/doc
which imports regexp.

Fixes #7453.

LGTM=r
R=r, josharian
CC=golang-codereviews
https://golang.org/cl/124940043
parent f497885c
...@@ -261,11 +261,14 @@ func loadImport(path string, srcDir string, stk *importStack, importPos []token. ...@@ -261,11 +261,14 @@ func loadImport(path string, srcDir string, stk *importStack, importPos []token.
// //
// TODO: After Go 1, decide when to pass build.AllowBinary here. // TODO: After Go 1, decide when to pass build.AllowBinary here.
// See issue 3268 for mistakes to avoid. // See issue 3268 for mistakes to avoid.
bp, err := buildContext.Import(path, srcDir, 0) bp, err := buildContext.Import(path, srcDir, build.ImportComment)
bp.ImportPath = importPath bp.ImportPath = importPath
if gobin != "" { if gobin != "" {
bp.BinDir = gobin bp.BinDir = gobin
} }
if err == nil && !isLocal && bp.ImportComment != "" && bp.ImportComment != path {
err = fmt.Errorf("directory %s contains package %q", bp.Dir, bp.ImportComment)
}
p.load(stk, bp, err) p.load(stk, bp, err)
if p.Error != nil && len(importPos) > 0 { if p.Error != nil && len(importPos) > 0 {
pos := importPos[0] pos := importPos[0]
......
...@@ -121,6 +121,42 @@ if ! ./testgo build -v ./testdata/testinternal2; then ...@@ -121,6 +121,42 @@ if ! ./testgo build -v ./testdata/testinternal2; then
ok=false ok=false
fi fi
export GOPATH=$(pwd)/testdata/importcom
TEST 'import comment - match'
if ! ./testgo build ./testdata/importcom/works.go; then
echo 'go build ./testdata/importcom/works.go failed'
ok=false
fi
TEST 'import comment - mismatch'
if ./testgo build ./testdata/importcom/wrongplace.go 2>testdata/err; then
echo 'go build ./testdata/importcom/wrongplace.go suceeded'
ok=false
elif ! grep 'wrongplace contains package "my/x"' testdata/err >/dev/null; then
echo 'go build did not mention incorrect import:'
cat testdata/err
ok=false
fi
TEST 'import comment - syntax error'
if ./testgo build ./testdata/importcom/bad.go 2>testdata/err; then
echo 'go build ./testdata/importcom/bad.go suceeded'
ok=false
elif ! grep 'cannot parse import comment' testdata/err >/dev/null; then
echo 'go build did not mention syntax error:'
cat testdata/err
ok=false
fi
TEST 'import comment - conflict'
if ./testgo build ./testdata/importcom/conflict.go 2>testdata/err; then
echo 'go build ./testdata/importcom/conflict.go suceeded'
ok=false
elif ! grep 'found import comments' testdata/err >/dev/null; then
echo 'go build did not mention comment conflict:'
cat testdata/err
ok=false
fi
rm -f ./testdata/err
unset GOPATH
TEST error message for syntax error in test go file says FAIL TEST error message for syntax error in test go file says FAIL
export GOPATH=$(pwd)/testdata export GOPATH=$(pwd)/testdata
if ./testgo test syntaxerror 2>testdata/err; then if ./testgo test syntaxerror 2>testdata/err; then
......
package conflict /* import "b" */
package x // important! not an import comment
package p
import _ "works/x"
...@@ -23,6 +23,7 @@ import ( ...@@ -23,6 +23,7 @@ import (
"strconv" "strconv"
"strings" "strings"
"unicode" "unicode"
"unicode/utf8"
) )
// A Context specifies the supporting context for a build. // A Context specifies the supporting context for a build.
...@@ -337,22 +338,29 @@ const ( ...@@ -337,22 +338,29 @@ const (
// If AllowBinary is set, Import can be satisfied by a compiled // If AllowBinary is set, Import can be satisfied by a compiled
// package object without corresponding sources. // package object without corresponding sources.
AllowBinary AllowBinary
// If ImportComment is set, parse import comments on package statements.
// Import returns an error if it finds a comment it cannot understand
// or finds conflicting comments in multiple source files.
// See golang.org/s/go14customimport for more information.
ImportComment
) )
// A Package describes the Go package found in a directory. // A Package describes the Go package found in a directory.
type Package struct { type Package struct {
Dir string // directory containing package sources Dir string // directory containing package sources
Name string // package name Name string // package name
Doc string // documentation synopsis ImportComment string // path in import comment on package statement
ImportPath string // import path of package ("" if unknown) Doc string // documentation synopsis
Root string // root of Go tree where this package lives ImportPath string // import path of package ("" if unknown)
SrcRoot string // package source root directory ("" if unknown) Root string // root of Go tree where this package lives
PkgRoot string // package install root directory ("" if unknown) SrcRoot string // package source root directory ("" if unknown)
BinDir string // command install directory ("" if unknown) PkgRoot string // package install root directory ("" if unknown)
Goroot bool // package found in Go root BinDir string // command install directory ("" if unknown)
PkgObj string // installed .a file Goroot bool // package found in Go root
AllTags []string // tags that can influence file selection in this directory PkgObj string // installed .a file
ConflictDir string // this directory shadows Dir in $GOPATH AllTags []string // tags that can influence file selection in this directory
ConflictDir string // this directory shadows Dir in $GOPATH
// Source files // Source files
GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles)
...@@ -597,7 +605,7 @@ Found: ...@@ -597,7 +605,7 @@ Found:
} }
var Sfiles []string // files with ".S" (capital S) var Sfiles []string // files with ".S" (capital S)
var firstFile string var firstFile, firstCommentFile string
imported := make(map[string][]token.Position) imported := make(map[string][]token.Position)
testImported := make(map[string][]token.Position) testImported := make(map[string][]token.Position)
xTestImported := make(map[string][]token.Position) xTestImported := make(map[string][]token.Position)
...@@ -684,6 +692,22 @@ Found: ...@@ -684,6 +692,22 @@ Found:
p.Doc = doc.Synopsis(pf.Doc.Text()) p.Doc = doc.Synopsis(pf.Doc.Text())
} }
if mode&ImportComment != 0 {
qcom, line := findImportComment(data)
if line != 0 {
com, err := strconv.Unquote(qcom)
if err != nil {
return p, fmt.Errorf("%s:%d: cannot parse import comment", filename, line)
}
if p.ImportComment == "" {
p.ImportComment = com
firstCommentFile = name
} else if p.ImportComment != com {
return p, fmt.Errorf("found import comments %q (%s) and %q (%s) in %s", p.ImportComment, firstCommentFile, com, name, p.Dir)
}
}
}
// Record imports and information about cgo. // Record imports and information about cgo.
isCgo := false isCgo := false
for _, decl := range pf.Decls { for _, decl := range pf.Decls {
...@@ -764,6 +788,117 @@ Found: ...@@ -764,6 +788,117 @@ Found:
return p, pkgerr return p, pkgerr
} }
func findImportComment(data []byte) (s string, line int) {
// expect keyword package
word, data := parseWord(data)
if string(word) != "package" {
return "", 0
}
// expect package name
_, data = parseWord(data)
// now ready for import comment, a // or /* */ comment
// beginning and ending on the current line.
for len(data) > 0 && (data[0] == ' ' || data[0] == '\t' || data[0] == '\r') {
data = data[1:]
}
var comment []byte
switch {
case bytes.HasPrefix(data, slashSlash):
i := bytes.Index(data, newline)
if i < 0 {
i = len(data)
}
comment = data[2:i]
case bytes.HasPrefix(data, slashStar):
data = data[2:]
i := bytes.Index(data, starSlash)
if i < 0 {
// malformed comment
return "", 0
}
comment = data[:i]
if bytes.Contains(comment, newline) {
return "", 0
}
}
comment = bytes.TrimSpace(comment)
// split comment into `import`, `"pkg"`
word, arg := parseWord(comment)
if string(word) != "import" {
return "", 0
}
line = 1 + bytes.Count(data[:cap(data)-cap(arg)], newline)
return strings.TrimSpace(string(arg)), line
}
var (
slashSlash = []byte("//")
slashStar = []byte("/*")
starSlash = []byte("*/")
newline = []byte("\n")
)
// skipSpaceOrComment returns data with any leading spaces or comments removed.
func skipSpaceOrComment(data []byte) []byte {
for len(data) > 0 {
switch data[0] {
case ' ', '\t', '\r', '\n':
data = data[1:]
continue
case '/':
if bytes.HasPrefix(data, slashSlash) {
i := bytes.Index(data, newline)
if i < 0 {
return nil
}
data = data[i+1:]
continue
}
if bytes.HasPrefix(data, slashStar) {
data = data[2:]
i := bytes.Index(data, starSlash)
if i < 0 {
return nil
}
data = data[i+2:]
continue
}
}
break
}
return data
}
// parseWord skips any leading spaces or comments in data
// and then parses the beginning of data as an identifier or keyword,
// returning that word and what remains after the word.
func parseWord(data []byte) (word, rest []byte) {
data = skipSpaceOrComment(data)
// Parse past leading word characters.
rest = data
for {
r, size := utf8.DecodeRune(rest)
if unicode.IsLetter(r) || '0' <= r && r <= '9' || r == '_' {
rest = rest[size:]
continue
}
break
}
word = data[:len(data)-len(rest)]
if len(word) == 0 {
return nil, nil
}
return word, rest
}
// MatchFile reports whether the file with the given name in the given directory // MatchFile reports whether the file with the given name in the given directory
// matches the context and would be included in a Package created by ImportDir // matches the context and would be included in a Package created by ImportDir
// of that directory. // of that directory.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment