From d00ac6d27372a4273825635281f2dc360d4be563 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 21 Dec 2018 17:54:19 -0500 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell Reviewed-by: Libor K --- go/packages/packagestest/expect.go | 30 ++++++----------------- internal/lsp/lsp_test.go | 39 +++++++++--------------------- internal/lsp/source/definition.go | 20 +++++---------- 3 files changed, 25 insertions(+), 64 deletions(-) diff --git a/go/packages/packagestest/expect.go b/go/packages/packagestest/expect.go index 5312ecca..077c3051 100644 --- a/go/packages/packagestest/expect.go +++ b/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)) - identifierType = reflect.TypeOf(expect.Identifier("")) - posType = reflect.TypeOf(token.Pos(0)) - positionType = reflect.TypeOf(token.Position{}) - rangeType = reflect.TypeOf(Range{}) - rangePositionType = reflect.TypeOf(RangePosition{}) - fsetType = reflect.TypeOf((*token.FileSet)(nil)) - regexType = reflect.TypeOf((*regexp.Regexp)(nil)) + noteType = reflect.TypeOf((*expect.Note)(nil)) + identifierType = reflect.TypeOf(expect.Identifier("")) + posType = reflect.TypeOf(token.Pos(0)) + positionType = reflect.TypeOf(token.Position{}) + rangeType = reflect.TypeOf(Range{}) + fsetType = reflect.TypeOf((*token.FileSet)(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] diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 264e4e01..65282e4e 100644 --- a/internal/lsp/lsp_test.go +++ b/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) { - if rng.Start.Filename != rng.End.Filename { - return +func (d diagnostics) collect(fset *token.FileSet, rng packagestest.Range, msg string) { + f := fset.File(rng.Start) + if _, ok := d[f.Name()]; !ok { + d[f.Name()] = []protocol.Diagnostic{} } - filename := rng.Start.Filename - if _, ok := d[filename]; !ok { - 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 a file has an empty diagnostic message, 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{ - 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), - }, - }, + Range: toProtocolRange(f, source.Range(rng)), 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} - sLoc := toProtocolLocation(fset, sRange) - tRange := source.Range{Start: target.Start, End: target.End} - tLoc := toProtocolLocation(fset, tRange) - d[sLoc] = tLoc + loc := toProtocolLocation(fset, source.Range(src)) + d[loc] = toProtocolLocation(fset, source.Range(target)) } diff --git a/internal/lsp/source/definition.go b/internal/lsp/source/definition.go index 778566cb..89f038e5 100644 --- a/internal/lsp/source/definition.go +++ b/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() - if err != nil { - return Range{}, err - } - return objToRange(ctx, v, fset, obj), nil + return objToRange(ctx, v, 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() - if err != nil { - return Range{}, err - } - return objToRange(ctx, v, fset, obj), nil + return objToRange(ctx, v, 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 }