Skip to content
Permalink
Browse files

internal/lsp: add ranges to some diagnostics messages

Added a View interface to the source package, which allows for reading
of other files (in the same package or in other packages). We were
already reading files in jump to definition (to handle the lack of
column information in export data), but now we can also read files in
diagnostics, which allows us to determine the end of an identifier so
that we can report ranges in diagnostic messages.

Updates golang/go#29150

Change-Id: I7958d860dea8f41f2df88a467b5e2946bba4d1c5
Reviewed-on: https://go-review.googlesource.com/c/154742
Reviewed-by: Ian Cottrell <iancottrell@google.com>
  • Loading branch information
stamblerre committed Dec 18, 2018
1 parent 3571f65 commit f344c7530cd827b6783f004fad0d8e0cd260ec96
@@ -121,6 +121,10 @@ 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 {
@@ -173,13 +177,14 @@ 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{})
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{})
rangePositionType = reflect.TypeOf(RangePosition{})
fsetType = reflect.TypeOf((*token.FileSet)(nil))
regexType = reflect.TypeOf((*regexp.Regexp)(nil))
)

// converter converts from a marker's argument parsed from the comment to
@@ -234,6 +239,17 @@ 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]
@@ -34,9 +34,9 @@ func NewView(rootPath string) *View {
}
}

// GetFile returns a File for the given uri.
// It will always succeed, adding the file to the managed set if needed.
func (v *View) GetFile(uri source.URI) *File {
// GetFile returns a File for the given URI. It will always succeed because it
// adds the file to the managed set if needed.
func (v *View) GetFile(uri source.URI) source.File {
v.mu.Lock()
f := v.getFile(uri)
v.mu.Unlock()
@@ -15,40 +15,35 @@ import (

func (s *server) CacheAndDiagnose(ctx context.Context, uri protocol.DocumentURI, text string) {
f := s.view.GetFile(source.URI(uri))
f.SetContent([]byte(text))

if f, ok := f.(*cache.File); ok {
f.SetContent([]byte(text))
}
go func() {
reports, err := source.Diagnostics(ctx, f)
reports, err := source.Diagnostics(ctx, s.view, f)
if err != nil {
return // handle error?
}
for filename, diagnostics := range reports {
uri := source.ToURI(filename)
s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
URI: protocol.DocumentURI(source.ToURI(filename)),
Diagnostics: toProtocolDiagnostics(s.view, diagnostics),
URI: protocol.DocumentURI(uri),
Diagnostics: toProtocolDiagnostics(s.view, uri, diagnostics),
})
}
}()
}

func toProtocolDiagnostics(v *cache.View, diagnostics []source.Diagnostic) []protocol.Diagnostic {
func toProtocolDiagnostics(v *cache.View, uri source.URI, diagnostics []source.Diagnostic) []protocol.Diagnostic {
reports := []protocol.Diagnostic{}
for _, diag := range diagnostics {
f := v.GetFile(source.ToURI(diag.Filename))
f := v.GetFile(uri)
tok, err := f.GetToken()
if err != nil {
continue // handle error?
}
pos := fromTokenPosition(tok, diag.Position)
if !pos.IsValid() {
continue // handle error?
}
reports = append(reports, protocol.Diagnostic{
Message: diag.Message,
Range: toProtocolRange(tok, source.Range{
Start: pos,
End: pos,
}),
Message: diag.Message,
Range: toProtocolRange(tok, diag.Range),
Severity: protocol.SeverityError, // all diagnostics have error severity for now
Source: "LSP",
})
@@ -59,10 +54,10 @@ func toProtocolDiagnostics(v *cache.View, diagnostics []source.Diagnostic) []pro
func sorted(d []protocol.Diagnostic) {
sort.Slice(d, func(i int, j int) bool {
if d[i].Range.Start.Line == d[j].Range.Start.Line {
if d[i].Range.Start.Character == d[j].Range.End.Character {
if d[i].Range.Start.Character == d[j].Range.Start.Character {
return d[i].Message < d[j].Message
}
return d[i].Range.Start.Character < d[j].Range.End.Character
return d[i].Range.Start.Character < d[j].Range.Start.Character
}
return d[i].Range.Start.Line < d[j].Range.Start.Line
})
@@ -11,7 +11,6 @@ import (
"go/token"
"os/exec"
"path/filepath"
"reflect"
"strings"
"testing"

@@ -36,7 +35,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
// We hardcode the expected number of test cases to ensure that all tests
// are being executed. If a test is added, this number must be changed.
const expectedCompletionsCount = 60
const expectedDiagnosticsCount = 15
const expectedDiagnosticsCount = 13
const expectedFormatCount = 3
const expectedDefinitionsCount = 16
const expectedTypeDefinitionsCount = 2
@@ -155,47 +154,94 @@ func (d diagnostics) test(t *testing.T, exported *packagestest.Exported, v *cach
count := 0
for filename, want := range d {
f := v.GetFile(source.ToURI(filename))
sourceDiagnostics, err := source.Diagnostics(context.Background(), f)
sourceDiagnostics, err := source.Diagnostics(context.Background(), v, f)
if err != nil {
t.Fatal(err)
}
got := toProtocolDiagnostics(v, sourceDiagnostics[filename])
got := toProtocolDiagnostics(v, source.ToURI(filename), sourceDiagnostics[filename])
sorted(got)
if equal := reflect.DeepEqual(want, got); !equal {
t.Error(diffD(filename, want, got))
if diff := diffDiagnostics(filename, want, got); diff != "" {
t.Error(diff)
}
count += len(want)
}
return count
}

func (d diagnostics) collect(pos token.Position, msg string) {
if _, ok := d[pos.Filename]; !ok {
d[pos.Filename] = []protocol.Diagnostic{}
func (d diagnostics) collect(rng packagestest.RangePosition, msg string) {
if rng.Start.Filename != rng.End.Filename {
return
}
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 msg == "" {
return
}
line := float64(pos.Line - 1)
col := float64(pos.Column - 1)
want := protocol.Diagnostic{
Range: protocol.Range{
Start: protocol.Position{
Line: line,
Character: col,
Line: float64(rng.Start.Line - 1),
Character: float64(rng.Start.Column - 1),
},
End: protocol.Position{
Line: line,
Character: col,
Line: float64(rng.End.Line - 1),
Character: float64(rng.End.Column - 1),
},
},
Severity: protocol.SeverityError,
Source: "LSP",
Message: msg,
}
d[pos.Filename] = append(d[pos.Filename], want)
d[filename] = append(d[filename], want)
}

// diffDiagnostics prints the diff between expected and actual diagnostics test
// results.
func diffDiagnostics(filename string, want, got []protocol.Diagnostic) string {
if len(got) != len(want) {
goto Failed
}
for i, w := range want {
g := got[i]
if w.Message != g.Message {
goto Failed
}
if w.Range.Start != g.Range.Start {
goto Failed
}
// Special case for diagnostics on parse errors.
if strings.Contains(filename, "noparse") {
if g.Range.Start != g.Range.End || w.Range.Start != g.Range.End {
goto Failed
}
} else {
if w.Range.End != g.Range.End {
goto Failed
}
}
if w.Severity != g.Severity {
goto Failed
}
if w.Source != g.Source {
goto Failed
}
}
return ""
Failed:
msg := &bytes.Buffer{}
fmt.Fprintf(msg, "diagnostics failed for %s:\nexpected:\n", filename)
for _, d := range want {
fmt.Fprintf(msg, " %v\n", d)
}
fmt.Fprintf(msg, "got:\n")
for _, d := range got {
fmt.Fprintf(msg, " %v\n", d)
}
return msg.String()
}

func (c completions) test(t *testing.T, exported *packagestest.Exported, s *server, items completionItems) {
@@ -240,7 +286,7 @@ func (c completions) test(t *testing.T, exported *packagestest.Exported, s *serv
if err != nil {
t.Fatalf("completion failed for %s:%v:%v: %v", filepath.Base(src.Filename), src.Line, src.Column, err)
}
if diff := diffC(src, want, got); diff != "" {
if diff := diffCompletionItems(src, want, got); diff != "" {
t.Errorf(diff)
}
}
@@ -279,6 +325,38 @@ func (i completionItems) collect(pos token.Pos, label, detail, kind string) {
}
}

// diffCompletionItems prints the diff between expected and actual completion
// test results.
func diffCompletionItems(pos token.Position, want, got []protocol.CompletionItem) string {
if len(got) != len(want) {
goto Failed
}
for i, w := range want {
g := got[i]
if w.Label != g.Label {
goto Failed
}
if w.Detail != g.Detail {
goto Failed
}
if w.Kind != g.Kind {
goto Failed
}
}
return ""
Failed:
msg := &bytes.Buffer{}
fmt.Fprintf(msg, "completion failed for %s:%v:%v:\nexpected:\n", filepath.Base(pos.Filename), pos.Line, pos.Column)
for _, d := range want {
fmt.Fprintf(msg, " %v\n", d)
}
fmt.Fprintf(msg, "got:\n")
for _, d := range got {
fmt.Fprintf(msg, " %v\n", d)
}
return msg.String()
}

func (f formats) test(t *testing.T, s *server) {
for filename, gofmted := range f {
edits, err := s.Formatting(context.Background(), &protocol.DocumentFormattingParams{
@@ -341,48 +419,3 @@ func (d definitions) collect(fset *token.FileSet, src, target packagestest.Range
tLoc := toProtocolLocation(fset, tRange)
d[sLoc] = tLoc
}

// diffD prints the diff between expected and actual diagnostics test results.
func diffD(filename string, want, got []protocol.Diagnostic) string {
msg := &bytes.Buffer{}
fmt.Fprintf(msg, "diagnostics failed for %s:\nexpected:\n", filename)
for _, d := range want {
fmt.Fprintf(msg, " %v\n", d)
}
fmt.Fprintf(msg, "got:\n")
for _, d := range got {
fmt.Fprintf(msg, " %v\n", d)
}
return msg.String()
}

// diffC prints the diff between expected and actual completion test results.
func diffC(pos token.Position, want, got []protocol.CompletionItem) string {
if len(got) != len(want) {
goto Failed
}
for i, w := range want {
g := got[i]
if w.Label != g.Label {
goto Failed
}
if w.Detail != g.Detail {
goto Failed
}
if w.Kind != g.Kind {
goto Failed
}
}
return ""
Failed:
msg := &bytes.Buffer{}
fmt.Fprintf(msg, "completion failed for %s:%v:%v:\nexpected:\n", filepath.Base(pos.Filename), pos.Line, pos.Column)
for _, d := range want {
fmt.Fprintf(msg, " %v\n", d)
}
fmt.Fprintf(msg, "got:\n")
for _, d := range got {
fmt.Fprintf(msg, " %v\n", d)
}
return msg.String()
}
@@ -143,12 +143,14 @@ func (s *server) WillSaveWaitUntil(context.Context, *protocol.WillSaveTextDocume
}

func (s *server) DidSave(context.Context, *protocol.DidSaveTextDocumentParams) error {
// TODO(rstambler): Should we clear the cache here?
return nil // ignore
}

func (s *server) DidClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error {
s.view.GetFile(source.URI(params.TextDocument.URI)).SetContent(nil)
f := s.view.GetFile(source.URI(params.TextDocument.URI))
if f, ok := f.(*cache.File); ok {
f.SetContent(nil)
}
return nil
}

@@ -214,7 +216,7 @@ func (s *server) Definition(ctx context.Context, params *protocol.TextDocumentPo
return nil, err
}
pos := fromProtocolPosition(tok, params.Position)
r, err := source.Definition(ctx, f, pos)
r, err := source.Definition(ctx, s.view, f, pos)
if err != nil {
return nil, err
}
@@ -228,7 +230,7 @@ func (s *server) TypeDefinition(ctx context.Context, params *protocol.TextDocume
return nil, err
}
pos := fromProtocolPosition(tok, params.Position)
r, err := source.TypeDefinition(ctx, f, pos)
r, err := source.TypeDefinition(ctx, s.view, f, pos)
if err != nil {
return nil, err
}

0 comments on commit f344c75

Please sign in to comment.
You can’t perform that action at this time.