Canonical path extraction: tests and error checking

Added a check and an error type for importRoot to be a prefix of
canonical import path, tests for the new code paths.

Fixed one unrelated bug along the way:
    Sprintf("%q", strings.Join(strs, "\", \""))
doesn't work as intended, since the %q verb will escape the inner
doublequotes.

internal/gps/_testdata/src/canon_confl/a.go

@@ -0,0 +1,9 @@
+// Copyright 2017 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 canonical // import "vanity1"
+
+var (
+	A = "A"
+)

internal/gps/_testdata/src/canon_confl/b.go

@@ -0,0 +1,9 @@
+// Copyright 2017 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 canonical // import "vanity2"
+
+var (
+	B = "B"
+)

internal/gps/_testdata/src/canonical/main.go

@@ -0,0 +1,9 @@
+// Copyright 2017 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 pkg // import "canonical"
+
+var (
+	A = "A"
+)

internal/gps/_testdata/src/canonical/sub/sub.go

@@ -0,0 +1,5 @@
+// Copyright 2017 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 sub // import "canonical/subpackage"

internal/gps/pkgtree/pkgtree.go

@@ -154,6 +154,13 @@ func ListPackages(fileRoot, importRoot string) (PackageTree, error) {
 			}
 		}
 
+		if pkg.CommentPath != "" && !strings.HasPrefix(pkg.CommentPath, importRoot) {
+			return &NonCanonicalImportRoot{
+				ImportRoot: importRoot,
+				Canonical:  pkg.CommentPath,
+			}
+		}
+
 		// This area has some...fuzzy rules, but check all the imports for
 		// local/relative/dot-ness, and record an error for the package if we
 		// see any.
@@ -347,15 +354,36 @@ func findImportComment(pkgName *ast.Ident, c *ast.CommentGroup) string {
 	return string(bytes.Trim(quotedPath, `"`))
 }
 
-// ConflictingImportComments conflict
+// ConflictingImportComments indicates that the package declares more than one
+// different canonical paths.
 type ConflictingImportComments struct {
 	ImportPath     string
 	ImportComments []string
 }
 
 func (e *ConflictingImportComments) Error() string {
-	return fmt.Sprintf("import path %s had conflicting import comments: %q",
-		e.ImportPath, strings.Join(e.ImportComments, "\", \""))
+	return fmt.Sprintf("import path %s had conflicting import comments: %s",
+		e.ImportPath, quotedPaths(e.ImportComments))
+}
+
+// NonCanonicalImportRoot reports the situation when the dependee imports a
+// package via something else that the package's declared canonical path.
+type NonCanonicalImportRoot struct {
+	ImportRoot string
+	Canonical  string
+}
+
+func (e *NonCanonicalImportRoot) Error() string {
+	return fmt.Sprintf("importing via path %q, but package insists on a canonical path %q",
+		e.ImportRoot, e.Canonical)
+}
+
+func quotedPaths(ps []string) string {
+	var quoted []string
+	for _, p := range ps {
+		quoted = append(quoted, fmt.Sprintf("%q", p))
+	}
+	return strings.Join(quoted, ", ")
 }
 
 // LocalImportsError indicates that a package contains at least one relative
@@ -376,7 +404,7 @@ func (e *LocalImportsError) Error() string {
 	case 1:
 		return fmt.Sprintf("import path %s had a local import: %q", e.ImportPath, e.LocalImports[0])
 	default:
-		return fmt.Sprintf("import path %s had local imports: %q", e.ImportPath, strings.Join(e.LocalImports, "\", \""))
+		return fmt.Sprintf("import path %s had local imports: %s", e.ImportPath, quotedPaths(e.LocalImports))
 	}
 }
 

internal/gps/pkgtree/pkgtree_test.go

@@ -1292,6 +1292,52 @@ func TestListPackages(t *testing.T) {
 				},
 			},
 		},
+		"canonical": {
+			fileRoot:   j("canonical"),
+			importRoot: "canonical",
+			out: PackageTree{
+				ImportRoot: "canonical",
+				Packages: map[string]PackageOrErr{
+					"canonical": {
+						P: Package{
+							ImportPath:  "canonical",
+							CommentPath: "canonical",
+							Name:        "pkg",
+							Imports:     []string{},
+						},
+					},
+					"canonical/sub": {
+						P: Package{
+							ImportPath:  "canonical/sub",
+							CommentPath: "canonical/subpackage",
+							Name:        "sub",
+							Imports:     []string{},
+						},
+					},
+				},
+			},
+		},
+		"conflicting canonical comments": {
+			fileRoot:   j("canon_confl"),
+			importRoot: "canon_confl",
+			out:        PackageTree{},
+			err: &ConflictingImportComments{
+				ImportPath: "canon_confl",
+				ImportComments: []string{
+					"vanity1",
+					"vanity2",
+				},
+			},
+		},
+		"non-canonical": {
+			fileRoot:   j("canonical"),
+			importRoot: "noncanonical",
+			out:        PackageTree{},
+			err: &NonCanonicalImportRoot{
+				ImportRoot: "noncanonical",
+				Canonical:  "canonical",
+			},
+		},
 	}
 
 	for name, fix := range table {