From 6316571e2c520e22a61c7aae2a2d05538f2966d7 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Sun, 12 Jan 2020 18:14:55 -0500 Subject: [PATCH] internal/lsp: don't require type-checking for formatting We had previously required a type-checked package for formatting requests, in order to determine if the package contained any parse errors. We can get this information directly from the ParseGoHandle, so there is no need to check the package. This will prevent `go list` errors from making their way into formatting requests, for which a `go list` really is not needed. Updates golang/go#36511 Change-Id: I297f8880367e800d2343f468535efa80d2937071 Reviewed-on: https://go-review.googlesource.com/c/tools/+/214421 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/source/format.go | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 406c795f6d..7d10981361 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -8,7 +8,6 @@ package source import ( "bytes" "context" - "fmt" "go/ast" "go/format" "go/parser" @@ -28,25 +27,15 @@ func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.T ctx, done := trace.StartSpan(ctx, "source.Format") defer done() - pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestCheckPackageHandle) - if err != nil { - return nil, fmt.Errorf("getting file for Format: %v", err) - } - - // Be extra careful that the file's ParseMode is correct, - // otherwise we might replace the user's code with a trimmed AST. - if pgh.Mode() != ParseFull { - return nil, errors.Errorf("%s was parsed in the incorrect mode", pgh.File().Identity().URI) - } - file, m, _, err := pgh.Parse(ctx) + pgh := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseFull) + file, m, parseErrors, err := pgh.Parse(ctx) if err != nil { return nil, err } - if hasListErrors(pkg) || hasParseErrors(pkg, fh.Identity().URI) { - // Even if this package has list or parse errors, this file may not - // have any parse errors and can still be formatted. Using format.Node - // on an ast with errors may result in code being added or removed. - // Attempt to format the source of this file instead. + // Even if this file has parse errors, it might still be possible to format it. + // Using format.Node on an AST with errors may result in code being modified. + // Attempt to format the source of this file instead. + if parseErrors != nil { formatted, err := formatSource(ctx, snapshot, fh) if err != nil { return nil, err @@ -303,16 +292,6 @@ func trimToFirstNonImport(fset *token.FileSet, f *ast.File, src []byte, err erro return src[0:fset.Position(end).Offset], true } -// hasParseErrors returns true if the given file has parse errors. -func hasParseErrors(pkg Package, uri span.URI) bool { - for _, e := range pkg.GetErrors() { - if e.File.URI == uri && e.Kind == ParseError { - return true - } - } - return false -} - func hasListErrors(pkg Package) bool { for _, err := range pkg.GetErrors() { if err.Kind == ListError {