internal/lsp: refactor go command error handling

Our handling of go command errors was cobbled together, leading to
unexpected gaps and duplication. Refactor it to be more coherent.

Our goal is to turn every go command error into a diagnostic in the
relevant location. The errors don't contain error positions, so we have
to guess where they belong using the module names mentioned in the
error. If we can't find any reference to those modules, we are forced to
add diagnostics to all go.mod files.

I may have destroyed the intent of TestMultiModule_OneBrokenModule but
I'm not sure what to do about it.

Some cleanup along the way:
- Stop parsing modfile.Parse error text: it returns structured errors
and we can just use them.
- Return CriticalErrors from awaitLoadedAllErrors, and do error
extraction lower in the stack. This prevents a ridiculous situation
where initialize formed a CriticalError, then awaitLoadedAllErrors
returned just its MainError, and then GetCriticalError parsed out
a new CriticalError from the MainError we got from a CriticalError.
- In initialize, return modDiagnostics even if load succeeds: we are
missing packages and should not silently fail, I think?
- During testing I tripped over ApplyQuickFixes' willingness to not
actually do anything, so I made that an error.

Fixes golang/go#44132.
I may also have fixed golang/go#44204 but I haven't checked.

Change-Id: Ibf819d0f044d4f99795978a28b18915893e50c88
Reviewed-on: https://go-review.googlesource.com/c/tools/+/291192
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This commit is contained in:
Heschi Kreinick 2021-02-09 21:57:43 -05:00
parent ffc207509a
commit 1e7abacf3b
11 changed files with 257 additions and 341 deletions

View File

@ -16,7 +16,6 @@ import (
"golang.org/x/tools/internal/lsp"
"golang.org/x/tools/internal/lsp/fake"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/tests"
"golang.org/x/tools/internal/testenv"
)
@ -526,7 +525,6 @@ func _() {
`
Run(t, generated, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
original := env.ReadWorkspaceFile("main.go")
var d protocol.PublishDiagnosticsParams
env.Await(
OnceMet(
@ -534,12 +532,8 @@ func _() {
ReadDiagnostics("main.go", &d),
),
)
// Apply fixes and save the buffer.
env.ApplyQuickFixes("main.go", d.Diagnostics)
env.SaveBuffer("main.go")
fixed := env.ReadWorkspaceFile("main.go")
if original != fixed {
t.Fatalf("generated file was changed by quick fixes:\n%s", tests.Diff(t, original, fixed))
if fixes := env.GetQuickFixes("main.go", d.Diagnostics); len(fixes) != 0 {
t.Errorf("got quick fixes %v, wanted none", fixes)
}
})
}

View File

@ -12,6 +12,7 @@ import (
"golang.org/x/tools/internal/lsp"
"golang.org/x/tools/internal/lsp/fake"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/testenv"
)
// An Expectation asserts that the state of the editor at a point in time
@ -580,3 +581,16 @@ func NoDiagnosticAt(name string, line, col int) DiagnosticExpectation {
func NoDiagnosticWithMessage(name, msg string) DiagnosticExpectation {
return DiagnosticExpectation{path: name, message: msg, present: false}
}
// GoSum asserts that a "go.sum is out of sync" diagnostic for the given module
// (as formatted in a go.mod file, e.g. "example.com v1.0.0") is present.
func (e *Env) GoSumDiagnostic(name, module string) Expectation {
e.T.Helper()
// In 1.16, go.sum diagnostics should appear on the relevant module. Earlier
// errors have no information and appear on the module declaration.
if testenv.Go1Point() >= 16 {
return e.DiagnosticAtRegexpWithMessage(name, module, "go.sum is out of sync")
} else {
return e.DiagnosticAtRegexpWithMessage(name, `module`, "go.sum is out of sync")
}
}

View File

@ -548,6 +548,7 @@ func main() {
d := protocol.PublishDiagnosticsParams{}
env.Await(
OnceMet(
// Make sure the diagnostic mentions the new version -- the old diagnostic is in the same place.
env.DiagnosticAtRegexpWithMessage("a/go.mod", "example.com v1.2.3", "example.com@v1.2.3"),
ReadDiagnostics("a/go.mod", &d),
),
@ -822,7 +823,7 @@ func main() {
env.OpenFile("go.mod")
env.Await(
OnceMet(
DiagnosticAt("go.mod", 0, 0),
env.GoSumDiagnostic("go.mod", `example.com v1.2.3`),
ReadDiagnostics("go.mod", d),
),
)
@ -1001,9 +1002,7 @@ require random.com v1.2.3
}
func TestSumUpdateQuickFix(t *testing.T) {
// Error messages changed in 1.16 that changed the diagnostic positions.
testenv.NeedsGo1Point(t, 16)
testenv.NeedsGo1Point(t, 14)
const mod = `
-- go.mod --
module mod.com
@ -1030,22 +1029,14 @@ func main() {
Modes(Singleton),
).Run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
pos := env.RegexpSearch("go.mod", "example.com")
params := &protocol.PublishDiagnosticsParams{}
env.Await(
OnceMet(
env.DiagnosticAtRegexp("go.mod", "example.com"),
env.GoSumDiagnostic("go.mod", "example.com"),
ReadDiagnostics("go.mod", params),
),
)
var diagnostic protocol.Diagnostic
for _, d := range params.Diagnostics {
if d.Range.Start.Line == uint32(pos.Line) {
diagnostic = d
break
}
}
env.ApplyQuickFixes("go.mod", []protocol.Diagnostic{diagnostic})
env.ApplyQuickFixes("go.mod", params.Diagnostics)
const want = `example.com v1.2.3 h1:Yryq11hF02fEf2JlOS2eph+ICE2/ceevGV3C9dl5V/c=
example.com v1.2.3/go.mod h1:Y2Rc5rVWjWur0h3pd9aEvK5Pof8YKDANh9gHA2Maujo=
`

View File

@ -818,25 +818,25 @@ func main() {
Modes(Experimental),
).Run(t, mod, func(t *testing.T, env *Env) {
params := &protocol.PublishDiagnosticsParams{}
env.OpenFile("a/go.mod")
env.OpenFile("b/go.mod")
env.Await(
ReadDiagnostics("a/go.mod", params),
OnceMet(
env.GoSumDiagnostic("b/go.mod", `example.com v1.2.3`),
ReadDiagnostics("b/go.mod", params),
),
)
for _, d := range params.Diagnostics {
if d.Message != `go.sum is out of sync with go.mod. Please update it by applying the quick fix.` {
if !strings.Contains(d.Message, "go.sum is out of sync") {
continue
}
actions, err := env.Editor.GetQuickFixes(env.Ctx, "a/go.mod", nil, []protocol.Diagnostic{d})
if err != nil {
t.Fatal(err)
}
actions := env.GetQuickFixes("b/go.mod", []protocol.Diagnostic{d})
if len(actions) != 2 {
t.Fatalf("expected 2 code actions, got %v", len(actions))
}
env.ApplyQuickFixes("a/go.mod", []protocol.Diagnostic{d})
env.ApplyQuickFixes("b/go.mod", []protocol.Diagnostic{d})
}
env.Await(
EmptyDiagnostics("a/go.mod"),
EmptyDiagnostics("b/go.mod"),
)
})
}

View File

@ -200,6 +200,16 @@ func (e *Env) ApplyQuickFixes(path string, diagnostics []protocol.Diagnostic) {
}
}
// GetQuickFixes returns the available quick fix code actions.
func (e *Env) GetQuickFixes(path string, diagnostics []protocol.Diagnostic) []protocol.CodeAction {
e.T.Helper()
actions, err := e.Editor.GetQuickFixes(e.Ctx, path, nil, diagnostics)
if err != nil {
e.T.Fatal(err)
}
return actions
}
// Hover in the editor, calling t.Fatal on any error.
func (e *Env) Hover(name string, pos fake.Pos) (*protocol.MarkupContent, fake.Pos) {
e.T.Helper()
@ -265,6 +275,8 @@ func (e *Env) RunGoCommandInDir(dir, verb string, args ...string) {
}
}
// DumpGoSum prints the correct go.sum contents for dir in txtar format,
// for use in creating regtests.
func (e *Env) DumpGoSum(dir string) {
e.T.Helper()

View File

@ -60,13 +60,26 @@ func (s *snapshot) ParseMod(ctx context.Context, modFH source.FileHandle) (*sour
Converter: span.NewContentConverter(modFH.URI().Filename(), contents),
Content: contents,
}
file, err := modfile.Parse(modFH.URI().Filename(), contents, nil)
file, parseErr := modfile.Parse(modFH.URI().Filename(), contents, nil)
// Attempt to convert the error to a standardized parse error.
var parseErrors []*source.Diagnostic
if err != nil {
if parseErr := extractErrorWithPosition(ctx, err.Error(), s); parseErr != nil {
parseErrors = []*source.Diagnostic{parseErr}
if parseErr != nil {
mfErrList, ok := parseErr.(modfile.ErrorList)
if !ok {
return &parseModData{err: fmt.Errorf("unexpected parse error type %v", parseErr)}
}
for _, mfErr := range mfErrList {
rng, err := rangeFromPositions(m, mfErr.Pos, mfErr.Pos)
if err != nil {
return &parseModData{err: err}
}
parseErrors = []*source.Diagnostic{{
URI: modFH.URI(),
Range: rng,
Severity: protocol.SeverityError,
Source: source.ParseError,
Message: mfErr.Err.Error(),
}}
}
}
return &parseModData{
@ -76,7 +89,7 @@ func (s *snapshot) ParseMod(ctx context.Context, modFH source.FileHandle) (*sour
File: file,
ParseErrors: parseErrors,
},
err: err,
err: parseErr,
}
}, nil)
@ -214,46 +227,67 @@ func (s *snapshot) ModWhy(ctx context.Context, fh source.FileHandle) (map[string
// extractGoCommandError tries to parse errors that come from the go command
// and shape them into go.mod diagnostics.
func (s *snapshot) extractGoCommandErrors(ctx context.Context, snapshot source.Snapshot, goCmdError string) []*source.Diagnostic {
var srcErrs []*source.Diagnostic
if srcErr := s.parseModError(ctx, goCmdError); srcErr != nil {
srcErrs = append(srcErrs, srcErr...)
func (s *snapshot) extractGoCommandErrors(ctx context.Context, goCmdError string) ([]*source.Diagnostic, error) {
diagLocations := map[*source.ParsedModule]span.Span{}
backupDiagLocations := map[*source.ParsedModule]span.Span{}
// The go command emits parse errors for completely invalid go.mod files.
// Those are reported by our own diagnostics and can be ignored here.
// As of writing, we are not aware of any other errors that include
// file/position information, so don't even try to find it.
if strings.Contains(goCmdError, "errors parsing go.mod") {
return nil, nil
}
if srcErr := extractErrorWithPosition(ctx, goCmdError, s); srcErr != nil {
srcErrs = append(srcErrs, srcErr)
} else {
for _, uri := range s.ModFiles() {
fh, err := s.GetFile(ctx, uri)
if err != nil {
continue
}
if srcErr := s.matchErrorToModule(ctx, fh, goCmdError); srcErr != nil {
srcErrs = append(srcErrs, srcErr)
}
// Match the error against all the mod files in the workspace.
for _, uri := range s.ModFiles() {
fh, err := s.GetFile(ctx, uri)
if err != nil {
return nil, err
}
pm, err := s.ParseMod(ctx, fh)
if err != nil {
return nil, err
}
spn, found, err := s.matchErrorToModule(ctx, pm, goCmdError)
if err != nil {
return nil, err
}
if found {
diagLocations[pm] = spn
} else {
backupDiagLocations[pm] = spn
}
}
return srcErrs
// If we didn't find any good matches, assign diagnostics to all go.mod files.
if len(diagLocations) == 0 {
diagLocations = backupDiagLocations
}
var srcErrs []*source.Diagnostic
for pm, spn := range diagLocations {
diag, err := s.goCommandDiagnostic(pm, spn, goCmdError)
if err != nil {
return nil, err
}
srcErrs = append(srcErrs, diag)
}
return srcErrs, nil
}
var moduleVersionInErrorRe = regexp.MustCompile(`[:\s]([+-._~0-9A-Za-z]+)@([+-._~0-9A-Za-z]+)[:\s]`)
// matchErrorToModule attempts to match module version in error messages.
// matchErrorToModule matches a go command error message to a go.mod file.
// Some examples:
//
// example.com@v1.2.2: reading example.com/@v/v1.2.2.mod: no such file or directory
// go: github.com/cockroachdb/apd/v2@v2.0.72: reading github.com/cockroachdb/apd/go.mod at revision v2.0.72: unknown revision v2.0.72
// go: example.com@v1.2.3 requires\n\trandom.org@v1.2.3: parsing go.mod:\n\tmodule declares its path as: bob.org\n\tbut was required as: random.org
//
// We search for module@version, starting from the end to find the most
// relevant module, e.g. random.org@v1.2.3 above. Then we associate the error
// with a directive that references any of the modules mentioned.
func (s *snapshot) matchErrorToModule(ctx context.Context, fh source.FileHandle, goCmdError string) *source.Diagnostic {
pm, err := s.ParseMod(ctx, fh)
if err != nil {
return nil
}
var innermost *module.Version
// It returns the location of a reference to the one of the modules and true
// if one exists. If none is found it returns a fallback location and false.
func (s *snapshot) matchErrorToModule(ctx context.Context, pm *source.ParsedModule, goCmdError string) (span.Span, bool, error) {
var reference *modfile.Line
matches := moduleVersionInErrorRe.FindAllStringSubmatch(goCmdError, -1)
@ -267,9 +301,6 @@ func (s *snapshot) matchErrorToModule(ctx context.Context, fh source.FileHandle,
if err := module.Check(ver.Path, ver.Version); err != nil {
continue
}
if innermost == nil {
innermost = &ver
}
reference = findModuleReference(pm.File, ver)
if reference != nil {
break
@ -278,52 +309,112 @@ func (s *snapshot) matchErrorToModule(ctx context.Context, fh source.FileHandle,
if reference == nil {
// No match for the module path was found in the go.mod file.
// Show the error on the module declaration, if one exists.
// Show the error on the module declaration, if one exists, or
// just the first line of the file.
if pm.File.Module == nil {
return nil
return span.New(pm.URI, span.NewPoint(1, 1, 0), span.Point{}), false, nil
}
reference = pm.File.Module.Syntax
spn, err := spanFromPositions(pm.Mapper, pm.File.Module.Syntax.Start, pm.File.Module.Syntax.End)
return spn, false, err
}
rng, err := rangeFromPositions(pm.Mapper, reference.Start, reference.End)
spn, err := spanFromPositions(pm.Mapper, reference.Start, reference.End)
return spn, true, err
}
// goCommandDiagnostic creates a diagnostic for a given go command error.
func (s *snapshot) goCommandDiagnostic(pm *source.ParsedModule, spn span.Span, goCmdError string) (*source.Diagnostic, error) {
rng, err := pm.Mapper.Range(spn)
if err != nil {
return nil
return nil, err
}
disabledByGOPROXY := strings.Contains(goCmdError, "disabled by GOPROXY=off")
shouldAddDep := strings.Contains(goCmdError, "to add it")
if innermost != nil && (disabledByGOPROXY || shouldAddDep) {
matches := moduleVersionInErrorRe.FindAllStringSubmatch(goCmdError, -1)
var innermost *module.Version
for i := len(matches) - 1; i >= 0; i-- {
ver := module.Version{Path: matches[i][1], Version: matches[i][2]}
// Any module versions that come from the workspace module should not
// be shown to the user.
if source.IsWorkspaceModuleVersion(ver.Version) {
continue
}
if err := module.Check(ver.Path, ver.Version); err != nil {
continue
}
innermost = &ver
break
}
switch {
case strings.Contains(goCmdError, "inconsistent vendoring"):
cmd, err := command.NewVendorCommand("Run go mod vendor", command.URIArg{URI: protocol.URIFromSpanURI(pm.URI)})
if err != nil {
return nil, err
}
return &source.Diagnostic{
URI: pm.URI,
Range: rng,
Severity: protocol.SeverityError,
Source: source.ListError,
Message: `Inconsistent vendoring detected. Please re-run "go mod vendor".
See https://github.com/golang/go/issues/39164 for more detail on this issue.`,
SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)},
}, nil
case strings.Contains(goCmdError, "updates to go.sum needed"), strings.Contains(goCmdError, "missing go.sum entry"):
var args []protocol.DocumentURI
for _, uri := range s.ModFiles() {
args = append(args, protocol.URIFromSpanURI(uri))
}
tidyCmd, err := command.NewTidyCommand("Run go mod tidy", command.URIArgs{URIs: args})
if err != nil {
return nil, err
}
updateCmd, err := command.NewUpdateGoSumCommand("Update go.sum", command.URIArgs{URIs: args})
if err != nil {
return nil, err
}
msg := "go.sum is out of sync with go.mod. Please update it by applying the quick fix."
if innermost != nil {
msg = fmt.Sprintf("go.sum is out of sync with go.mod: entry for %v is missing. Please updating it by applying the quick fix.", innermost)
}
return &source.Diagnostic{
URI: pm.URI,
Range: rng,
Severity: protocol.SeverityError,
Source: source.ListError,
Message: msg,
SuggestedFixes: []source.SuggestedFix{
source.SuggestedFixFromCommand(tidyCmd),
source.SuggestedFixFromCommand(updateCmd),
},
}, nil
case strings.Contains(goCmdError, "disabled by GOPROXY=off") && innermost != nil:
title := fmt.Sprintf("Download %v@%v", innermost.Path, innermost.Version)
cmd, err := command.NewAddDependencyCommand(title, command.DependencyArgs{
URI: protocol.URIFromSpanURI(fh.URI()),
URI: protocol.URIFromSpanURI(pm.URI),
AddRequire: false,
GoCmdArgs: []string{fmt.Sprintf("%v@%v", innermost.Path, innermost.Version)},
})
if err != nil {
return nil
}
msg := goCmdError
if disabledByGOPROXY {
msg = fmt.Sprintf("%v@%v has not been downloaded", innermost.Path, innermost.Version)
return nil, err
}
return &source.Diagnostic{
URI: fh.URI(),
URI: pm.URI,
Range: rng,
Severity: protocol.SeverityError,
Message: msg,
Message: fmt.Sprintf("%v@%v has not been downloaded", innermost.Path, innermost.Version),
Source: source.ListError,
SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)},
}
}
diagSource := source.ListError
if fh != nil {
diagSource = source.ParseError
}
return &source.Diagnostic{
URI: fh.URI(),
Range: rng,
Severity: protocol.SeverityError,
Source: diagSource,
Message: goCmdError,
}, nil
default:
return &source.Diagnostic{
URI: pm.URI,
Range: rng,
Severity: protocol.SeverityError,
Source: source.ListError,
Message: goCmdError,
}, nil
}
}
@ -345,56 +436,3 @@ func findModuleReference(mf *modfile.File, ver module.Version) *modfile.Line {
}
return nil
}
// errorPositionRe matches errors messages of the form <filename>:<line>:<col>,
// where the <col> is optional.
var errorPositionRe = regexp.MustCompile(`(?P<pos>.*:([\d]+)(:([\d]+))?): (?P<msg>.+)`)
// extractErrorWithPosition returns a structured error with position
// information for the given unstructured error. If a file handle is provided,
// the error position will be on that file. This is useful for parse errors,
// where we already know the file with the error.
func extractErrorWithPosition(ctx context.Context, goCmdError string, src source.FileSource) *source.Diagnostic {
matches := errorPositionRe.FindStringSubmatch(strings.TrimSpace(goCmdError))
if len(matches) == 0 {
return nil
}
var pos, msg string
for i, name := range errorPositionRe.SubexpNames() {
if name == "pos" {
pos = matches[i]
}
if name == "msg" {
msg = matches[i]
}
}
spn := span.Parse(pos)
fh, err := src.GetFile(ctx, spn.URI())
if err != nil {
return nil
}
content, err := fh.Read()
if err != nil {
return nil
}
m := &protocol.ColumnMapper{
URI: spn.URI(),
Converter: span.NewContentConverter(spn.URI().Filename(), content),
Content: content,
}
rng, err := m.Range(spn)
if err != nil {
return nil
}
diagSource := source.ListError
if fh != nil {
diagSource = source.ParseError
}
return &source.Diagnostic{
URI: spn.URI(),
Range: rng,
Severity: protocol.SeverityError,
Source: diagSource,
Message: msg,
}
}

View File

@ -151,74 +151,6 @@ func (s *snapshot) ModTidy(ctx context.Context, pm *source.ParsedModule) (*sourc
return mth.tidy(ctx, s)
}
func (s *snapshot) parseModError(ctx context.Context, errText string) []*source.Diagnostic {
// Match on common error messages. This is really hacky, but I'm not sure
// of any better way. This can be removed when golang/go#39164 is resolved.
isInconsistentVendor := strings.Contains(errText, "inconsistent vendoring")
isGoSumUpdates := strings.Contains(errText, "updates to go.sum needed") || strings.Contains(errText, "missing go.sum entry")
if !isInconsistentVendor && !isGoSumUpdates {
return nil
}
switch {
case isInconsistentVendor:
uri := s.ModFiles()[0]
args := command.URIArg{URI: protocol.URIFromSpanURI(uri)}
rng, err := s.uriToModDecl(ctx, uri)
if err != nil {
return nil
}
cmd, err := command.NewVendorCommand("Run go mod vendor", args)
if err != nil {
return nil
}
return []*source.Diagnostic{{
URI: uri,
Range: rng,
Severity: protocol.SeverityError,
Source: source.ListError,
Message: `Inconsistent vendoring detected. Please re-run "go mod vendor".
See https://github.com/golang/go/issues/39164 for more detail on this issue.`,
SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)},
}}
case isGoSumUpdates:
var args []protocol.DocumentURI
for _, uri := range s.ModFiles() {
args = append(args, protocol.URIFromSpanURI(uri))
}
var diagnostics []*source.Diagnostic
for _, uri := range s.ModFiles() {
rng, err := s.uriToModDecl(ctx, uri)
if err != nil {
return nil
}
tidyCmd, err := command.NewTidyCommand("Run go mod tidy", command.URIArgs{URIs: args})
if err != nil {
return nil
}
updateCmd, err := command.NewUpdateGoSumCommand("Update go.sum", command.URIArgs{URIs: args})
if err != nil {
return nil
}
diagnostics = append(diagnostics, &source.Diagnostic{
URI: uri,
Range: rng,
Severity: protocol.SeverityError,
Source: source.ListError,
Message: `go.sum is out of sync with go.mod. Please update it by applying the quick fix.`,
SuggestedFixes: []source.SuggestedFix{
source.SuggestedFixFromCommand(tidyCmd),
source.SuggestedFixFromCommand(updateCmd),
},
})
}
return diagnostics
default:
return nil
}
}
func (s *snapshot) uriToModDecl(ctx context.Context, uri span.URI) (protocol.Range, error) {
fh, err := s.GetFile(ctx, uri)
@ -546,6 +478,14 @@ func missingModuleForImport(snapshot source.Snapshot, m *protocol.ColumnMapper,
}
func rangeFromPositions(m *protocol.ColumnMapper, s, e modfile.Position) (protocol.Range, error) {
spn, err := spanFromPositions(m, s, e)
if err != nil {
return protocol.Range{}, err
}
return m.Range(spn)
}
func spanFromPositions(m *protocol.ColumnMapper, s, e modfile.Position) (span.Span, error) {
toPoint := func(offset int) (span.Point, error) {
l, c, err := m.Converter.ToPosition(offset)
if err != nil {
@ -555,11 +495,11 @@ func rangeFromPositions(m *protocol.ColumnMapper, s, e modfile.Position) (protoc
}
start, err := toPoint(s.Byte)
if err != nil {
return protocol.Range{}, err
return span.Span{}, err
}
end, err := toPoint(e.Byte)
if err != nil {
return protocol.Range{}, err
return span.Span{}, err
}
return m.Range(span.New(m.URI, start, end))
return span.New(m.URI, start, end), nil
}

View File

@ -1022,22 +1022,22 @@ func (s *snapshot) isOpenLocked(uri span.URI) bool {
}
func (s *snapshot) awaitLoaded(ctx context.Context) error {
err := s.awaitLoadedAllErrors(ctx)
loadErr := s.awaitLoadedAllErrors(ctx)
// If we still have absolutely no metadata, check if the view failed to
// initialize and return any errors.
// TODO(rstambler): Should we clear the error after we return it?
s.mu.Lock()
defer s.mu.Unlock()
if len(s.metadata) == 0 {
return err
if len(s.metadata) == 0 && loadErr != nil {
return loadErr.MainError
}
return nil
}
func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError {
loadErr := s.awaitLoadedAllErrors(ctx)
if errors.Is(loadErr, context.Canceled) {
if loadErr != nil && errors.Is(loadErr.MainError, context.Canceled) {
return nil
}
@ -1060,14 +1060,10 @@ func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError {
return nil
}
if strings.Contains(loadErr.Error(), "cannot find main module") {
if strings.Contains(loadErr.MainError.Error(), "cannot find main module") {
return s.workspaceLayoutError(ctx)
}
criticalErr := &source.CriticalError{
MainError: loadErr,
DiagList: s.extractGoCommandErrors(ctx, s, loadErr.Error()),
}
return criticalErr
return loadErr
}
const adHocPackagesWarning = `You are outside of a module and outside of $GOPATH/src.
@ -1095,27 +1091,32 @@ func containsCommandLineArguments(pkgs []source.Package) bool {
return false
}
func (s *snapshot) awaitLoadedAllErrors(ctx context.Context) error {
func (s *snapshot) awaitLoadedAllErrors(ctx context.Context) *source.CriticalError {
// Do not return results until the snapshot's view has been initialized.
s.AwaitInitialized(ctx)
if ctx.Err() != nil {
return ctx.Err()
return &source.CriticalError{MainError: ctx.Err()}
}
if err := s.reloadWorkspace(ctx); err != nil {
return err
diags, _ := s.extractGoCommandErrors(ctx, err.Error())
return &source.CriticalError{
MainError: err,
DiagList: diags,
}
}
if err := s.reloadOrphanedFiles(ctx); err != nil {
return err
diags, _ := s.extractGoCommandErrors(ctx, err.Error())
return &source.CriticalError{
MainError: err,
DiagList: diags,
}
}
// TODO(rstambler): Should we be more careful about returning the
// initialization error? Is it possible for the initialization error to be
// corrected without a successful reinitialization?
if s.initializedErr == nil {
return nil
}
return s.initializedErr.MainError
return s.initializedErr
}
func (s *snapshot) AwaitInitialized(ctx context.Context) {

View File

@ -573,15 +573,15 @@ func (s *snapshot) initialize(ctx context.Context, firstAttempt bool) {
}
if err != nil {
event.Error(ctx, "initial workspace load failed", err)
if modDiagnostics != nil {
s.initializedErr = &source.CriticalError{
MainError: errors.Errorf("errors loading modules: %v: %w", err, modDiagnostics),
DiagList: modDiagnostics,
}
} else {
s.initializedErr = &source.CriticalError{
MainError: err,
}
extractedDiags, _ := s.extractGoCommandErrors(ctx, err.Error())
s.initializedErr = &source.CriticalError{
MainError: err,
DiagList: append(modDiagnostics, extractedDiags...),
}
} else if len(modDiagnostics) != 0 {
s.initializedErr = &source.CriticalError{
MainError: fmt.Errorf("error loading module names"),
DiagList: modDiagnostics,
}
} else {
// Clear out the initialization error, in case it had been set

View File

@ -8,12 +8,9 @@ import (
"io/ioutil"
"os"
"path/filepath"
"strings"
"testing"
"golang.org/x/mod/modfile"
"golang.org/x/tools/internal/lsp/fake"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
)
@ -106,88 +103,6 @@ module fg
}
}
// This tests the logic used to extract positions from parse and other Go
// command errors.
func TestExtractPositionFromError(t *testing.T) {
workspace := `
-- a/go.mod --
modul a.com
-- b/go.mod --
module b.com
go 1.12.hello
-- c/go.mod --
module c.com
require a.com master
`
dir, err := fake.Tempdir(workspace)
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)
tests := []struct {
filename string
wantRng protocol.Range
}{
{
filename: "a/go.mod",
wantRng: protocol.Range{},
},
{
filename: "b/go.mod",
wantRng: protocol.Range{
Start: protocol.Position{Line: 2},
End: protocol.Position{Line: 2},
},
},
{
filename: "c/go.mod",
wantRng: protocol.Range{
Start: protocol.Position{Line: 2},
End: protocol.Position{Line: 2},
},
},
}
for _, test := range tests {
ctx := context.Background()
rel := fake.RelativeTo(dir)
uri := span.URIFromPath(rel.AbsPath(test.filename))
if source.DetectLanguage("", uri.Filename()) != source.Mod {
t.Fatalf("expected only go.mod files")
}
// Try directly parsing the given, invalid go.mod file. Then, extract a
// position from the error message.
src := &osFileSource{}
modFH, err := src.GetFile(ctx, uri)
if err != nil {
t.Fatal(err)
}
content, err := modFH.Read()
if err != nil {
t.Fatal(err)
}
_, parseErr := modfile.Parse(uri.Filename(), content, nil)
if parseErr == nil {
t.Fatalf("%s: expected an unparseable go.mod file", uri.Filename())
}
srcErr := extractErrorWithPosition(ctx, parseErr.Error(), src)
if srcErr == nil {
t.Fatalf("unable to extract positions from %v", parseErr.Error())
}
if srcErr.URI != uri {
t.Errorf("unexpected URI: got %s, wanted %s", srcErr.URI, uri)
}
if protocol.CompareRange(test.wantRng, srcErr.Range) != 0 {
t.Errorf("unexpected range: got %s, wanted %s", srcErr.Range, test.wantRng)
}
if !strings.HasSuffix(parseErr.Error(), srcErr.Message) {
t.Errorf("unexpected message: got %s, wanted %s", srcErr.Message, parseErr)
}
}
}
func TestInVendor(t *testing.T) {
for _, tt := range []struct {
path string

View File

@ -747,17 +747,26 @@ func (e *Editor) Symbol(ctx context.Context, query string) ([]SymbolInformation,
// OrganizeImports requests and performs the source.organizeImports codeAction.
func (e *Editor) OrganizeImports(ctx context.Context, path string) error {
return e.codeAction(ctx, path, nil, nil, protocol.SourceOrganizeImports)
_, err := e.codeAction(ctx, path, nil, nil, protocol.SourceOrganizeImports)
return err
}
// RefactorRewrite requests and performs the source.refactorRewrite codeAction.
func (e *Editor) RefactorRewrite(ctx context.Context, path string, rng *protocol.Range) error {
return e.codeAction(ctx, path, rng, nil, protocol.RefactorRewrite)
applied, err := e.codeAction(ctx, path, rng, nil, protocol.RefactorRewrite)
if applied == 0 {
return errors.Errorf("no refactorings were applied")
}
return err
}
// ApplyQuickFixes requests and performs the quickfix codeAction.
func (e *Editor) ApplyQuickFixes(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic) error {
return e.codeAction(ctx, path, rng, diagnostics, protocol.QuickFix, protocol.SourceFixAll)
applied, err := e.codeAction(ctx, path, rng, diagnostics, protocol.QuickFix, protocol.SourceFixAll)
if applied == 0 {
return errors.Errorf("no quick fixes were applied")
}
return err
}
// GetQuickFixes returns the available quick fix code actions.
@ -765,14 +774,15 @@ func (e *Editor) GetQuickFixes(ctx context.Context, path string, rng *protocol.R
return e.getCodeActions(ctx, path, rng, diagnostics, protocol.QuickFix, protocol.SourceFixAll)
}
func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic, only ...protocol.CodeActionKind) error {
func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic, only ...protocol.CodeActionKind) (int, error) {
actions, err := e.getCodeActions(ctx, path, rng, diagnostics, only...)
if err != nil {
return err
return 0, err
}
applied := 0
for _, action := range actions {
if action.Title == "" {
return errors.Errorf("empty title for code action")
return 0, errors.Errorf("empty title for code action")
}
var match bool
for _, o := range only {
@ -784,6 +794,7 @@ func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Rang
if !match {
continue
}
applied++
for _, change := range action.Edit.DocumentChanges {
path := e.sandbox.Workdir.URIToPath(change.TextDocument.URI)
if int32(e.buffers[path].version) != change.TextDocument.Version {
@ -792,7 +803,7 @@ func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Rang
}
edits := convertEdits(change.Edits)
if err := e.EditBuffer(ctx, path, edits); err != nil {
return errors.Errorf("editing buffer %q: %w", path, err)
return 0, errors.Errorf("editing buffer %q: %w", path, err)
}
}
// Execute any commands. The specification says that commands are
@ -802,15 +813,15 @@ func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Rang
Command: action.Command.Command,
Arguments: action.Command.Arguments,
}); err != nil {
return err
return 0, err
}
}
// Some commands may edit files on disk.
if err := e.sandbox.Workdir.CheckForFileChanges(ctx); err != nil {
return err
return 0, err
}
}
return nil
return applied, nil
}
func (e *Editor) getCodeActions(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic, only ...protocol.CodeActionKind) ([]protocol.CodeAction, error) {