cmd/go: implement 'internal' convention

See golang.org/s/go14internal for design.

LGTM=r
R=r, adg
CC=golang-codereviews
https://codereview.appspot.com/120600043

5 files changed, 83 insertions, 5 deletions

diff --git a/src/cmd/go/pkg.go b/src/cmd/go/pkg.go
index d45df265b..d0dbefed0 100644
--- a/src/cmd/go/pkg.go
+++ b/src/cmd/go/pkg.go
@@ -244,6 +244,9 @@ func loadImport(path string, srcDir string, stk *importStack, importPos []token.
 		importPath = dirToImportPath(filepath.Join(srcDir, path))
 	}
 	if p := packageCache[importPath]; p != nil {
+		if perr := disallowInternal(srcDir, p, stk); perr != p {
+			return perr
+		}
 		return reusePackage(p, stk)
 	}
 
@@ -270,6 +273,10 @@ func loadImport(path string, srcDir string, stk *importStack, importPos []token.
 		p.Error.Pos = pos.String()
 	}
 
+	if perr := disallowInternal(srcDir, p, stk); perr != p {
+		return perr
+	}
+
 	return p
 }
 
@@ -298,6 +305,54 @@ func reusePackage(p *Package, stk *importStack) *Package {
 	return p
 }
 
+// disallowInternal checks that srcDir is allowed to import p.
+// If the import is allowed, disallowInternal returns the original package p.
+// If not, it returns a new package containing just an appropriate error.
+func disallowInternal(srcDir string, p *Package, stk *importStack) *Package {
+	// golang.org/s/go14internal:
+	// An import of a path containing the element “internal”
+	// is disallowed if the importing code is outside the tree
+	// rooted at the parent of the “internal” directory.
+	//
+	// ... For Go 1.4, we will implement the rule first for $GOROOT, but not $GOPATH.
+
+	// Only applies to $GOROOT.
+	if !p.Standard {
+		return p
+	}
+
+	// The stack includes p.ImportPath.
+	// If that's the only thing on the stack, we started
+	// with a name given on the command line, not an
+	// import. Anything listed on the command line is fine.
+	if len(*stk) == 1 {
+		return p
+	}
+
+	// Check for "internal" element: four cases depending on begin of string and/or end of string.
+	if p.ImportPath != "internal" &&
+		!strings.HasPrefix(p.ImportPath, "internal/") &&
+		!strings.HasSuffix(p.ImportPath, "/internal") &&
+		!strings.Contains(p.ImportPath, "/internal/") {
+		return p
+	}
+
+	// Internal is present. Check directory.
+	parent := filepath.Dir(p.Dir)
+	if hasPathPrefix(srcDir, parent) {
+		return p
+	}
+
+	// Internal is present, and srcDir is outside parent's tree. Not allowed.
+	perr := *p
+	perr.Error = &PackageError{
+		ImportStack: stk.copy(),
+		Err:         "use of internal package not allowed",
+	}
+	perr.Incomplete = true
+	return &perr
+}
+
 type targetDir int
 
 const (
@@ -482,7 +537,7 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package
 
 	// Build list of imported packages and full dependency list.
 	imports := make([]*Package, 0, len(p.Imports))
-	deps := make(map[string]bool)
+	deps := make(map[string]*Package)
 	for i, path := range importPaths {
 		if path == "C" {
 			continue
@@ -502,10 +557,10 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package
 			path = p1.ImportPath
 			importPaths[i] = path
 		}
-		deps[path] = true
+		deps[path] = p1
 		imports = append(imports, p1)
-		for _, dep := range p1.Deps {
-			deps[dep] = true
+		for _, dep := range p1.deps {
+			deps[dep.ImportPath] = dep
 		}
 		if p1.Incomplete {
 			p.Incomplete = true
@@ -519,7 +574,7 @@ func (p *Package) load(stk *importStack, bp *build.Package, err error) *Package
 	}
 	sort.Strings(p.Deps)
 	for _, dep := range p.Deps {
-		p1 := packageCache[dep]
+		p1 := deps[dep]
 		if p1 == nil {
 			panic("impossible: missing entry in package cache for " + dep + " imported by " + p.ImportPath)
 		}
diff --git a/src/cmd/go/test.bash b/src/cmd/go/test.bash
index c62f62940..e5ba12b1d 100755
--- a/src/cmd/go/test.bash
+++ b/src/cmd/go/test.bash
@@ -105,6 +105,22 @@ cp -R testdata/local "testdata/$bad"
 testlocal "$bad" 'with bad characters in path'
 rm -rf "testdata/$bad"
 
+TEST 'internal packages in $GOROOT are respected'
+if ./testgo build -v ./testdata/testinternal >testdata/std.out 2>&1; then
+	echo "go build ./testdata/testinternal succeeded incorrectly"
+	ok=false
+elif ! grep 'use of internal package not allowed' testdata/std.out >/dev/null; then
+	echo "wrong error message for testdata/testinternal"
+	cat std.out
+	ok=false
+fi
+
+TEST 'internal packages outside $GOROOT are not respected'
+if ! ./testgo build -v ./testdata/testinternal2; then
+	echo "go build ./testdata/testinternal2 failed"
+	ok=false
+fi
+
 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/testinternal/p.go b/src/cmd/go/testdata/testinternal/p.go
new file mode 100644
index 000000000..e3558a53b
--- /dev/null
+++ b/src/cmd/go/testdata/testinternal/p.go
@@ -0,0 +1,3 @@
+package p
+
+import _ "net/http/internal"
diff --git a/src/cmd/go/testdata/testinternal2/p.go b/src/cmd/go/testdata/testinternal2/p.go
new file mode 100644
index 000000000..c594f5c5e
--- /dev/null
+++ b/src/cmd/go/testdata/testinternal2/p.go
@@ -0,0 +1,3 @@
+package p
+
+import _ "./x/y/z/internal/w"
diff --git a/src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go b/src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go
new file mode 100644
index 000000000..a796c0b5f
--- /dev/null
+++ b/src/cmd/go/testdata/testinternal2/x/y/z/internal/w/w.go
@@ -0,0 +1 @@
+package w