]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go, go/build: implement import comment checking
authorRuss Cox <rsc@golang.org>
Tue, 12 Aug 2014 21:41:03 +0000 (17:41 -0400)
committerRuss Cox <rsc@golang.org>
Tue, 12 Aug 2014 21:41:03 +0000 (17:41 -0400)
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

13 files changed:
src/cmd/go/pkg.go
src/cmd/go/test.bash
src/cmd/go/testdata/importcom/bad.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/conflict.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/src/bad/bad.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/src/conflict/a.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/src/conflict/b.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/src/works/x/x.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/src/works/x/x1.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/src/wrongplace/x.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/works.go [new file with mode: 0644]
src/cmd/go/testdata/importcom/wrongplace.go [new file with mode: 0644]
src/pkg/go/build/build.go

index be691a6bc9afd79a6b339291642f4ca5639cee1e..eb8c451783f7a18dc0378301277ef0947993ab54 100644 (file)
@@ -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.
        // 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
        if 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)
        if p.Error != nil && len(importPos) > 0 {
                pos := importPos[0]
index 411ef1863f9b3bf5a00699556a05bda745472a00..93a7c67c18506dbf9c6a4a307910aad07d295866 100755 (executable)
@@ -121,6 +121,42 @@ if ! ./testgo build -v ./testdata/testinternal2; then
        ok=false
 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
 export GOPATH=$(pwd)/testdata
 if ./testgo test syntaxerror 2>testdata/err; then
diff --git a/src/cmd/go/testdata/importcom/bad.go b/src/cmd/go/testdata/importcom/bad.go
new file mode 100644 (file)
index 0000000..e104c2e
--- /dev/null
@@ -0,0 +1,3 @@
+package p
+
+import "bad"
diff --git a/src/cmd/go/testdata/importcom/conflict.go b/src/cmd/go/testdata/importcom/conflict.go
new file mode 100644 (file)
index 0000000..995556c
--- /dev/null
@@ -0,0 +1,3 @@
+package p
+
+import "conflict"
diff --git a/src/cmd/go/testdata/importcom/src/bad/bad.go b/src/cmd/go/testdata/importcom/src/bad/bad.go
new file mode 100644 (file)
index 0000000..bc51fd3
--- /dev/null
@@ -0,0 +1 @@
+package bad // import
diff --git a/src/cmd/go/testdata/importcom/src/conflict/a.go b/src/cmd/go/testdata/importcom/src/conflict/a.go
new file mode 100644 (file)
index 0000000..2d67703
--- /dev/null
@@ -0,0 +1 @@
+package conflict // import "a"
diff --git a/src/cmd/go/testdata/importcom/src/conflict/b.go b/src/cmd/go/testdata/importcom/src/conflict/b.go
new file mode 100644 (file)
index 0000000..8fcfb3c
--- /dev/null
@@ -0,0 +1 @@
+package conflict /* import "b" */
diff --git a/src/cmd/go/testdata/importcom/src/works/x/x.go b/src/cmd/go/testdata/importcom/src/works/x/x.go
new file mode 100644 (file)
index 0000000..044c6ec
--- /dev/null
@@ -0,0 +1 @@
+package x // import "works/x"
diff --git a/src/cmd/go/testdata/importcom/src/works/x/x1.go b/src/cmd/go/testdata/importcom/src/works/x/x1.go
new file mode 100644 (file)
index 0000000..2449b29
--- /dev/null
@@ -0,0 +1 @@
+package x // important! not an import comment
diff --git a/src/cmd/go/testdata/importcom/src/wrongplace/x.go b/src/cmd/go/testdata/importcom/src/wrongplace/x.go
new file mode 100644 (file)
index 0000000..b89849d
--- /dev/null
@@ -0,0 +1 @@
+package x // import "my/x"
diff --git a/src/cmd/go/testdata/importcom/works.go b/src/cmd/go/testdata/importcom/works.go
new file mode 100644 (file)
index 0000000..31b55d0
--- /dev/null
@@ -0,0 +1,3 @@
+package p
+
+import _ "works/x"
diff --git a/src/cmd/go/testdata/importcom/wrongplace.go b/src/cmd/go/testdata/importcom/wrongplace.go
new file mode 100644 (file)
index 0000000..e2535e0
--- /dev/null
@@ -0,0 +1,3 @@
+package p
+
+import "wrongplace"
index 6db0275032d3d86d15cdea32eec032c7605959cc..1a133041e8f7b93da240778fc81fec4b319b18f5 100644 (file)
@@ -23,6 +23,7 @@ import (
        "strconv"
        "strings"
        "unicode"
+       "unicode/utf8"
 )
 
 // A Context specifies the supporting context for a build.
@@ -337,22 +338,29 @@ const (
        // If AllowBinary is set, Import can be satisfied by a compiled
        // package object without corresponding sources.
        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.
 type Package struct {
-       Dir         string   // directory containing package sources
-       Name        string   // package name
-       Doc         string   // documentation synopsis
-       ImportPath  string   // import path of package ("" if unknown)
-       Root        string   // root of Go tree where this package lives
-       SrcRoot     string   // package source root directory ("" if unknown)
-       PkgRoot     string   // package install root directory ("" if unknown)
-       BinDir      string   // command install directory ("" if unknown)
-       Goroot      bool     // package found in Go root
-       PkgObj      string   // installed .a file
-       AllTags     []string // tags that can influence file selection in this directory
-       ConflictDir string   // this directory shadows Dir in $GOPATH
+       Dir           string   // directory containing package sources
+       Name          string   // package name
+       ImportComment string   // path in import comment on package statement
+       Doc           string   // documentation synopsis
+       ImportPath    string   // import path of package ("" if unknown)
+       Root          string   // root of Go tree where this package lives
+       SrcRoot       string   // package source root directory ("" if unknown)
+       PkgRoot       string   // package install root directory ("" if unknown)
+       BinDir        string   // command install directory ("" if unknown)
+       Goroot        bool     // package found in Go root
+       PkgObj        string   // installed .a file
+       AllTags       []string // tags that can influence file selection in this directory
+       ConflictDir   string   // this directory shadows Dir in $GOPATH
 
        // Source files
        GoFiles        []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles)
@@ -597,7 +605,7 @@ Found:
        }
 
        var Sfiles []string // files with ".S" (capital S)
-       var firstFile string
+       var firstFile, firstCommentFile string
        imported := make(map[string][]token.Position)
        testImported := make(map[string][]token.Position)
        xTestImported := make(map[string][]token.Position)
@@ -684,6 +692,22 @@ Found:
                        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.
                isCgo := false
                for _, decl := range pf.Decls {
@@ -764,6 +788,117 @@ Found:
        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
 // matches the context and would be included in a Package created by ImportDir
 // of that directory.