avoid panic when a package's imports cannot be loaded

I mistakenly understood that, when the DepsErrors field has errors,
the Error field would contain an error as well.
That is not always the case; for example,
the imports_missing package in the added test script
had DepsErrors set but Error empty, causing a nil dereference panic.

Make the code more robust, and report both kinds of load errors.

Fixes #694.

shared.go

@@ -147,7 +147,7 @@ type listedPackage struct {
 	Imports         []string
 
 	Incomplete bool
-	// These two exist only to fill Incomplete.
+	// These two exist to report package loading errors to the user.
 	Error      *packageError
 	DepsErrors []*packageError
 
@@ -256,30 +256,38 @@ func appendListedPackages(packages []string, mainBuild bool) error {
 
 		// Sometimes cmd/go sets Error without setting Incomplete per the docs.
 		// TODO: remove the workaround once https://go.dev/issue/57724 is fixed.
-		if (pkg.Error != nil || pkg.DepsErrors != nil) && !pkg.Incomplete {
+		if pkg.Error != nil || pkg.DepsErrors != nil {
 			pkg.Incomplete = true
 		}
 
-		if pkg.Incomplete {
+		if perr := pkg.Error; perr != nil {
 			switch {
 			// All errors in non-std packages are fatal,
 			// but only some errors in std packages are.
 			case strings.Contains(pkg.ImportPath, "."):
 				fallthrough
 			default:
-				pkgErrors = append(pkgErrors, pkg.Error.Err)
+				// Error messages sometimes include a trailing newline.
-				continue
+				pkgErrors = append(pkgErrors, strings.TrimSpace(perr.Err))
 
 			// Some packages in runtimeLinknamed are OS-specific,
 			// like crypto/internal/boring/fipstls, so "no Go files"
 			// for the current OS can be ignored safely as an error.
-			case pkg.Standard && strings.Contains(pkg.Error.Err, "build constraints exclude all Go files"):
+			case pkg.Standard && strings.Contains(perr.Err, "build constraints exclude all Go files"):
 			// Some packages in runtimeLinknamed are recent,
 			// like "arena", so older Go versions that we support
 			// do not yet have them and that's OK.
 			// Note that pkg.Standard is false for them.
-			case strings.Contains(pkg.Error.Err, "is not in GOROOT"):
+			case strings.Contains(perr.Err, "is not in GOROOT"):
-			case strings.Contains(pkg.Error.Err, "cannot find package"):
+			case strings.Contains(perr.Err, "cannot find package"):
+			}
+		}
+		if len(pkg.DepsErrors) > 0 {
+			// DepsErrors appears to not include the "# pkgpath" line.
+			pkgErrors = append(pkgErrors, "# "+pkg.ImportPath)
+			for _, derr := range pkg.DepsErrors {
+				// Error messages sometimes include a trailing newline.
+				pkgErrors = append(pkgErrors, strings.TrimSpace(derr.Err))
 			}
 		}
 
@@ -320,6 +328,7 @@ func appendListedPackages(packages []string, mainBuild bool) error {
 			path == "runtime/cgo":
 
 		// We can't obfuscate packages which weren't loaded.
+		// This can happen since we ignore some pkg.Error messages above.
 		case pkg.Incomplete:
 
 		// No point in obfuscating empty packages.

testdata/script/list_error.txtar

@@ -0,0 +1,30 @@
+! garble build ./...
+cmpenv stderr stderr.golden
+
+-- stderr.golden --
+# test/main/broken
+broken${/}broken.go:5:16: cannot use 123 (untyped int constant) as string value in variable declaration
+# test/main/imports_missing
+package test/main/missing is not in GOROOT (${GOROOT}${/}src${/}test${/}main${/}missing)
+-- go.mod --
+module test/main
+
+go 1.20
+-- broken/broken.go --
+package broken
+
+// A broken package will list in JSON form like:
+// {..., "Incomplete": true, "Error": {...}, ...}
+var x string = 123
+-- imports_broken/imports.go --
+package imports_broken
+
+// Importing a broken package will list in JSON form like:
+// {..., "Incomplete": false, ...}
+import _ "test/main/broken"
+-- imports_missing/imports.go --
+package imports_missing
+
+// Importing a missing package will list in JSON form like:
+// {..., "Incomplete": true, "DepsErrors": [...], ...}
+import _ "test/main/missing"