internal/lsp/source: optimize computeFixEdits
In the common case that a file has imports, we're going to diff just the
import block. That means that ApplyFixes doesn't need to produce the
whole formatted file, which is a huge speedup. We will do more work twice
for files with no imports, but those are presumably pretty short.
Hat tip to Muir for pointing towards this in
https://go-review.googlesource.com/c/tools/+/209579/2/internal/imports/imports.go#87
even if I didn't catch it.
Updates golang/go#36001.
Change-Id: Ibbeb4d88c6505eac26a36994de514813606c8c79
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210200
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/imports/imports.go b/internal/imports/imports.go
index 7c1b475..e066d90 100644
--- a/internal/imports/imports.go
+++ b/internal/imports/imports.go
@@ -83,8 +83,9 @@
return getFixes(fileSet, file, filename, opt.Env)
}
-// ApplyFix will apply all of the fixes to the file and format it.
-func ApplyFixes(fixes []*ImportFix, filename string, src []byte, opt *Options) (formatted []byte, err error) {
+// ApplyFixes applies all of the fixes to the file and formats it. extraMode
+// is added in when parsing the file.
+func ApplyFixes(fixes []*ImportFix, filename string, src []byte, opt *Options, extraMode parser.Mode) (formatted []byte, err error) {
src, opt, err = initialize(filename, src, opt)
if err != nil {
return nil, err
@@ -100,6 +101,8 @@
if opt.AllErrors {
parserMode |= parser.AllErrors
}
+ parserMode |= extraMode
+
file, err := parser.ParseFile(fileSet, filename, src, parserMode)
if file == nil {
return nil, err
diff --git a/internal/imports/imports_test.go b/internal/imports/imports_test.go
index 2288873..1901e6a 100644
--- a/internal/imports/imports_test.go
+++ b/internal/imports/imports_test.go
@@ -71,7 +71,7 @@
t.Errorf("%s: %s", test.name, err.Error())
}
- got, err = ApplyFixes(fixes, "", []byte(input), test.opt)
+ got, err = ApplyFixes(fixes, "", []byte(input), test.opt, 0)
if err != nil {
t.Errorf("%s: %s", test.name, err.Error())
}
diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go
index 0e49f9a..35b7981 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -93,7 +93,7 @@
pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle)
if err != nil {
- return nil, nil, fmt.Errorf("getting file for AllImportsFixes: %v", err)
+ return nil, nil, errors.Errorf("getting file for AllImportsFixes: %v", err)
}
if hasListErrors(pkg) {
return nil, nil, errors.Errorf("%s has list errors, not running goimports", f.URI())
@@ -112,7 +112,7 @@
return err
}, options)
if err != nil {
- return nil, nil, err
+ return nil, nil, errors.Errorf("computing fix edits: %v", err)
}
return allFixEdits, editsPerFix, nil
@@ -186,7 +186,8 @@
filename := ph.File().Identity().URI.Filename()
// Apply the fixes and re-parse the file so that we can locate the
// new imports.
- fixedData, err := imports.ApplyFixes(fixes, filename, origData, options)
+ fixedData, err := imports.ApplyFixes(fixes, filename, origData, options, parser.ImportsOnly)
+ fixedData = append(fixedData, '\n') // ApplyFixes comes out missing the newline, go figure.
if err != nil {
return nil, err
}
@@ -211,7 +212,11 @@
// somewhere before that.
if origImportOffset == 0 || fixedImportsOffset == 0 {
left, _ = trimToFirstNonImport(view.Session().Cache().FileSet(), origAST, origData, nil)
- // We need the whole AST here, not just the ImportsOnly AST we parsed above.
+ fixedData, err = imports.ApplyFixes(fixes, filename, origData, options, 0)
+ if err != nil {
+ return nil, err
+ }
+ // We need the whole file here, not just the ImportsOnly versions we made above.
fixedAST, err = parser.ParseFile(fixedFset, filename, fixedData, 0)
if fixedAST == nil {
return nil, err
