internal/lsp: avoid logging context cancellation

This change adds a helper function that checks if the context is
canceled, and if so, doesn't log the error. Tried to use it everywhere
in internal/lsp where it fits, which resulted in changing a few pieces
of error handling throughout.

Updates golang/go#37875

Change-Id: I59cbc6f893e3b70cf84524d9944ff7f4b4febd78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226371
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: stamblerre

internal/lsp/cache/analysis.go

@@ -348,6 +348,10 @@ func runAnalysis(ctx context.Context, fset *token.FileSet, analyzer *analysis.An
 			event.Error(ctx, "unable to compute analysis error position", err, tag.Category.Of(diag.Category), tag.Package.Of(pkg.ID()))
 			continue
 		}
+		if ctx.Err() != nil {
+			data.err = ctx.Err()
+			return data
+		}
 		data.diagnostics = append(data.diagnostics, srcErr)
 	}
 	return data

internal/lsp/cache/check.go

@@ -144,7 +144,9 @@ func (s *snapshot) buildKey(ctx context.Context, id packageID, mode source.Parse
 		depHandle, err := s.buildPackageHandle(ctx, depID, mode)
 		if err != nil {
 			event.Error(ctx, "no dep handle", err, tag.Package.Of(string(depID)))
-
+			if ctx.Err() != nil {
+				return nil, nil, ctx.Err()
+			}
 			// One bad dependency should not prevent us from checking the entire package.
 			// Add a special key to mark a bad dependency.
 			depKeys = append(depKeys, packageHandleKey(fmt.Sprintf("%s import not found", id)))

internal/lsp/cache/errors.go

@@ -60,6 +60,9 @@ func sourceError(ctx context.Context, fset *token.FileSet, pkg *pkg, e interface
 		kind = source.ParseError
 		spn, err = scannerErrorRange(fset, pkg, e.Pos)
 		if err != nil {
+			if ctx.Err() != nil {
+				return nil, ctx.Err()
+			}
 			event.Error(ctx, "no span for scanner.Error pos", err, tag.Package.Of(pkg.ID()))
 			spn = span.Parse(e.Pos.String())
 		}
@@ -73,6 +76,9 @@ func sourceError(ctx context.Context, fset *token.FileSet, pkg *pkg, e interface
 		kind = source.ParseError
 		spn, err = scannerErrorRange(fset, pkg, e[0].Pos)
 		if err != nil {
+			if ctx.Err() != nil {
+				return nil, ctx.Err()
+			}
 			event.Error(ctx, "no span for scanner.Error pos", err, tag.Package.Of(pkg.ID()))
 			spn = span.Parse(e[0].Pos.String())
 		}

internal/lsp/code_action.go

@@ -16,7 +16,6 @@ import (
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/telemetry/event"
-	errors "golang.org/x/xerrors"
 )
 
 func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) {
@@ -44,7 +43,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 		}
 	}
 	if len(wanted) == 0 {
-		return nil, errors.Errorf("no supported code action to execute for %s, wanted %v", uri, params.Context.Only)
+		return nil, fmt.Errorf("no supported code action to execute for %s, wanted %v", uri, params.Context.Only)
 	}
 
 	var codeActions []protocol.CodeAction

internal/lsp/completion.go

@@ -28,9 +28,8 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 	case source.Mod:
 		candidates, surrounding = nil, nil
 	}
-
 	if err != nil {
-		event.Print(ctx, "no completions found", tag.Position.Of(params.Position), event.Err.Of(err))
+		event.Error(ctx, "no completions found", err, tag.Position.Of(params.Position))
 	}
 	if candidates == nil {
 		return &protocol.CompletionList{

internal/lsp/diagnostics.go

@@ -57,14 +57,14 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
 
 	// Diagnose the go.mod file.
 	reports, missingModules, err := mod.Diagnostics(ctx, snapshot)
-	if ctx.Err() != nil {
-		return nil
-	}
 	if err != nil {
 		event.Error(ctx, "warning: diagnose go.mod", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
 	}
-	// Ensure that the reports returned from mod.Diagnostics are only related to the
-	// go.mod file for the module.
+	if ctx.Err() != nil {
+		return nil
+	}
+	// Ensure that the reports returned from mod.Diagnostics are only related
+	// to the go.mod file for the module.
 	if len(reports) > 1 {
 		panic("unexpected reports from mod.Diagnostics")
 	}
@@ -81,9 +81,6 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
 
 	// Diagnose all of the packages in the workspace.
 	wsPackages, err := snapshot.WorkspacePackages(ctx)
-	if ctx.Err() != nil {
-		return nil
-	}
 	if err != nil {
 		event.Error(ctx, "failed to load workspace packages, skipping diagnostics", err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder))
 		return nil
@@ -107,9 +104,6 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
 					Message: `You are neither in a module nor in your GOPATH. If you are using modules, please open your editor at the directory containing the go.mod. If you believe this warning is incorrect, please file an issue: https://github.com/golang/go/issues/new.`,
 				})
 			}
-			if ctx.Err() != nil {
-				return
-			}
 			if err != nil {
 				event.Error(ctx, "warning: diagnose package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(ph.ID()))
 				return
@@ -185,9 +179,7 @@ func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, r
 			URI:         protocol.URIFromSpanURI(key.id.URI),
 			Version:     key.id.Version,
 		}); err != nil {
-			if ctx.Err() == nil {
-				event.Error(ctx, "publishReports: failed to deliver diagnostic", err, tag.URI.Of(key.id.URI))
-			}
+			event.Error(ctx, "publishReports: failed to deliver diagnostic", err, tag.URI.Of(key.id.URI))
 			continue
 		}
 		// Update the delivered map.

internal/lsp/link.go

@@ -56,13 +56,13 @@ func modLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandl
 		// dependency within the require statement.
 		start, end := token.Pos(s+i), token.Pos(s+i+len(dep))
 		target := fmt.Sprintf("https://%s/mod/%s", view.Options().LinkTarget, req.Mod.String())
-		if l, err := toProtocolLink(view, m, target, start, end, source.Mod); err == nil {
-			links = append(links, l)
-		} else {
-			event.Error(ctx, "failed to create protocol link", err)
+		l, err := toProtocolLink(view, m, target, start, end, source.Mod)
+		if err != nil {
+			return nil, err
 		}
+		links = append(links, l)
 	}
-	// TODO(ridersofrohan): handle links for replace and exclude directives
+	// TODO(ridersofrohan): handle links for replace and exclude directives.
 	if syntax := file.Syntax; syntax == nil {
 		return links, nil
 	}
@@ -110,11 +110,12 @@ func goLinks(ctx context.Context, view source.View, fh source.FileHandle) ([]pro
 				target = fmt.Sprintf("https://%s/%s", view.Options().LinkTarget, target)
 				// Account for the quotation marks in the positions.
 				start, end := n.Path.Pos()+1, n.Path.End()-1
-				if l, err := toProtocolLink(view, m, target, start, end, source.Go); err == nil {
-					links = append(links, l)
-				} else {
-					event.Error(ctx, "failed to create protocol link", err)
+				l, err := toProtocolLink(view, m, target, start, end, source.Go)
+				if err != nil {
+					event.Error(ctx, "failed to create link", err)
+					return false
 				}
+				links = append(links, l)
 			}
 			return false
 		case *ast.BasicLit:

internal/lsp/protocol/context.go

@@ -19,6 +19,9 @@ func WithClient(ctx context.Context, client Client) context.Context {
 }
 
 func LogEvent(ctx context.Context, ev event.Event, tags event.TagMap) context.Context {
+	if ctx.Err() != nil {
+		return ctx
+	}
 	if !ev.IsLog() {
 		return ctx
 	}

internal/lsp/source/completion_format.go

@@ -247,9 +247,7 @@ func (c *completer) formatBuiltin(cand candidate) CompletionItem {
 		item.Kind = protocol.FunctionCompletion
 		astObj, err := c.snapshot.View().LookupBuiltin(c.ctx, obj.Name())
 		if err != nil {
-			if c.ctx.Err() == nil {
-				event.Error(c.ctx, "no builtin package", err)
-			}
+			event.Error(c.ctx, "no builtin package", err)
 			break
 		}
 		decl, ok := astObj.Decl.(*ast.FuncDecl)

internal/lsp/source/diagnostics.go

@@ -131,10 +131,10 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missi
 		analyzers = snapshot.View().Options().TypeErrorAnalyzers
 	}
 	if err := analyses(ctx, snapshot, reports, ph, analyzers); err != nil {
+		event.Error(ctx, "analyses failed", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(ph.ID()))
 		if ctx.Err() != nil {
 			return nil, warn, ctx.Err()
 		}
-		event.Error(ctx, "failed to run analyses", err, tag.Package.Of(ph.ID()))
 	}
 	return reports, warn, nil
 }
@@ -227,7 +227,6 @@ func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports m
 	}
 	file, _, m, _, err := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseHeader).Parse(ctx)
 	if err != nil {
-		event.Error(ctx, "could not parse go file when checking for missing modules", err)
 		return err
 	}
 	// Make a dependency->import map to improve performance when finding missing dependencies.

internal/lsp/source/highlight.go

@@ -40,7 +40,7 @@ func Highlight(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protoc
 	}
 	path, _ := astutil.PathEnclosingInterval(file, rng.Start, rng.Start)
 	if len(path) == 0 {
-		return nil, errors.Errorf("no enclosing position found for %v:%v", int(pos.Line), int(pos.Character))
+		return nil, fmt.Errorf("no enclosing position found for %v:%v", int(pos.Line), int(pos.Character))
 	}
 	// If start==end for astutil.PathEnclosingInterval, the 1-char interval following start is used instead.
 	// As a result, we might not get an exact match so we should check the 1-char interval to the left of the
@@ -147,10 +147,9 @@ Outer:
 	if resultsList != nil && -1 < index && index < len(resultsList.List) {
 		rng, err := nodeToProtocolRange(view, pkg, resultsList.List[index])
 		if err != nil {
-			event.Error(ctx, "Error getting range for node", err)
-		} else {
-			result[rng] = true
+			return nil, err
 		}
+		result[rng] = true
 	}
 	// Add the "func" part of the func declaration.
 	if highlightAllReturnsAndFunc {
@@ -185,9 +184,9 @@ Outer:
 				rng, err := nodeToProtocolRange(view, pkg, toAdd)
 				if err != nil {
 					event.Error(ctx, "Error getting range for node", err)
-				} else {
-					result[rng] = true
+					return false
 				}
+				result[rng] = true
 				return false
 			}
 		}
@@ -268,11 +267,12 @@ func highlightImportUses(ctx context.Context, view View, pkg Package, path []ast
 		if !strings.Contains(basicLit.Value, obj.Name()) {
 			return true
 		}
-		if rng, err := nodeToProtocolRange(view, pkg, n); err == nil {
-			result[rng] = true
-		} else {
+		rng, err := nodeToProtocolRange(view, pkg, n)
+		if err != nil {
 			event.Error(ctx, "Error getting range for node", err)
+			return false
 		}
+		result[rng] = true
 		return false
 	})
 	return rangeMapToSlice(result), nil
@@ -311,11 +311,12 @@ func highlightIdentifiers(ctx context.Context, view View, pkg Package, path []as
 		if nObj := pkg.GetTypesInfo().ObjectOf(n); nObj != idObj {
 			return false
 		}
-		if rng, err := nodeToProtocolRange(view, pkg, n); err == nil {
-			result[rng] = true
-		} else {
+		rng, err := nodeToProtocolRange(view, pkg, n)
+		if err != nil {
 			event.Error(ctx, "Error getting range for node", err)
+			return false
 		}
+		result[rng] = true
 		return false
 	})
 	return rangeMapToSlice(result), nil