internal/lsp/regtest: run one quick fix at a time in TestUnknownRevision

There are two possible quick fixes for a missing go.sum entry, and the regression tests always run all available fixes. That never made sense, but I never got around to fixing it because it didn't cause a problem. Now that it turns out to be the cause of the problem described in CL 315152, fix it and roll that CL back. Change-Id: I49430429a99a412f43bd11b30afe8903db99a694 Reviewed-on: https://go-review.googlesource.com/c/tools/+/315910 Trust: Heschi Kreinick <heschi@google.com> Run-TryBot: Heschi Kreinick <heschi@google.com> Reviewed-by: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org>
golang · May 3, 2021 · 42984c4 · 42984c4
1 parent 19b1717
commit 42984c4
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 33 deletions.
diff --git a/gopls/internal/regtest/modfile/modfile_test.go b/gopls/internal/regtest/modfile/modfile_test.go
@@ -519,8 +519,6 @@ func TestUnknownRevision(t *testing.T) {
 -- a/go.mod --
 module mod.com
 
-go 1.16
-
 require (
 	example.com v1.2.2
 )
@@ -556,8 +554,12 @@ func main() {
 					ReadDiagnostics("a/go.mod", &d),
 				),
 			)
-			env.ApplyQuickFixes("a/go.mod", d.Diagnostics)
-			env.SaveBuffer("a/go.mod") // Save to trigger diagnostics.
+			qfs := env.GetQuickFixes("a/go.mod", d.Diagnostics)
+			if len(qfs) == 0 {
+				t.Fatalf("got 0 code actions to fix %v, wanted at least 1", d.Diagnostics)
+			}
+			env.ApplyCodeAction(qfs[0]) // Arbitrarily pick a single fix to apply. Applying all of them seems to cause trouble in this particular test.
+			env.SaveBuffer("a/go.mod")  // Save to trigger diagnostics.
 			env.Await(
 				EmptyDiagnostics("a/go.mod"),
 				env.DiagnosticAtRegexp("a/main.go", "x = "),
@@ -569,8 +571,6 @@ func main() {
 -- a/go.mod --
 module mod.com
 
-go 1.16
-
 require (
 	example.com v1.2.3
 )

diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go
@@ -754,13 +754,13 @@ 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 {
-	_, err := e.codeAction(ctx, path, nil, nil, protocol.SourceOrganizeImports)
+	_, err := e.applyCodeActions(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 {
-	applied, err := e.codeAction(ctx, path, rng, nil, protocol.RefactorRewrite)
+	applied, err := e.applyCodeActions(ctx, path, rng, nil, protocol.RefactorRewrite)
 	if applied == 0 {
 		return errors.Errorf("no refactorings were applied")
 	}
@@ -769,19 +769,46 @@ func (e *Editor) RefactorRewrite(ctx context.Context, path string, rng *protocol
 
 // ApplyQuickFixes requests and performs the quickfix codeAction.
 func (e *Editor) ApplyQuickFixes(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic) error {
-	applied, err := e.codeAction(ctx, path, rng, diagnostics, protocol.QuickFix, protocol.SourceFixAll)
+	applied, err := e.applyCodeActions(ctx, path, rng, diagnostics, protocol.SourceFixAll, protocol.QuickFix)
 	if applied == 0 {
 		return errors.Errorf("no quick fixes were applied")
 	}
 	return err
 }
 
+// ApplyCodeAction applies the given code action.
+func (e *Editor) ApplyCodeAction(ctx context.Context, action protocol.CodeAction) error {
+	for _, change := range action.Edit.DocumentChanges {
+		path := e.sandbox.Workdir.URIToPath(change.TextDocument.URI)
+		if int32(e.buffers[path].version) != change.TextDocument.Version {
+			// Skip edits for old versions.
+			continue
+		}
+		edits := convertEdits(change.Edits)
+		if err := e.EditBuffer(ctx, path, edits); err != nil {
+			return errors.Errorf("editing buffer %q: %w", path, err)
+		}
+	}
+	// Execute any commands. The specification says that commands are
+	// executed after edits are applied.
+	if action.Command != nil {
+		if _, err := e.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{
+			Command:   action.Command.Command,
+			Arguments: action.Command.Arguments,
+		}); err != nil {
+			return err
+		}
+	}
+	// Some commands may edit files on disk.
+	return e.sandbox.Workdir.CheckForFileChanges(ctx)
+}
+
 // GetQuickFixes returns the available quick fix code actions.
 func (e *Editor) GetQuickFixes(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
 	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) (int, error) {
+func (e *Editor) applyCodeActions(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 0, err
@@ -802,29 +829,7 @@ func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Rang
 			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 {
-				// Skip edits for old versions.
-				continue
-			}
-			edits := convertEdits(change.Edits)
-			if err := e.EditBuffer(ctx, path, edits); err != nil {
-				return 0, errors.Errorf("editing buffer %q: %w", path, err)
-			}
-		}
-		// Execute any commands. The specification says that commands are
-		// executed after edits are applied.
-		if action.Command != nil {
-			if _, err := e.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{
-				Command:   action.Command.Command,
-				Arguments: action.Command.Arguments,
-			}); err != nil {
-				return 0, err
-			}
-		}
-		// Some commands may edit files on disk.
-		if err := e.sandbox.Workdir.CheckForFileChanges(ctx); err != nil {
+		if err := e.ApplyCodeAction(ctx, action); err != nil {
 			return 0, err
 		}
 	}

diff --git a/internal/lsp/regtest/wrappers.go b/internal/lsp/regtest/wrappers.go
@@ -201,6 +201,14 @@ func (e *Env) ApplyQuickFixes(path string, diagnostics []protocol.Diagnostic) {
 	}
 }
 
+// ApplyCodeAction applies the given code action.
+func (e *Env) ApplyCodeAction(action protocol.CodeAction) {
+	e.T.Helper()
+	if err := e.Editor.ApplyCodeAction(e.Ctx, action); err != nil {
+		e.T.Fatal(err)
+	}
+}
+
 // GetQuickFixes returns the available quick fix code actions.
 func (e *Env) GetQuickFixes(path string, diagnostics []protocol.Diagnostic) []protocol.CodeAction {
 	e.T.Helper()