go/packages: handle more errors that 'go list' prints to stderr

This change adds special handling for two more errors that 'go list'
prints to stderr:

1. when trying to construct an ad-hoc package from files in multiple
   directories
2. the "unexpected directory layout" error (#24854)

Change-Id: Icbd14bcc02c2ca9676dc0449e155ff817da038cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190077
Run-TryBot: Dominik Honnef <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>

Author: dominikh

go/packages/golist.go

@@ -847,6 +847,11 @@ func invokeGo(cfg *Config, args ...string) (*bytes.Buffer, error) {
 			return nil, goTooOldError{fmt.Errorf("unsupported version of go: %s: %s", exitErr, stderr)}
 		}
 
+		// Related to #24854
+		if len(stderr.String()) > 0 && strings.Contains(stderr.String(), "unexpected directory layout") {
+			return nil, fmt.Errorf("%s", stderr.String())
+		}
+
 		// This error only appears in stderr. See golang.org/cl/166398 for a fix in go list to show
 		// the error in the Err section of stdout in case -e option is provided.
 		// This fix is provided for backwards compatibility.
@@ -856,6 +861,13 @@ func invokeGo(cfg *Config, args ...string) (*bytes.Buffer, error) {
 			return bytes.NewBufferString(output), nil
 		}
 
+		// Similar to the previous error, but currently lacks a fix in Go.
+		if len(stderr.String()) > 0 && strings.Contains(stderr.String(), "named files must all be in one directory") {
+			output := fmt.Sprintf(`{"ImportPath": "command-line-arguments","Incomplete": true,"Error": {"Pos": "","Err": %q}}`,
+				strings.Trim(stderr.String(), "\n"))
+			return bytes.NewBufferString(output), nil
+		}
+
 		// Backwards compatibility for Go 1.11 because 1.12 and 1.13 put the directory in the ImportPath.
 		// If the package doesn't exist, put the absolute path of the directory into the error message,
 		// as Go 1.13 list does.

go/packages/packages_test.go

@@ -1979,6 +1979,69 @@ func testReturnErrorWhenUsingNonGoFiles(t *testing.T, exporter packagestest.Expo
 	}
 }
 
+func TestReturnErrorWhenUsingGoFilesInMultipleDirectories(t *testing.T) {
+	packagestest.TestAll(t, testReturnErrorWhenUsingGoFilesInMultipleDirectories)
+}
+func testReturnErrorWhenUsingGoFilesInMultipleDirectories(t *testing.T, exporter packagestest.Exporter) {
+	exported := packagestest.Export(t, exporter, []packagestest.Module{{
+		Name: "golang.org/gopatha",
+		Files: map[string]interface{}{
+			"a/a.go": `package a`,
+			"b/b.go": `package b`,
+		}}})
+	defer exported.Cleanup()
+	want := "named files must all be in one directory"
+	pkgs, err := packages.Load(exported.Config, exported.File("golang.org/gopatha", "a/a.go"), exported.File("golang.org/gopatha", "b/b.go"))
+	if err != nil {
+		// Check if the error returned is the one we expected.
+		if !strings.Contains(err.Error(), want) {
+			t.Fatalf("want error message: %s, got: %s", want, err.Error())
+		}
+		return
+	}
+	if len(pkgs) != 1 || pkgs[0].PkgPath != "command-line-arguments" {
+		t.Fatalf("packages.Load: want [command-line-arguments], got %v", pkgs)
+	}
+	if len(pkgs[0].Errors) != 1 {
+		t.Fatalf("result of Load: want package with one error, got: %+v", pkgs[0])
+	}
+	got := pkgs[0].Errors[0].Error()
+	if !strings.Contains(got, want) {
+		t.Fatalf("want error message: %s, got: %s", want, got)
+	}
+}
+
+func TestReturnErrorForUnexpectedDirectoryLayout(t *testing.T) {
+	packagestest.TestAll(t, testReturnErrorForUnexpectedDirectoryLayout)
+}
+func testReturnErrorForUnexpectedDirectoryLayout(t *testing.T, exporter packagestest.Exporter) {
+	exported := packagestest.Export(t, exporter, []packagestest.Module{{
+		Name: "golang.org/gopatha",
+		Files: map[string]interface{}{
+			"a/testdata/a.go": `package a; import _ "b"`,
+			"a/vendor/b/b.go": `package b; import _ "fmt"`,
+		}}})
+	defer exported.Cleanup()
+	want := "unexpected directory layout"
+	// triggering this error requires a relative package path
+	exported.Config.Dir = filepath.Dir(exported.File("golang.org/gopatha", "a/testdata/a.go"))
+	pkgs, err := packages.Load(exported.Config, ".")
+
+	// This error doesn't seem to occur in module mode; so only
+	// complain if we get zero packages while also getting no error.
+	if err == nil {
+		if len(pkgs) == 0 {
+			// TODO(dh): we'll need to expand on the error check if/when Go stops emitting this error
+			t.Fatalf("want error, got nil")
+		}
+		return
+	}
+	// Check if the error returned is the one we expected.
+	if !strings.Contains(err.Error(), want) {
+		t.Fatalf("want error message: %s, got: %s", want, err.Error())
+	}
+}
+
 func TestMissingDependency(t *testing.T) { packagestest.TestAll(t, testMissingDependency) }
 func testMissingDependency(t *testing.T, exporter packagestest.Exporter) {
 	exported := packagestest.Export(t, exporter, []packagestest.Module{{