go/packages: prefer import map to identity imports

For any identity imports to be added to the import map of a package,
make sure these are not already in there.  If the package imports "C",
be sure not to overwrite the import map entries for "runtime/cgo", "unsafe",
and "syscall".

Test the import map when test variants are loaded. This tests the fix
in 'go list' made by https://golang.org/cl/128836

Fixes golang/go#26847

Change-Id: I05dff4d3a75fab03f333f6d88128de6a6bf169e6
Reviewed-on: https://go-review.googlesource.com/128876
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>

go/packages/golist.go

@@ -230,13 +230,7 @@ func golistDriverCurrent(cfg *Config, words ...string) (*driverResponse, error)
 			delete(ids, id)
 		}
 		for id := range ids {
-			// Go issue 26136: go list omits imports in cgo-generated files.
 			if id == "C" {
-				imports["unsafe"] = &Package{ID: "unsafe"}
-				imports["syscall"] = &Package{ID: "syscall"}
-				if pkgpath != "runtime/cgo" {
-					imports["runtime/cgo"] = &Package{ID: "runtime/cgo"}
-				}
 				continue
 			}
 

go/packages/golist_fallback.go

@@ -54,10 +54,10 @@ func golistDriverFallback(cfg *Config, words ...string) (*driverResponse, error)
 			for _, id := range importlist {
 
 				if id == "C" {
-					importMap["unsafe"] = &Package{ID: "unsafe"}
-					importMap["syscall"] = &Package{ID: "syscall"}
-					if pkgpath != "runtime/cgo" {
-						importMap["runtime/cgo"] = &Package{ID: "runtime/cgo"}
+					for _, path := range []string{"unsafe", "syscall", "runtime/cgo"} {
+						if pkgpath != path && importMap[path] == nil {
+							importMap[path] = &Package{ID: path}
+						}
 					}
 					continue
 				}

go/packages/packages_test.go

@@ -16,6 +16,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"runtime"
 	"sort"
 	"strings"
 	"sync"
@@ -279,6 +280,98 @@ func TestLoadImportsGraph(t *testing.T) {
 	}
 }
 
+func TestLoadImportsTestVariants(t *testing.T) {
+	if usesOldGolist {
+		t.Skip("not yet supported in pre-Go 1.10.4 golist fallback implementation")
+	}
+
+	tmp, cleanup := makeTree(t, map[string]string{
+		"src/a/a.go":       `package a; import _ "b"`,
+		"src/b/b.go":       `package b`,
+		"src/b/b_test.go":  `package b`,
+		"src/b/bx_test.go": `package b_test; import _ "a"`,
+	})
+	defer cleanup()
+
+	cfg := &packages.Config{
+		Mode:  packages.LoadImports,
+		Env:   append(os.Environ(), "GOPATH="+tmp),
+		Tests: true,
+	}
+	initial, err := packages.Load(cfg, "a", "b")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Check graph topology.
+	graph, _ := importGraph(initial)
+	wantGraph := `
+* a
+  a [b.test]
+* b
+* b [b.test]
+* b.test
+* b_test [b.test]
+  a -> b
+  a [b.test] -> b [b.test]
+  b.test -> b [b.test]
+  b.test -> b_test [b.test]
+  b.test -> os (pruned)
+  b.test -> testing (pruned)
+  b.test -> testing/internal/testdeps (pruned)
+  b_test [b.test] -> a [b.test]
+`[1:]
+
+	if graph != wantGraph {
+		t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph)
+	}
+}
+
+func TestLoadImportsC(t *testing.T) {
+	// This test checks that when a package depends on the
+	// test variant of "syscall", "unsafe", or "runtime/cgo", that dependency
+	// is not removed when those packages are added when it imports "C".
+	//
+	// For this test to work, the external test of syscall must have a dependency
+	// on net, and net must import "syscall" and "C".
+	if runtime.GOOS == "windows" {
+		t.Skipf("skipping on windows; packages on windows do not satisfy conditions for test.")
+	}
+
+	if usesOldGolist {
+		t.Skip("not yet supported in pre-Go 1.10.4 golist fallback implementation")
+	}
+
+	cfg := &packages.Config{
+		Mode:  packages.LoadImports,
+		Tests: true,
+	}
+	initial, err := packages.Load(cfg, "syscall", "net")
+	if err != nil {
+		t.Fatalf("failed to load imports: %v", err)
+	}
+
+	_, all := importGraph(initial)
+
+	for _, test := range []struct {
+		pattern    string
+		wantImport string // an import to check for
+	}{
+		{"net", "syscall:syscall"},
+		{"net [syscall.test]", "syscall:syscall [syscall.test]"},
+		{"syscall_test [syscall.test]", "net:net [syscall.test]"},
+	} {
+		// Test the import paths.
+		pkg := all[test.pattern]
+		if pkg == nil {
+			t.Errorf("package %q not loaded", test.pattern)
+		}
+		if imports := strings.Join(imports(pkg), " "); !strings.Contains(imports, test.wantImport) {
+			t.Errorf("package %q: got \n%s, \nwant to have %s", test.pattern, imports, test.wantImport)
+		}
+	}
+}
+
 func TestVendorImports(t *testing.T) {
 	tmp, cleanup := makeTree(t, map[string]string{
 		"src/a/a.go":          `package a; import _ "b"; import _ "c";`,