internal/lsp: use the AST to get correct ranges

Fixes golang/go#29150

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

Author: stamblerre

internal/lsp/cache/errors.go

@@ -3,15 +3,19 @@ package cache
 import (
 	"bytes"
 	"context"
+	"go/ast"
 	"go/scanner"
+	"go/token"
 	"go/types"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/span"
+	errors "golang.org/x/xerrors"
 )
 
 func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, error) {
@@ -23,6 +27,7 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e
 		fixes         []source.SuggestedFix
 		related       []source.RelatedInformation
 	)
+	fset := pkg.snapshot.view.session.cache.fset
 	switch e := e.(type) {
 	case packages.Error:
 		if e.Pos == "" {
@@ -32,21 +37,32 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e
 		}
 		msg = e.Msg
 		kind = toSourceErrorKind(e.Kind)
+
 	case *scanner.Error:
 		msg = e.Msg
 		kind = source.ParseError
-		spn = span.Parse(e.Pos.String())
+		spn, err = scannerErrorRange(ctx, fset, pkg, e.Pos)
+		if err != nil {
+			return nil, err
+		}
+
 	case scanner.ErrorList:
 		// The first parser error is likely the root cause of the problem.
-		if e.Len() > 0 {
-			spn = span.Parse(e[0].Pos.String())
-			msg = e[0].Msg
-			kind = source.ParseError
+		if e.Len() <= 0 {
+			return nil, errors.Errorf("no errors in %v", e)
 		}
+		msg = e[0].Msg
+		kind = source.ParseError
+		spn, err = scannerErrorRange(ctx, fset, pkg, e[0].Pos)
+		if err != nil {
+			return nil, err
+		}
+
 	case types.Error:
-		spn = span.Parse(pkg.snapshot.view.session.cache.fset.Position(e.Pos).String())
 		msg = e.Msg
 		kind = source.TypeError
+		spn, err = typeErrorRange(ctx, fset, pkg, e.Pos)
+
 	case *analysis.Diagnostic:
 		spn, err = span.NewRange(pkg.snapshot.view.session.cache.fset, e.Pos, e.End).Span()
 		if err != nil {
@@ -64,7 +80,7 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e
 			return nil, err
 		}
 	}
-	rng, err := spanToRange(ctx, pkg, spn, kind == source.TypeError)
+	rng, err := spanToRange(ctx, pkg, spn)
 	if err != nil {
 		return nil, err
 	}
@@ -88,7 +104,7 @@ func suggestedFixes(ctx context.Context, pkg *pkg, diag *analysis.Diagnostic) ([
 			if err != nil {
 				return nil, err
 			}
-			rng, err := spanToRange(ctx, pkg, spn, false)
+			rng, err := spanToRange(ctx, pkg, spn)
 			if err != nil {
 				return nil, err
 			}
@@ -112,7 +128,7 @@ func relatedInformation(ctx context.Context, pkg *pkg, diag *analysis.Diagnostic
 		if err != nil {
 			return nil, err
 		}
-		rng, err := spanToRange(ctx, pkg, spn, false)
+		rng, err := spanToRange(ctx, pkg, spn)
 		if err != nil {
 			return nil, err
 		}
@@ -138,32 +154,81 @@ func toSourceErrorKind(kind packages.ErrorKind) source.ErrorKind {
 	}
 }
 
+func typeErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, pos token.Pos) (span.Span, error) {
+	spn, err := span.NewRange(fset, pos, pos).Span()
+	if err != nil {
+		return span.Span{}, err
+	}
+	posn := fset.Position(pos)
+	ph, _, err := pkg.FindFile(ctx, span.FileURI(posn.Filename))
+	if err != nil {
+		return span.Span{}, err
+	}
+	file, m, _, err := ph.Cached(ctx)
+	if err != nil {
+		return span.Span{}, err
+	}
+	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
+	if len(path) > 0 {
+		if s, err := span.NewRange(fset, path[0].Pos(), path[0].End()).Span(); err == nil {
+			return s, nil
+		}
+	}
+	s, err := spn.WithOffset(m.Converter)
+	if err != nil {
+		return span.Span{}, err
+	}
+	data, _, err := ph.File().Read(ctx)
+	if err != nil {
+		return span.Span{}, err
+	}
+	start := s.Start()
+	offset := start.Offset()
+	if offset < len(data) {
+		if width := bytes.IndexAny(data[offset:], " \n,():;[]"); width > 0 {
+			return span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+width, offset+width)), nil
+		}
+	}
+	return spn, nil
+}
+
+func scannerErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, posn token.Position) (span.Span, error) {
+	ph, _, err := pkg.FindFile(ctx, span.FileURI(posn.Filename))
+	if err != nil {
+		return span.Span{}, err
+	}
+	file, _, _, err := ph.Cached(ctx)
+	if err != nil {
+		return span.Span{}, err
+	}
+	tok := fset.File(file.Pos())
+	if tok == nil {
+		return span.Span{}, errors.Errorf("no token.File for %s", ph.File().Identity().URI)
+	}
+	pos := tok.Pos(posn.Offset)
+	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
+	if len(path) > 0 {
+		switch n := path[0].(type) {
+		case *ast.BadDecl, *ast.BadExpr, *ast.BadStmt:
+			if s, err := span.NewRange(fset, n.Pos(), n.End()).Span(); err == nil {
+				return s, nil
+			}
+		}
+	}
+	return span.NewRange(fset, pos, pos).Span()
+}
+
 // spanToRange converts a span.Span to a protocol.Range,
 // assuming that the span belongs to the package whose diagnostics are being computed.
-func spanToRange(ctx context.Context, pkg *pkg, spn span.Span, isTypeError bool) (protocol.Range, error) {
-	ph, err := pkg.File(spn.URI())
+func spanToRange(ctx context.Context, pkg *pkg, spn span.Span) (protocol.Range, error) {
+	ph, _, err := pkg.FindFile(ctx, spn.URI())
 	if err != nil {
 		return protocol.Range{}, err
 	}
 	_, m, _, err := ph.Cached(ctx)
 	if err != nil {
 		return protocol.Range{}, err
 	}
-	if spn.IsPoint() && isTypeError {
-		data, _, err := ph.File().Read(ctx)
-		if err != nil {
-			return protocol.Range{}, err
-		}
-		if s, err := spn.WithOffset(m.Converter); err == nil {
-			start := s.Start()
-			offset := start.Offset()
-			if offset < len(data) {
-				if width := bytes.IndexAny(data[offset:], " \n,():;[]"); width > 0 {
-					spn = span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+width, offset+width))
-				}
-			}
-		}
-	}
 	return m.Range(spn)
 }
 

internal/lsp/tests/diagnostics.go

@@ -38,12 +38,7 @@ func DiffDiagnostics(want, got []source.Diagnostic) string {
 		if protocol.ComparePosition(w.Range.Start, g.Range.Start) != 0 {
 			return summarizeDiagnostics(i, want, got, "incorrect Start got %v want %v", g.Range.Start, w.Range.Start)
 		}
-		// Special case for diagnostics on parse errors.
-		if strings.Contains(string(g.URI), "noparse") {
-			if protocol.ComparePosition(g.Range.Start, g.Range.End) != 0 || protocol.ComparePosition(w.Range.Start, g.Range.End) != 0 {
-				return summarizeDiagnostics(i, want, got, "incorrect End got %v want %v", g.Range.End, w.Range.Start)
-			}
-		} else if !protocol.IsPoint(g.Range) { // Accept any 'want' range if the diagnostic returns a zero-length range.
+		if !protocol.IsPoint(g.Range) { // Accept any 'want' range if the diagnostic returns a zero-length range.
 			if protocol.ComparePosition(w.Range.End, g.Range.End) != 0 {
 				return summarizeDiagnostics(i, want, got, "incorrect End got %v want %v", g.Range.End, w.Range.End)
 			}