From 2b03ca6e44eb97adb8824db161059e54d6014dc3 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Tue, 28 May 2019 16:31:28 -0400 Subject: [PATCH] go/analysis: add an End field to Diagnostic This will allow diagnostics to denote the range they apply to. The ranges are now interpreted using the internal/span library. This is primarily intended for the benefit of the LSP, which will be able to (in future CLs) more accurately highlight the part of the code a diagnostic applies to. Change-Id: Ic35cec2b21060c9dc6a8f5ebb7faa62d81a07435 Reviewed-on: https://go-review.googlesource.com/c/tools/+/179237 Run-TryBot: Michael Matloob Reviewed-by: Ian Cottrell --- go/analysis/analysis.go | 17 +++++++++++++++-- go/analysis/analysistest/analysistest.go | 1 + go/analysis/internal/analysisflags/flags.go | 9 ++++++++- go/analysis/internal/checker/checker.go | 6 ++++-- internal/lsp/source/diagnostics.go | 2 +- 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/go/analysis/analysis.go b/go/analysis/analysis.go index 8eb73162..19e1e421 100644 --- a/go/analysis/analysis.go +++ b/go/analysis/analysis.go @@ -161,6 +161,15 @@ func (pass *Pass) Reportf(pos token.Pos, format string, args ...interface{}) { pass.Report(Diagnostic{Pos: pos, Message: msg}) } +// reportNodef is a helper function that reports a Diagnostic using the +// range denoted by the AST node. +// +// WARNING: This is an experimental API and may change in the future. +func (pass *Pass) reportNodef(node ast.Node, format string, args ...interface{}) { + msg := fmt.Sprintf(format, args...) + pass.Report(Diagnostic{Pos: node.Pos(), End: node.End(), Message: msg}) +} + func (pass *Pass) String() string { return fmt.Sprintf("%s@%s", pass.Analyzer.Name, pass.Pkg.Path()) } @@ -203,13 +212,17 @@ type Fact interface { AFact() // dummy method to avoid type errors } -// A Diagnostic is a message associated with a source location. +// A Diagnostic is a message associated with a source location or range. // // An Analyzer may return a variety of diagnostics; the optional Category, // which should be a constant, may be used to classify them. // It is primarily intended to make it easy to look up documentation. +// +// If End is provided, the diagnostic is specified to apply to the range between +// Pos and End. type Diagnostic struct { Pos token.Pos - Category string // optional + End token.Pos // optional + Category string // optional Message string } diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index ad979c14..c81e0205 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -257,6 +257,7 @@ func check(t Testing, gopath string, pass *analysis.Pass, diagnostics []analysis // Check the diagnostics match expectations. for _, f := range diagnostics { + // TODO(matloob): Support ranges in analysistest. posn := pass.Fset.Position(f.Pos) checkMessage(posn, "diagnostic", "", f.Message) } diff --git a/go/analysis/internal/analysisflags/flags.go b/go/analysis/internal/analysisflags/flags.go index 062d0624..a3c2f096 100644 --- a/go/analysis/internal/analysisflags/flags.go +++ b/go/analysis/internal/analysisflags/flags.go @@ -323,9 +323,14 @@ func PrintPlain(fset *token.FileSet, diag analysis.Diagnostic) { // -c=N: show offending line plus N lines of context. if Context >= 0 { + posn := fset.Position(diag.Pos) + end := fset.Position(diag.End) + if !end.IsValid() { + end = posn + } data, _ := ioutil.ReadFile(posn.Filename) lines := strings.Split(string(data), "\n") - for i := posn.Line - Context; i <= posn.Line+Context; i++ { + for i := posn.Line - Context; i <= end.Line+Context; i++ { if 1 <= i && i <= len(lines) { fmt.Fprintf(os.Stderr, "%d\t%s\n", i, lines[i-1]) } @@ -353,6 +358,8 @@ func (tree JSONTree) Add(fset *token.FileSet, id, name string, diags []analysis. Message string `json:"message"` } var diagnostics []jsonDiagnostic + // TODO(matloob): Should the JSON diagnostics contain ranges? + // If so, how should they be formatted? for _, f := range diags { diagnostics = append(diagnostics, jsonDiagnostic{ Category: f.Category, diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index 4cade777..e3b05066 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -295,7 +295,8 @@ func printDiagnostics(roots []*action) (exitcode int) { // avoid double-reporting in source files that belong to // multiple packages, such as foo and foo.test. type key struct { - token.Position + pos token.Position + end token.Position *analysis.Analyzer message string } @@ -313,7 +314,8 @@ func printDiagnostics(roots []*action) (exitcode int) { // as most users don't care. posn := act.pkg.Fset.Position(diag.Pos) - k := key{posn, act.a, diag.Message} + end := act.pkg.Fset.Position(diag.End) + k := key{posn, end, act.a, diag.Message} if seen[k] { continue // duplicate } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 3f88a884..bd0cd2d7 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -133,7 +133,7 @@ func diagnostics(ctx context.Context, v View, pkg Package, reports map[span.URI] func analyses(ctx context.Context, v View, pkg Package, reports map[span.URI][]Diagnostic) error { // Type checking and parsing succeeded. Run analyses. if err := runAnalyses(ctx, v, pkg, func(a *analysis.Analyzer, diag analysis.Diagnostic) error { - r := span.NewRange(v.Session().Cache().FileSet(), diag.Pos, 0) + r := span.NewRange(v.Session().Cache().FileSet(), diag.Pos, diag.End) s, err := r.Span() if err != nil { // The diagnostic has an invalid position, so we don't have a valid span.