Skip to content

Commit 8c74bfb

Browse files
HelcaraxanBryan C. Mills
authored and
Bryan C. Mills
committed
cmd/go: fix listing of ambiguous paths
Passing ambiguous patterns, ending in `.go`, to `go list` results in them being interpreted as Go files despite potentially being package references. This can then result in errors on other package references. The parsing logic is modified to check for a locally present file corresponding to any pattern ending in `.go`. If no such file is present the pattern is considered to be a package reference. We're also adding a variety of non-regression tests that fail with the original parsing code but passes after applying the fix. Fixes #32483 Fixes #34653 Change-Id: I073871da0dfc5641a359643f95ac14608fdca09b GitHub-Last-Rev: 5abc200 GitHub-Pull-Request: #34663 Reviewed-on: https://go-review.googlesource.com/c/go/+/198459 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
1 parent a704224 commit 8c74bfb

File tree

4 files changed

+51
-20
lines changed

4 files changed

+51
-20
lines changed

src/cmd/go/internal/load/pkg.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,9 +1950,14 @@ func Packages(args []string) []*Package {
19501950
// cannot be loaded at all.
19511951
// The packages that fail to load will have p.Error != nil.
19521952
func PackagesAndErrors(patterns []string) []*Package {
1953-
if len(patterns) > 0 {
1954-
for _, p := range patterns {
1955-
if strings.HasSuffix(p, ".go") {
1953+
for _, p := range patterns {
1954+
// Listing is only supported with all patterns referring to either:
1955+
// - Files that are part of the same directory.
1956+
// - Explicit package paths or patterns.
1957+
if strings.HasSuffix(p, ".go") {
1958+
// We need to test whether the path is an actual Go file and not a
1959+
// package path or pattern ending in '.go' (see golang.org/issue/34653).
1960+
if fi, err := os.Stat(p); err == nil && !fi.IsDir() {
19561961
return []*Package{GoFilesPackage(patterns)}
19571962
}
19581963
}

src/cmd/go/internal/modget/get.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -678,15 +678,6 @@ func runGet(cmd *base.Command, args []string) {
678678
if *getD || len(pkgPatterns) == 0 {
679679
return
680680
}
681-
// TODO(golang.org/issue/32483): handle paths ending with ".go" consistently
682-
// with 'go build'. When we load packages above, we interpret arguments as
683-
// package patterns, not source files. To preserve that interpretation here,
684-
// we add a trailing slash to any patterns ending with ".go".
685-
for i := range pkgPatterns {
686-
if strings.HasSuffix(pkgPatterns[i], ".go") {
687-
pkgPatterns[i] += "/"
688-
}
689-
}
690681
work.BuildInit()
691682
pkgs := load.PackagesForBuild(pkgPatterns)
692683
work.InstallPackages(pkgPatterns, pkgs)
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Ensures that we can correctly list package patterns ending in '.go'.
2+
# See golang.org/issue/34653.
3+
4+
# A single pattern for a package ending in '.go'.
5+
go list ./foo.go
6+
stdout '^test/foo.go$'
7+
8+
# Multiple patterns for packages including one ending in '.go'.
9+
go list ./bar ./foo.go
10+
stdout '^test/bar$'
11+
stdout '^test/foo.go$'
12+
13+
# A single pattern for a Go file.
14+
go list ./a.go
15+
stdout '^command-line-arguments$'
16+
17+
# A single typo-ed pattern for a Go file. This should
18+
# treat the wrong pattern as if it were a package.
19+
! go list ./foo.go/b.go
20+
stderr 'package ./foo.go/b.go: cannot find package "."'
21+
22+
# Multiple patterns for Go files with a typo. This should
23+
# treat the wrong pattern as if it were a non-existint file.
24+
! go list ./foo.go/a.go ./foo.go/b.go
25+
[windows] stderr './foo.go/b.go: The system cannot find the file specified'
26+
[!windows] stderr './foo.go/b.go: no such file or directory'
27+
28+
-- a.go --
29+
package main
30+
-- bar/a.go --
31+
package bar
32+
-- foo.go/a.go --
33+
package foo.go
34+
-- go.mod --
35+
module "test"
36+
37+
go 1.13

src/cmd/go/testdata/script/mod_get_trailing_slash.txt

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
# go list should fail to load a package ending with ".go" since that denotes
2-
# a source file. However, ".go/" should work.
3-
# TODO(golang.org/issue/32483): perhaps we should treat non-existent paths
4-
# with .go suffixes as package paths instead.
5-
! go list example.com/dotgo.go
1+
# go list should succeed to load a package ending with ".go" if the path does
2+
# not correspond to an existing local file. Listing a pattern ending with
3+
# ".go/" should try to list a package regardless of whether a file exists at the
4+
# path without the suffixed "/" or not.
5+
go list example.com/dotgo.go
6+
stdout ^example.com/dotgo.go$
67
go list example.com/dotgo.go/
78
stdout ^example.com/dotgo.go$
89

@@ -15,9 +16,6 @@ go get -d example.com/dotgo.go@v1.0.0
1516
go get -d example.com/dotgo.go/@v1.0.0
1617

1718
# go get (without -d) should also succeed in either case.
18-
# TODO(golang.org/issue/32483): we should be consistent with 'go build',
19-
# 'go list', and other commands. 'go list example.com/dotgo.go' (above) and
20-
# 'go get example.com/dotgo.go' should both succeed or both fail.
2119
[short] skip
2220
go get example.com/dotgo.go
2321
go get example.com/dotgo.go/

0 commit comments

Comments
 (0)