gopls/internal/cache: failure to extract diagnostic fixes is an error

Although the LSP specifies that diagnostic data should be transmitted
unmodified to codeAction requests, we can't control whether clients
abide by this rule. Therefore, a failure to extract fixes should be
treated as a user-facing error, not a bug.

Fixes golang/go#68819

Change-Id: I57e629cf381ee1112d98d22e728449995679b05f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/628237
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

gopls/internal/cache/diagnostics.go

@@ -191,28 +191,26 @@ func bundleLazyFixes(sd *Diagnostic) bool {
 
 // BundledLazyFixes extracts any bundled codeActions from the
 // diag.Data field.
-func BundledLazyFixes(diag protocol.Diagnostic) []protocol.CodeAction {
+func BundledLazyFixes(diag protocol.Diagnostic) ([]protocol.CodeAction, error) {
 	var fix lazyFixesJSON
 	if diag.Data != nil {
 		err := protocol.UnmarshalJSON(*diag.Data, &fix)
 		if err != nil {
-			bug.Reportf("unmarshalling lazy fix: %v", err)
-			return nil
+			return nil, fmt.Errorf("unmarshalling fix from diagnostic data: %v", err)
 		}
 	}
 
 	var actions []protocol.CodeAction
 	for _, action := range fix.Actions {
 		// See bundleLazyFixes: for now we only support bundling commands.
 		if action.Edit != nil {
-			bug.Reportf("bundled fix %q includes workspace edits", action.Title)
-			continue
+			return nil, fmt.Errorf("bundled fix %q includes workspace edits", action.Title)
 		}
 		// associate the action with the incoming diagnostic
 		// (Note that this does not mutate the fix.Fixes slice).
 		action.Diagnostics = []protocol.Diagnostic{diag}
 		actions = append(actions, action)
 	}
 
-	return actions
+	return actions, nil
 }

gopls/internal/server/code_action.go

@@ -265,7 +265,10 @@ func (s *server) codeActionsMatchingDiagnostics(ctx context.Context, uri protoco
 	var actions []protocol.CodeAction
 	var unbundled []protocol.Diagnostic // diagnostics without bundled code actions in their Data field
 	for _, pd := range pds {
-		bundled := cache.BundledLazyFixes(pd)
+		bundled, err := cache.BundledLazyFixes(pd)
+		if err != nil {
+			return nil, err
+		}
 		if len(bundled) > 0 {
 			for _, fix := range bundled {
 				if enabled(fix.Kind) {