[release-branch.go1.10] go/internal/srcimporter: simplify and fix package file lookup

The old code was a blend of (copied) code that existed before go/build,
and incorrect adjustments made when go/build was introduced. This change
leaves package path determination entirely to go/build and in the process
fixes issues with relative import paths.

Fixes #23092
Fixes #24392

Change-Id: I9e900538b365398751bace56964495c5440ac4ae
Reviewed-on: https://go-review.googlesource.com/83415
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-on: https://go-review.googlesource.com/102789
Run-TryBot: Andrew Bonventre <andybons@golang.org>

diff --git a/src/go/internal/srcimporter/srcimporter.go b/src/go/internal/srcimporter/srcimporter.go
index b0dc8abfc2b8b40012539b2b0e7cdadc230fca5d..9ed7e5e4dca6f98bdd61184cc6a85261b78e5658 100644 (file)
--- a/src/go/internal/srcimporter/srcimporter.go
+++ b/src/go/internal/srcimporter/srcimporter.go
@@ -44,9 +44,9 @@ func New(ctxt *build.Context, fset *token.FileSet, packages map[string]*types.Pa
 // for a package that is in the process of being imported.
 var importing types.Package
 
-// Import(path) is a shortcut for ImportFrom(path, "", 0).
+// Import(path) is a shortcut for ImportFrom(path, ".", 0).
 func (p *Importer) Import(path string) (*types.Package, error) {
-       return p.ImportFrom(path, "", 0)
+       return p.ImportFrom(path, ".", 0) // use "." rather than "" (see issue #24441)
 }
 
 // ImportFrom imports the package with the given import path resolved from the given srcDir,
@@ -60,23 +60,10 @@ func (p *Importer) ImportFrom(path, srcDir string, mode types.ImportMode) (*type
                panic("non-zero import mode")
        }
 
-       // determine package path (do vendor resolution)
-       var bp *build.Package
-       var err error
-       switch {
-       default:
-               if abs, err := p.absPath(srcDir); err == nil { // see issue #14282
-                       srcDir = abs
-               }
-               bp, err = p.ctxt.Import(path, srcDir, build.FindOnly)
-
-       case build.IsLocalImport(path):
-               // "./x" -> "srcDir/x"
-               bp, err = p.ctxt.ImportDir(filepath.Join(srcDir, path), build.FindOnly)
-
-       case p.isAbsPath(path):
-               return nil, fmt.Errorf("invalid absolute import path %q", path)
+       if abs, err := p.absPath(srcDir); err == nil { // see issue #14282
+               srcDir = abs
        }
+       bp, err := p.ctxt.Import(path, srcDir, 0)
        if err != nil {
                return nil, err // err may be *build.NoGoError - return as is
        }
@@ -113,11 +100,6 @@ func (p *Importer) ImportFrom(path, srcDir string, mode types.ImportMode) (*type
                }
        }()
 
-       // collect package files
-       bp, err = p.ctxt.ImportDir(bp.Dir, 0)
-       if err != nil {
-               return nil, err // err may be *build.NoGoError - return as is
-       }
        var filenames []string
        filenames = append(filenames, bp.GoFiles...)
        filenames = append(filenames, bp.CgoFiles...)

diff --git a/src/go/internal/srcimporter/srcimporter_test.go b/src/go/internal/srcimporter/srcimporter_test.go
index 356e71d12873a78ce3ce06ccfa016e7c05aca994..dd4d56ad17dd612079a32808abb137f1c2ec6f03 100644 (file)
--- a/src/go/internal/srcimporter/srcimporter_test.go
+++ b/src/go/internal/srcimporter/srcimporter_test.go
@@ -10,6 +10,7 @@ import (
        "go/types"
        "internal/testenv"
        "io/ioutil"
+       "path"
        "path/filepath"
        "runtime"
        "strings"
@@ -162,3 +163,34 @@ func TestIssue20855(t *testing.T) {
                t.Error("got no package despite no hard errors")
        }
 }
+
+func testImportPath(t *testing.T, pkgPath string) {
+       if !testenv.HasSrc() {
+               t.Skip("no source code available")
+       }
+
+       pkgName := path.Base(pkgPath)
+
+       pkg, err := importer.Import(pkgPath)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       if pkg.Name() != pkgName {
+               t.Errorf("got %q; want %q", pkg.Name(), pkgName)
+       }
+
+       if pkg.Path() != pkgPath {
+               t.Errorf("got %q; want %q", pkg.Path(), pkgPath)
+       }
+}
+
+// TestIssue23092 tests relative imports.
+func TestIssue23092(t *testing.T) {
+       testImportPath(t, "./testdata/issue23092")
+}
+
+// TestIssue24392 tests imports against a path containing 'testdata'.
+func TestIssue24392(t *testing.T) {
+       testImportPath(t, "go/internal/srcimporter/testdata/issue24392")
+}

diff --git a/src/go/internal/srcimporter/testdata/issue23092/issue23092.go b/src/go/internal/srcimporter/testdata/issue23092/issue23092.go
new file mode 100644 (file)
index 0000000..608698b
--- /dev/null
+++ b/src/go/internal/srcimporter/testdata/issue23092/issue23092.go
@@ -0,0 +1,5 @@
+// Copyright 2018 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 issue23092

diff --git a/src/go/internal/srcimporter/testdata/issue24392/issue24392.go b/src/go/internal/srcimporter/testdata/issue24392/issue24392.go
new file mode 100644 (file)
index 0000000..8ad5221
--- /dev/null
+++ b/src/go/internal/srcimporter/testdata/issue24392/issue24392.go
@@ -0,0 +1,5 @@
+// Copyright 2018 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 issue24392