Skip to content

Commit

Permalink
cmd/go: special error message for direct self import
Browse files Browse the repository at this point in the history
This attempts to sort out confusion about cyclical imports. In addition
to the error message "import cycle not allowed", for direct self
imports we will now instead give a specialized error message, namely:

    self import not allowed

Fixes golang#23295

Change-Id: I14e2418aa4eceb8caf2e6520c686fac7e85cb0dd
  • Loading branch information
HaraldNordgren committed Jan 7, 2018
1 parent a62071a commit 6a820f5
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 1 deletion.
16 changes: 16 additions & 0 deletions src/cmd/go/go_test.go
Expand Up @@ -1351,6 +1351,22 @@ func TestImportCommentConflict(t *testing.T) {
tg.grepStderr("found import comments", "go build did not mention comment conflict")
}

func TestImportCycle(t *testing.T) {
tg := testgo(t)
defer tg.cleanup()
tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata/importcycle"))
tg.runFail("build", "./testdata/importcycle/cycle.go")
tg.grepStderr("import cycle not allowed", "go build did not mention cyclical import")
}

func TestImportCycleSelfImport(t *testing.T) {
tg := testgo(t)
defer tg.cleanup()
tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata/importcycle"))
tg.runFail("build", "./testdata/importcycle/selfimport.go")
tg.grepStderr("self import not allowed", "go build did not mention self import")
}

// cmd/go: custom import path checking should not apply to Go packages without import comment.
func TestIssue10952(t *testing.T) {
testenv.MustHaveExternalNetwork(t)
Expand Down
15 changes: 14 additions & 1 deletion src/cmd/go/internal/load/pkg.go
Expand Up @@ -598,6 +598,19 @@ func hasGoFiles(dir string) bool {
return false
}

// getImportError returns the error message for cyclical imports. It gives a
// special message for a packages that directly imports itself. For all other
// cyclical imports it gives a general message about import cycles.
func getImportError(p *Package) string {
for _, i := range p.Imports {
if i == p.ImportPath {
return "self import not allowed"
}
}

return "import cycle not allowed"
}

// reusePackage reuses package p to satisfy the import at the top
// of the import stack stk. If this use causes an import loop,
// reusePackage updates p's error information to record the loop.
Expand All @@ -609,7 +622,7 @@ func reusePackage(p *Package, stk *ImportStack) *Package {
if p.Error == nil {
p.Error = &PackageError{
ImportStack: stk.Copy(),
Err: "import cycle not allowed",
Err: getImportError(p),
IsImportCycle: true,
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/go/testdata/importcycle/cycle.go
@@ -0,0 +1,3 @@
package p

import "cycle1"
3 changes: 3 additions & 0 deletions src/cmd/go/testdata/importcycle/selfimport.go
@@ -0,0 +1,3 @@
package p

import "selfimport"
6 changes: 6 additions & 0 deletions src/cmd/go/testdata/importcycle/src/cycle1/cycle1.go
@@ -0,0 +1,6 @@
package cycle1

import (
"_noncyclic"
"cycle2"
)
3 changes: 3 additions & 0 deletions src/cmd/go/testdata/importcycle/src/cycle2/cycle2.go
@@ -0,0 +1,3 @@
package cycle2

import "cycle1"
1 change: 1 addition & 0 deletions src/cmd/go/testdata/importcycle/src/noncyclic/noncyclic.go
@@ -0,0 +1 @@
package _noncyclic
6 changes: 6 additions & 0 deletions src/cmd/go/testdata/importcycle/src/selfimport/selfimport.go
@@ -0,0 +1,6 @@
package selfimport

import (
"_noncyclic"
"selfimport"
)

0 comments on commit 6a820f5

Please sign in to comment.