internal/lsp: ignore AST errors when generating import edits
go/parser.ParseFile can return both an AST and errors. We should still
be able to do import organization even if the AST contains errors, as
long as they are below the portion of the file that contains the import
block.
Change-Id: Id6b86171fca3e15d02910d1c6f4ce25e803754d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207261
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index af1131c..82230c9 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -19,7 +19,6 @@
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/snippet"
"golang.org/x/tools/internal/span"
- "golang.org/x/tools/internal/telemetry/log"
"golang.org/x/tools/internal/telemetry/trace"
errors "golang.org/x/xerrors"
)
@@ -350,8 +349,6 @@
if !c.inDeepCompletion() || c.deepState.isHighScore(cand.score) {
if item, err := c.item(cand); err == nil {
c.items = append(c.items, item)
- } else {
- log.Error(c.ctx, "error generating completion item", err)
}
}
}
diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go
index 1d5b3c2..351b1a2 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -11,6 +11,7 @@
"go/ast"
"go/format"
"go/parser"
+ "go/scanner"
"go/token"
"golang.org/x/tools/internal/imports"
@@ -219,6 +220,7 @@
}
fixedFset := token.NewFileSet()
fixedAST, err := parser.ParseFile(fixedFset, filename, fixedData, parser.ImportsOnly)
+ // Any error here prevents us from computing the edits.
if err != nil {
return nil, err
}
@@ -236,13 +238,17 @@
// first non-imports decl. We know the imports code will insert
// somewhere before that.
if origImportOffset == 0 || fixedImportsOffset == 0 {
- left = trimToFirstNonImport(view.Session().Cache().FileSet(), origAST, origData)
+ left, _ = trimToFirstNonImport(view.Session().Cache().FileSet(), origAST, origData, nil)
// We need the whole AST here, not just the ImportsOnly AST we parsed above.
fixedAST, err = parser.ParseFile(fixedFset, filename, fixedData, 0)
- if err != nil {
+ if fixedAST == nil {
return nil, err
}
- right = trimToFirstNonImport(fixedFset, fixedAST, fixedData)
+ var ok bool
+ right, ok = trimToFirstNonImport(fixedFset, fixedAST, fixedData, err)
+ if !ok {
+ return nil, errors.Errorf("error %v detected in the import block", err)
+ }
// We're now working with a prefix of the original file, so we can
// use the original converter, and there is no offset on the edits.
converter = origMapper.Converter
@@ -287,7 +293,7 @@
// trimToFirstNonImport returns src from the beginning to the first non-import
// declaration, or the end of the file if there is no such decl.
-func trimToFirstNonImport(fset *token.FileSet, f *ast.File, src []byte) []byte {
+func trimToFirstNonImport(fset *token.FileSet, f *ast.File, src []byte, err error) ([]byte, bool) {
var firstDecl ast.Decl
for _, decl := range f.Decls {
if gen, ok := decl.(*ast.GenDecl); ok && gen.Tok == token.IMPORT {
@@ -296,12 +302,30 @@
firstDecl = decl
break
}
-
+ tok := fset.File(f.Pos())
+ if tok == nil {
+ return nil, false
+ }
end := f.End()
if firstDecl != nil {
- end = fset.File(f.Pos()).LineStart(fset.Position(firstDecl.Pos()).Line - 1)
+ end = tok.LineStart(fset.Position(firstDecl.Pos()).Line - 1)
}
- return src[0:fset.Position(end).Offset]
+ // Any errors in the file must be after the part of the file that we care about.
+ switch err := err.(type) {
+ case *scanner.Error:
+ pos := tok.Pos(err.Pos.Offset)
+ if pos <= end {
+ return nil, false
+ }
+ case scanner.ErrorList:
+ if err.Len() > 0 {
+ pos := tok.Pos(err[0].Pos.Offset)
+ if pos <= end {
+ return nil, false
+ }
+ }
+ }
+ return src[0:fset.Position(end).Offset], true
}
// CandidateImports returns every import that could be added to filename.
