Skip to content

Commit

Permalink
cmd/go/internal/vet: print line numbers appropriately on list errors
Browse files Browse the repository at this point in the history
Fixes golang#36173

For reasons that are unclear to me, this commit:
golang@f1d5ce0
introduces a TestPackagesFor function that strips line numbers from error
messages. This commit introduces a new version of that function for 'go vet'
that always keeps the line numbers.
  • Loading branch information
nicks committed Feb 11, 2020
1 parent 753d56d commit 5a6a3df
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 2 deletions.
9 changes: 8 additions & 1 deletion src/cmd/go/internal/load/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -1300,8 +1300,15 @@ func disallowInternal(srcDir string, importer *Package, importerPath string, p *

// Internal is present, and srcDir is outside parent's tree. Not allowed.
perr := *p

// The stack in the error should point to the importing package,
// so pop off the last stack frame.
importStack := stk.Copy()
if len(importStack) > 0 {
importStack = importStack[:len(importStack)-1]
}
perr.Error = &PackageError{
ImportStack: stk.Copy(),
ImportStack: importStack,
Err: ImportErrorf(p.ImportPath, "use of internal package "+p.ImportPath+" not allowed"),
}
perr.Incomplete = true
Expand Down
14 changes: 13 additions & 1 deletion src/cmd/go/internal/load/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,19 @@ func TestPackagesFor(p *Package, cover *TestCover) (pmain, ptest, pxtest *Packag
}
if len(p1.DepsErrors) > 0 {
perr := p1.DepsErrors[0]
perr.Pos = "" // show full import stack
isDirect := false

// The package with the error has the highest index in the stack.
if len(perr.ImportStack) >= 1 {
importer := strings.TrimSuffix(perr.ImportStack[len(perr.ImportStack)-1], " (test)")
isDirect = importer == p1.ImportPath
}

if !isDirect {
// Remove the position information, so that we'll
// print the full import stack to the error.
perr.Pos = ""
}
err = perr
break
}
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/go/testdata/script/test_import_error_stack.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
! go test testdep/p1
stderr 'package testdep/p1 \(test\)\n\timports testdep/p2\n\timports testdep/p3: build constraints exclude all Go files ' # check for full import stack

! go vet testdep/p1
stderr 'package testdep/p1 \(test\)\n\timports testdep/p2\n\timports testdep/p3: build constraints exclude all Go files ' # check for full import stack

-- testdep/p1/p1.go --
package p1
-- testdep/p1/p1_test.go --
Expand Down
71 changes: 71 additions & 0 deletions src/cmd/go/testdata/script/vet_internal.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
env GO111MODULE=off

# Issue 36173. Verify that "go vet" prints line numbers on load errors.

! go vet a/a.go
stderr 'a.go:5:3: use of internal package'

! go vet a/a_test.go
stderr 'a_test.go:4:3: use of internal package'

! go vet a
stderr 'a.go:5:3: use of internal package'

go vet b/b.go
! stderr 'use of internal package'

! go vet b/b_test.go
stderr 'b_test.go:4:3: use of internal package'

! go vet depends-on-a/depends-on-a.go
stderr 'a.go:5:3: use of internal package'

! go vet depends-on-a/depends-on-a_test.go
stderr 'package command-line-arguments \(test\)\n\timports a: use of internal package a/x/internal/y not allowed'

! go vet depends-on-a
stderr 'a.go:5:3: use of internal package'

-- a/a.go --
// A package with bad imports in both src and test
package a

import (
_ "a/x/internal/y"
)

-- a/a_test.go --
package a

import (
_ "a/x/internal/y"
)

-- b/b.go --
// A package with a bad import in test only
package b

-- b/b_test.go --
package b

import (
_ "a/x/internal/y"
)

-- depends-on-a/depends-on-a.go --
// A package that depends on a package with a bad import
package depends

import (
_ "a"
)

-- depends-on-a/depends-on-a_test.go --
package depends

import (
_ "a"
)

-- a/x/internal/y/y.go --
package y

0 comments on commit 5a6a3df

Please sign in to comment.