internal/lsp: propagate errors from find references

We were ignoring errors in a few cases and returning incorrect errors in
the case of the command-line interface (no identifier found when there
were just no references). I think it is better for references to return
an error than incomplete results.

Change-Id: Id90bca58ebdd9f6a910853cb4ac5b6ab6bec57f8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

Author: stamblerre

internal/lsp/cmd/references.go

@@ -71,10 +71,6 @@ func (r *references) Run(ctx context.Context, args ...string) error {
 	if err != nil {
 		return err
 	}
-	if len(locations) == 0 {
-		return tool.CommandLineErrorf("%v: not an identifier", from)
-	}
-
 	var spans []string
 	for _, l := range locations {
 		f := conn.AddFile(ctx, span.NewURI(l.URI))

internal/lsp/references.go

@@ -9,10 +9,7 @@ import (
 
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
-	"golang.org/x/tools/internal/lsp/telemetry"
 	"golang.org/x/tools/internal/span"
-	"golang.org/x/tools/internal/telemetry/log"
-	"golang.org/x/tools/internal/telemetry/tag"
 )
 
 func (s *Server) references(ctx context.Context, params *protocol.ReferenceParams) ([]protocol.Location, error) {
@@ -44,19 +41,14 @@ func (s *Server) references(ctx context.Context, params *protocol.ReferenceParam
 	for _, ph := range phs {
 		ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.SpecificPackageHandle(ph.ID()))
 		if err != nil {
-			if err == source.ErrNoIdentFound {
-				return nil, err
-			}
-			log.Error(ctx, "no identifier", err, telemetry.URI.Of(uri))
-			continue
+			return nil, err
 		}
 
 		lastIdent = ident
 
 		references, err := ident.References(ctx)
 		if err != nil {
-			log.Error(ctx, "no references", err, tag.Of("Identifier", ident.Name))
-			continue
+			return nil, err
 		}
 
 		for _, ref := range references {

internal/lsp/source/references.go

@@ -10,8 +10,6 @@ import (
 	"go/types"
 
 	"golang.org/x/tools/go/types/objectpath"
-	"golang.org/x/tools/internal/lsp/telemetry"
-	"golang.org/x/tools/internal/telemetry/log"
 	"golang.org/x/tools/internal/telemetry/trace"
 	errors "golang.org/x/xerrors"
 )
@@ -50,13 +48,11 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro
 		for _, id := range reverseDeps {
 			ph, err := i.Snapshot.PackageHandle(ctx, id)
 			if err != nil {
-				log.Error(ctx, "References: no CheckPackageHandle", err, telemetry.Package.Of(id))
-				continue
+				return nil, err
 			}
 			pkg, err := ph.Check(ctx)
 			if err != nil {
-				log.Error(ctx, "References: no Package", err, telemetry.Package.Of(id))
-				continue
+				return nil, err
 			}
 			searchpkgs = append(searchpkgs, pkg)
 		}