go/importer: make source importer more tolerant in presence of errors

If the source importer only encounters "soft" type checking errors
it can safely return the type-checked package because it will be
completely set up. This makes the source importer slightly more
robust in the presence of errors.

Fixes #20855.

Change-Id: I5af9ccdb30eee6bca7a0fab872f6057bde521bf3
Reviewed-on: https://go-review.googlesource.com/55730
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/go/internal/srcimporter/srcimporter.go b/src/go/internal/srcimporter/srcimporter.go
index 50cf361dbb01be39207701deeedf7c0e91413009..b0dc8abfc2b8b40012539b2b0e7cdadc230fca5d 100644 (file)
--- a/src/go/internal/srcimporter/srcimporter.go
+++ b/src/go/internal/srcimporter/srcimporter.go
@@ -128,19 +128,33 @@ func (p *Importer) ImportFrom(path, srcDir string, mode types.ImportMode) (*type
        }
 
        // type-check package files
+       var firstHardErr error
        conf := types.Config{
                IgnoreFuncBodies: true,
                FakeImportC:      true,
-               Importer:         p,
-               Sizes:            p.sizes,
+               // continue type-checking after the first error
+               Error: func(err error) {
+                       if firstHardErr == nil && !err.(types.Error).Soft {
+                               firstHardErr = err
+                       }
+               },
+               Importer: p,
+               Sizes:    p.sizes,
        }
        pkg, err = conf.Check(bp.ImportPath, p.fset, files, nil)
        if err != nil {
-               // Type-checking stops after the first error (types.Config.Error is not set),
-               // so the returned package is very likely incomplete. Don't return it since
-               // we don't know its condition: It's very likely unsafe to use and it's also
-               // not added to p.packages which may cause further problems (issue #20837).
-               return nil, fmt.Errorf("type-checking package %q failed (%v)", bp.ImportPath, err)
+               // If there was a hard error it is possibly unsafe
+               // to use the package as it may not be fully populated.
+               // Do not return it (see also #20837, #20855).
+               if firstHardErr != nil {
+                       pkg = nil
+                       err = firstHardErr // give preference to first hard error over any soft error
+               }
+               return pkg, fmt.Errorf("type-checking package %q failed (%v)", bp.ImportPath, err)
+       }
+       if firstHardErr != nil {
+               // this can only happen if we have a bug in go/types
+               panic("package is not safe yet no error was returned")
        }
 
        p.packages[bp.ImportPath] = pkg

diff --git a/src/go/internal/srcimporter/srcimporter_test.go b/src/go/internal/srcimporter/srcimporter_test.go
index 79921b5e7855bd0ba46f1541baa137a6fe493aaa..356e71d12873a78ce3ce06ccfa016e7c05aca994 100644 (file)
--- a/src/go/internal/srcimporter/srcimporter_test.go
+++ b/src/go/internal/srcimporter/srcimporter_test.go
@@ -148,3 +148,17 @@ func TestReimport(t *testing.T) {
                t.Errorf("got %v; want reimport error", err)
        }
 }
+
+func TestIssue20855(t *testing.T) {
+       if !testenv.HasSrc() {
+               t.Skip("no source code available")
+       }
+
+       pkg, err := importer.ImportFrom("go/internal/srcimporter/testdata/issue20855", ".", 0)
+       if err == nil || !strings.Contains(err.Error(), "missing function body") {
+               t.Fatalf("got unexpected or no error: %v", err)
+       }
+       if pkg == nil {
+               t.Error("got no package despite no hard errors")
+       }
+}

diff --git a/src/go/internal/srcimporter/testdata/issue20855/issue20855.go b/src/go/internal/srcimporter/testdata/issue20855/issue20855.go
new file mode 100644 (file)
index 0000000..d55448b
--- /dev/null
+++ b/src/go/internal/srcimporter/testdata/issue20855/issue20855.go
@@ -0,0 +1,7 @@
+// 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 issue20855
+
+func init() // "missing function body" is a soft error