internal/lsp: finish work when synchronous commands complete

Synchronous commands were bypassing the logic to handle the command
error an publish a final workDoneProgressEnd message. Push this logic
down into Server.runCommand.

Also, switch 'generate' to be synchronous, since this makes sense and
causes the synchronous path to be exercised in tests.

Finally, stop defaulting the command title to Command.Name. All Commands
should have Titles.

Change-Id: Id0b1e5283e54d82c307e933782317372bd8971af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/265737
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 959a2a5..328ba92 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -42,10 +42,6 @@
 	if !match {
 		return nil, fmt.Errorf("%s is not a supported command", command.ID())
 	}
-	title := command.Title
-	if title == "" {
-		title = command.Name
-	}
 	// Some commands require that all files are saved to disk. If we detect
 	// unsaved files, warn the user instead of running the commands.
 	unsaved := false
@@ -60,7 +56,7 @@
 		case source.CommandTest.ID(), source.CommandGenerate.ID(), source.CommandToggleDetails.ID():
 			// TODO(PJW): for Toggle, not an error if it is being disabled
 			err := errors.New("unsaved files in the view")
-			s.showCommandError(ctx, title, err)
+			s.showCommandError(ctx, command.Title, err)
 			return nil, err
 		}
 	}
@@ -69,7 +65,7 @@
 	if command.IsSuggestedFix() {
 		err := s.runSuggestedFixCommand(ctx, command, params.Arguments)
 		if err != nil {
-			s.showCommandError(ctx, title, err)
+			s.showCommandError(ctx, command.Title, err)
 		}
 		return nil, err
 	}
@@ -78,25 +74,13 @@
 	// 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, title, "Running...", params.WorkDoneToken, cancel)
+	work := s.progress.start(ctx, command.Title, "Running...", params.WorkDoneToken, cancel)
 	if command.Synchronous {
 		return nil, s.runCommand(ctx, work, command, params.Arguments)
 	}
 	go func() {
 		defer cancel()
-		err := s.runCommand(ctx, work, command, params.Arguments)
-		switch {
-		case errors.Is(err, context.Canceled):
-			work.end(title + ": canceled")
-		case err != nil:
-			event.Error(ctx, fmt.Sprintf("%s: command error", title), err)
-			work.end(title + ": failed")
-			// Show a message when work completes with error, because the progress end
-			// message is typically dismissed immediately by LSP clients.
-			s.showCommandError(ctx, title, err)
-		default:
-			work.end(command.ID() + ": completed")
-		}
+		s.runCommand(ctx, work, command, params.Arguments)
 	}()
 	return nil, nil
 }
@@ -141,7 +125,21 @@
 	}
 }
 
-func (s *Server) runCommand(ctx context.Context, work *workDone, command *source.Command, args []json.RawMessage) error {
+func (s *Server) runCommand(ctx context.Context, work *workDone, command *source.Command, args []json.RawMessage) (err error) {
+	defer func() {
+		switch {
+		case errors.Is(err, context.Canceled):
+			work.end(command.Title + ": canceled")
+		case err != nil:
+			event.Error(ctx, fmt.Sprintf("%s: command error", command.Title), err)
+			work.end(command.Title + ": failed")
+			// Show a message when work completes with error, because the progress end
+			// message is typically dismissed immediately by LSP clients.
+			s.showCommandError(ctx, command.Title, err)
+		default:
+			work.end(command.ID() + ": completed")
+		}
+	}()
 	switch command {
 	case source.CommandTest:
 		var uri protocol.DocumentURI
diff --git a/internal/lsp/source/command.go b/internal/lsp/source/command.go
index 4b85b53..00deb13 100644
--- a/internal/lsp/source/command.go
+++ b/internal/lsp/source/command.go
@@ -81,8 +81,9 @@
 
 	// CommandGenerate runs `go generate` for a given directory.
 	CommandGenerate = &Command{
-		Name:  "generate",
-		Title: "Run go generate",
+		Name:        "generate",
+		Title:       "Run go generate",
+		Synchronous: true,
 	}
 
 	// CommandTidy runs `go mod tidy` for a module.