internal/lsp: use mode (GOPATH/modules) oriented error messages

We can offer better error messages to users in GOPATH mode. The Go
command has clear error messages indicating where it's looking for
its dependencies, so we can borrow these messages too.

The error message will look like this:
https://user-images.githubusercontent.com/5856771/100292862-bde8bb00-2f4f-11eb-9920-d945ee2c7e87.png

Change-Id: I39d367d0731ea5b7b7bb963699be3003b4fefe86
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274119
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2020-11-30 18:45:54 +00:00
parent f0400ba216
commit fd09bd90d8
7 changed files with 60 additions and 20 deletions

View File

@ -1557,3 +1557,40 @@ import _ "mod.com/triple/a"
)
})
}
func TestBadImport(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
const mod = `
-- go.mod --
module mod.com
go 1.12
-- main.go --
package main
import (
_ "nosuchpkg"
)
`
t.Run("module", func(t *testing.T) {
run(t, mod, func(t *testing.T, env *Env) {
env.Await(
env.DiagnosticAtRegexpWithMessage("main.go", `"nosuchpkg"`, `could not import nosuchpkg (no required module provides package "nosuchpkg"`),
)
})
})
t.Run("GOPATH", func(t *testing.T) {
withOptions(
InGOPATH(),
EditorConfig{
Env: map[string]string{"GO111MODULE": "off"},
},
WithModes(WithoutExperiments),
).run(t, mod, func(t *testing.T, env *Env) {
env.Await(
env.DiagnosticAtRegexpWithMessage("main.go", `"nosuchpkg"`, `cannot find package "nosuchpkg" in any of`),
)
})
})
}

View File

@ -11,6 +11,7 @@ import (
"go/ast"
"go/types"
"path"
"path/filepath"
"sort"
"strings"
"sync"
@ -407,7 +408,7 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source
}
dep := resolveImportPath(pkgPath, pkg, deps)
if dep == nil {
return nil, errors.Errorf("no package for import %s", pkgPath)
return nil, snapshot.missingPkgError(pkgPath)
}
if !isValidImport(m.pkgPath, dep.m.pkgPath) {
return nil, errors.Errorf("invalid use of internal package %s", pkgPath)
@ -453,6 +454,24 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source
return pkg, nil
}
// missingPkgError returns an error message for a missing package that varies
// based on the user's workspace mode.
func (s *snapshot) missingPkgError(pkgPath string) error {
if s.workspaceMode()&moduleMode != 0 {
return fmt.Errorf("no required module provides package %q", pkgPath)
}
gorootSrcPkg := filepath.FromSlash(filepath.Join(s.view.goroot, "src", pkgPath))
var b strings.Builder
b.WriteString(fmt.Sprintf("cannot find package %q in any of \n\t%s (from $GOROOT)", pkgPath, gorootSrcPkg))
for _, gopath := range strings.Split(s.view.gopath, ":") {
gopathSrcPkg := filepath.FromSlash(filepath.Join(gopath, "src", pkgPath))
b.WriteString(fmt.Sprintf("\n\t%s (from $GOPATH)", gopathSrcPkg))
}
return errors.New(b.String())
}
type extendedError struct {
primary types.Error
secondaries []types.Error

View File

@ -119,7 +119,7 @@ type workspaceInformation struct {
}
type environmentVariables struct {
gocache, gopath, goprivate, gomodcache string
gocache, gopath, goroot, goprivate, gomodcache string
}
type workspaceMode int
@ -784,6 +784,7 @@ func (s *Session) getGoEnv(ctx context.Context, folder string, configEnv []strin
vars := map[string]*string{
"GOCACHE": &envVars.gocache,
"GOPATH": &envVars.gopath,
"GOROOT": &envVars.goroot,
"GOPRIVATE": &envVars.goprivate,
"GOMODCACHE": &envVars.gomodcache,
}

View File

@ -50,10 +50,6 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnost
expect = fmt.Sprintf("%v:%v: %v", uri.Filename(), diag.Range.Start.Line+1, diag.Message)
}
expect = r.NormalizePrefix(expect)
// Skip the badimport test for now, until we do a better job with diagnostic ranges.
if strings.Contains(uri.Filename(), "badimport") {
continue
}
_, found := got[expect]
if !found {
t.Errorf("missing diagnostic %q, %v", expect, got)
@ -62,10 +58,6 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnost
}
}
for extra := range got {
// Skip the badimport test for now, until we do a better job with diagnostic ranges.
if strings.Contains(extra, "badimport") {
continue
}
t.Errorf("extra diagnostic %q", extra)
}
}

View File

@ -1,5 +0,0 @@
package bad
import (
_ "nosuchpkg" //@diag("_", "compiler", "could not import nosuchpkg (no package for import nosuchpkg)", "error")
)

View File

@ -8,7 +8,7 @@ DeepCompletionsCount = 5
FuzzyCompletionsCount = 8
RankedCompletionsCount = 159
CaseSensitiveCompletionsCount = 4
DiagnosticsCount = 38
DiagnosticsCount = 37
FoldingRangesCount = 2
FormatCount = 6
ImportCount = 8

View File

@ -129,10 +129,6 @@ func DiffDiagnostics(uri span.URI, want, got []*source.Diagnostic) string {
if w.Source != g.Source {
return summarizeDiagnostics(i, uri, want, got, "incorrect Source got %v want %v", g.Source, w.Source)
}
// Don't check the range on the badimport test.
if strings.Contains(uri.Filename(), "badimport") {
continue
}
if protocol.ComparePosition(w.Range.Start, g.Range.Start) != 0 {
return summarizeDiagnostics(i, uri, want, got, "incorrect Start got %v want %v", g.Range.Start, w.Range.Start)
}