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: I888b33804eec7f38c329ae2e4343b82fcc22f1df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/272166
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2020-11-21 01:17:56 -05:00
parent e7a5458643
commit ccae4fb384
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 = 158
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)
}