internal/lsp/source: synchronous commands the default

This makes all commands with the exception of running tests synchronous.

Remove the special handling for applying suggested fixes. Instead, make
workDone reporting a no-op if the workDone handle is nil.

Change-Id: I6801c91f8564d68b8bdaaf9456cd6752b164d0b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265583
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 398b2a5..2603a51 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -60,29 +60,26 @@
 			return nil, err
 		}
 	}
-	// If the command has a suggested fix function available, use it and apply
-	// the edits to the workspace.
-	if command.IsSuggestedFix() {
-		err := s.runSuggestedFixCommand(ctx, command, params.Arguments)
-		if err != nil {
-			s.showCommandError(ctx, command.Title, err)
-		}
-		return nil, err
-	}
 	ctx, cancel := context.WithCancel(xcontext.Detach(ctx))
-	// Start progress prior to spinning off a goroutine specifically so that
-	// clients are aware of the work item before the command completes. This
-	// matters for regtests, where having a continuous thread of work is
-	// convenient for assertions.
-	work := s.progress.start(ctx, command.Title, "Running...", params.WorkDoneToken, cancel)
-	if command.Synchronous {
-		return nil, s.runCommand(ctx, work, command, params.Arguments)
+
+	var work *workDone
+	// Don't show progress for suggested fixes. They should be quick.
+	if !command.IsSuggestedFix() {
+		// Start progress prior to spinning off a goroutine specifically so that
+		// clients are aware of the work item before the command completes. This
+		// matters for regtests, where having a continuous thread of work is
+		// convenient for assertions.
+		work = s.progress.start(ctx, command.Title, "Running...", params.WorkDoneToken, cancel)
 	}
-	go func() {
-		defer cancel()
-		s.runCommand(ctx, work, command, params.Arguments)
-	}()
-	return nil, nil
+	if command.Async {
+		go func() {
+			defer cancel()
+			s.runCommand(ctx, work, command, params.Arguments)
+		}()
+		return nil, nil
+	}
+	defer cancel()
+	return nil, s.runCommand(ctx, work, command, params.Arguments)
 }
 
 func (s *Server) runSuggestedFixCommand(ctx context.Context, command *source.Command, args []json.RawMessage) error {
@@ -140,6 +137,11 @@
 			work.end(command.ID() + ": completed")
 		}
 	}()
+	// If the command has a suggested fix function available, use it and apply
+	// the edits to the workspace.
+	if command.IsSuggestedFix() {
+		return s.runSuggestedFixCommand(ctx, command, args)
+	}
 	switch command {
 	case source.CommandTest:
 		var uri protocol.DocumentURI
diff --git a/internal/lsp/progress.go b/internal/lsp/progress.go
index d444ad1..4d18fd8 100644
--- a/internal/lsp/progress.go
+++ b/internal/lsp/progress.go
@@ -162,6 +162,9 @@
 
 // report reports an update on WorkDone report back to the client.
 func (wd *workDone) report(message string, percentage float64) {
+	if wd == nil {
+		return
+	}
 	wd.cancelMu.Lock()
 	cancelled := wd.cancelled
 	wd.cancelMu.Unlock()
@@ -192,6 +195,9 @@
 
 // end reports a workdone completion back to the client.
 func (wd *workDone) end(message string) {
+	if wd == nil {
+		return
+	}
 	var err error
 	switch {
 	case wd.err != nil:
diff --git a/internal/lsp/source/command.go b/internal/lsp/source/command.go
index 00deb13..8467cf4 100644
--- a/internal/lsp/source/command.go
+++ b/internal/lsp/source/command.go
@@ -23,9 +23,8 @@
 	Title string
 	Name  string
 
-	// Synchronous controls whether the command executes synchronously within the
-	// ExecuteCommand request (applying suggested fixes is always synchronous).
-	Synchronous bool
+	// Async controls whether the command executes asynchronously.
+	Async bool
 
 	// appliesFn is an optional field to indicate whether or not a command can
 	// be applied to the given inputs. If it returns false, we should not
@@ -77,13 +76,13 @@
 	CommandTest = &Command{
 		Name:  "test",
 		Title: "Run test(s)",
+		Async: true,
 	}
 
 	// CommandGenerate runs `go generate` for a given directory.
 	CommandGenerate = &Command{
-		Name:        "generate",
-		Title:       "Run go generate",
-		Synchronous: true,
+		Name:  "generate",
+		Title: "Run go generate",
 	}
 
 	// CommandTidy runs `go mod tidy` for a module.
@@ -156,9 +155,8 @@
 
 	// CommandGenerateGoplsMod (re)generates the gopls.mod file.
 	CommandGenerateGoplsMod = &Command{
-		Name:        "generate_gopls_mod",
-		Title:       "Generate gopls.mod",
-		Synchronous: true,
+		Name:  "generate_gopls_mod",
+		Title: "Generate gopls.mod",
 	}
 )