From b26ad605a92af65deef30eb196ff2173566ee60f Mon Sep 17 00:00:00 2001 From: griesemer Date: Tue, 15 Aug 2017 14:44:26 +0200 Subject: [PATCH] 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 --- src/go/internal/srcimporter/srcimporter.go | 28 ++++++++++++++----- .../internal/srcimporter/srcimporter_test.go | 14 ++++++++++ .../testdata/issue20855/issue20855.go | 7 +++++ 3 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 src/go/internal/srcimporter/testdata/issue20855/issue20855.go diff --git a/src/go/internal/srcimporter/srcimporter.go b/src/go/internal/srcimporter/srcimporter.go index 50cf361dbb..b0dc8abfc2 100644 --- 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 79921b5e78..356e71d128 100644 --- 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 index 0000000000..d55448b44c --- /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