internal/lsp: remove unnecessary packagestest.RangePosition type

Some cleanup of lsp_test.

Change-Id: I0cf4eb73f223845374c7f7f4939a461ff676bde8
Reviewed-on: https://go-review.googlesource.com/c/155579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Libor K <aschrak1978@gmail.com>

go/packages/packagestest/expect.go

@@ -121,10 +121,6 @@ type Range struct {
 	End   token.Pos
 }
 
-type RangePosition struct {
-	Start, End token.Position
-}
-
 // Mark adds a new marker to the known set.
 func (e *Exported) Mark(name string, r Range) {
 	if e.markers == nil {
@@ -177,14 +173,13 @@ func (e *Exported) getMarkers() error {
 }
 
 var (
-	noteType          = reflect.TypeOf((*expect.Note)(nil))
+	noteType       = reflect.TypeOf((*expect.Note)(nil))
-	identifierType    = reflect.TypeOf(expect.Identifier(""))
+	identifierType = reflect.TypeOf(expect.Identifier(""))
-	posType           = reflect.TypeOf(token.Pos(0))
+	posType        = reflect.TypeOf(token.Pos(0))
-	positionType      = reflect.TypeOf(token.Position{})
+	positionType   = reflect.TypeOf(token.Position{})
-	rangeType         = reflect.TypeOf(Range{})
+	rangeType      = reflect.TypeOf(Range{})
-	rangePositionType = reflect.TypeOf(RangePosition{})
+	fsetType       = reflect.TypeOf((*token.FileSet)(nil))
-	fsetType          = reflect.TypeOf((*token.FileSet)(nil))
+	regexType      = reflect.TypeOf((*regexp.Regexp)(nil))
-	regexType         = reflect.TypeOf((*regexp.Regexp)(nil))
 )
 
 // converter converts from a marker's argument parsed from the comment to
@@ -239,17 +234,6 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) {
 			}
 			return reflect.ValueOf(r), remains, nil
 		}, nil
-	case pt == rangePositionType:
-		return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) {
-			r, remains, err := e.rangeConverter(n, args)
-			if err != nil {
-				return reflect.Value{}, nil, err
-			}
-			return reflect.ValueOf(RangePosition{
-				Start: e.fset.Position(r.Start),
-				End:   e.fset.Position(r.End),
-			}), remains, nil
-		}, nil
 	case pt == identifierType:
 		return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) {
 			arg := args[0]

internal/lsp/lsp_test.go

@@ -104,7 +104,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 
 	t.Run("Diagnostics", func(t *testing.T) {
 		t.Helper()
-		diagnosticsCount := expectedDiagnostics.test(t, exported, s.view)
+		diagnosticsCount := expectedDiagnostics.test(t, s.view)
 		if goVersion111 { // TODO(rstambler): Remove this when we no longer support Go 1.10.
 			if diagnosticsCount != expectedDiagnosticsCount {
 				t.Errorf("got %v diagnostics expected %v", diagnosticsCount, expectedDiagnosticsCount)
@@ -149,7 +149,7 @@ type completions map[token.Position][]token.Pos
 type formats map[string]string
 type definitions map[protocol.Location]protocol.Location
 
-func (d diagnostics) test(t *testing.T, exported *packagestest.Exported, v source.View) int {
+func (d diagnostics) test(t *testing.T, v source.View) int {
 	count := 0
 	ctx := context.Background()
 	for filename, want := range d {
@@ -167,35 +167,23 @@ func (d diagnostics) test(t *testing.T, exported *packagestest.Exported, v sourc
 	return count
 }
 
-func (d diagnostics) collect(rng packagestest.RangePosition, msg string) {
+func (d diagnostics) collect(fset *token.FileSet, rng packagestest.Range, msg string) {
-	if rng.Start.Filename != rng.End.Filename {
+	f := fset.File(rng.Start)
-		return
+	if _, ok := d[f.Name()]; !ok {
+		d[f.Name()] = []protocol.Diagnostic{}
 	}
-	filename := rng.Start.Filename
+	// If a file has an empty diagnostic message, return. This allows us to
-	if _, ok := d[filename]; !ok {
+	// avoid testing diagnostics in files that may have a lot of them.
-		d[filename] = []protocol.Diagnostic{}
-	}
-	// If a file has an empty diagnostics, mark that and return. This allows us
-	// to avoid testing diagnostics in files that may have a lot of them.
 	if msg == "" {
 		return
 	}
 	want := protocol.Diagnostic{
-		Range: protocol.Range{
+		Range:    toProtocolRange(f, source.Range(rng)),
-			Start: protocol.Position{
-				Line:      float64(rng.Start.Line - 1),
-				Character: float64(rng.Start.Column - 1),
-			},
-			End: protocol.Position{
-				Line:      float64(rng.End.Line - 1),
-				Character: float64(rng.End.Column - 1),
-			},
-		},
 		Severity: protocol.SeverityError,
 		Source:   "LSP",
 		Message:  msg,
 	}
-	d[filename] = append(d[filename], want)
+	d[f.Name()] = append(d[f.Name()], want)
 }
 
 // diffDiagnostics prints the diff between expected and actual diagnostics test
@@ -412,9 +400,6 @@ func (d definitions) test(t *testing.T, s *server, typ bool) {
 }
 
 func (d definitions) collect(fset *token.FileSet, src, target packagestest.Range) {
-	sRange := source.Range{Start: src.Start, End: src.End}
+	loc := toProtocolLocation(fset, source.Range(src))
-	sLoc := toProtocolLocation(fset, sRange)
+	d[loc] = toProtocolLocation(fset, source.Range(target))
-	tRange := source.Range{Start: target.Start, End: target.End}
-	tLoc := toProtocolLocation(fset, tRange)
-	d[sLoc] = tLoc
 }

internal/lsp/source/definition.go

@@ -44,11 +44,7 @@ func Definition(ctx context.Context, v View, f File, pos token.Pos) (Range, erro
 			}
 		}
 	}
-	fset, err := f.GetFileSet()
+	return objToRange(ctx, v, obj), nil
-	if err != nil {
-		return Range{}, err
-	}
-	return objToRange(ctx, v, fset, obj), nil
 }
 
 func TypeDefinition(ctx context.Context, v View, f File, pos token.Pos) (Range, error) {
@@ -75,11 +71,7 @@ func TypeDefinition(ctx context.Context, v View, f File, pos token.Pos) (Range,
 	if obj == nil {
 		return Range{}, fmt.Errorf("no object for type %s", typ.String())
 	}
-	fset, err := f.GetFileSet()
+	return objToRange(ctx, v, obj), nil
-	if err != nil {
-		return Range{}, err
-	}
-	return objToRange(ctx, v, fset, obj), nil
 }
 
 func typeToObject(typ types.Type) (obj types.Object) {
@@ -137,9 +129,9 @@ func checkIdentifier(f *ast.File, pos token.Pos) (ident, error) {
 	return result, nil
 }
 
-func objToRange(ctx context.Context, v View, fset *token.FileSet, obj types.Object) Range {
+func objToRange(ctx context.Context, v View, obj types.Object) Range {
 	p := obj.Pos()
-	tok := fset.File(p)
+	tok := v.FileSet().File(p)
 	pos := tok.Position(p)
 	if pos.Column == 1 {
 		// We do not have full position information because exportdata does not
@@ -152,11 +144,11 @@ func objToRange(ctx context.Context, v View, fset *token.FileSet, obj types.Obje
 		if err != nil {
 			goto Return
 		}
-		tok, err := f.GetToken()
+		src, err := f.Read()
 		if err != nil {
 			goto Return
 		}
-		src, err := f.Read()
+		tok, err := f.GetToken()
 		if err != nil {
 			goto Return
 		}