From e5b8258f49188ec2c4702802a35d1ea0c7ccee77 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 5 Apr 2019 18:05:31 -0400 Subject: [PATCH] internal/lsp: stop formatting on files that do not parse This change will stop formatting from working on any file that does not parse. This is a temporary fix to handle the formatting problems mentioned here: https://github.com/Microsoft/vscode-go/issues/2410, but is not a long-term solution. Change-Id: Ie34b1876519832d6859db95fdcad7cc37a20b769 Reviewed-on: https://go-review.googlesource.com/c/tools/+/171019 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/source/format.go | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index bdcf64af..cc7d72a1 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -9,11 +9,11 @@ import ( "bytes" "context" "fmt" - "go/ast" "go/format" "strings" "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/go/packages" "golang.org/x/tools/imports" "golang.org/x/tools/internal/lsp/diff" "golang.org/x/tools/internal/span" @@ -21,28 +21,16 @@ import ( // Format formats a file with a given range. func Format(ctx context.Context, f File, rng span.Range) ([]TextEdit, error) { + pkg := f.GetPackage(ctx) + if hasParseErrors(pkg.GetErrors()) { + return nil, fmt.Errorf("%s has parse errors, not formatting", f.URI()) + } fAST := f.GetAST(ctx) path, exact := astutil.PathEnclosingInterval(fAST, rng.Start, rng.End) if !exact || len(path) == 0 { return nil, fmt.Errorf("no exact AST node matching the specified range") } node := path[0] - // format.Node can fail when the AST contains a bad expression or - // statement. For now, we preemptively check for one. - // TODO(rstambler): This should really return an error from format.Node. - var isBad bool - ast.Inspect(node, func(n ast.Node) bool { - switch n.(type) { - case *ast.BadDecl, *ast.BadExpr, *ast.BadStmt: - isBad = true - return false - default: - return true - } - }) - if isBad { - return nil, fmt.Errorf("unable to format file due to a badly formatted AST") - } // format.Node changes slightly from one release to another, so the version // of Go used to build the LSP server will determine how it formats code. // This should be acceptable for all users, who likely be prompted to rebuild @@ -55,6 +43,15 @@ func Format(ctx context.Context, f File, rng span.Range) ([]TextEdit, error) { return computeTextEdits(ctx, f, buf.String()), nil } +func hasParseErrors(errors []packages.Error) bool { + for _, err := range errors { + if err.Kind == packages.ParseError { + return true + } + } + return false +} + // Imports formats a file using the goimports tool. func Imports(ctx context.Context, f File, rng span.Range) ([]TextEdit, error) { formatted, err := imports.Process(f.GetToken(ctx).Name(), f.GetContent(ctx), nil)