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 <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/go/analysis/analysis.go b/go/analysis/analysis.go
index 8eb7316..19e1e42 100644
--- a/go/analysis/analysis.go
+++ b/go/analysis/analysis.go
@@ -161,6 +161,15 @@
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 @@
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 ad979c1..c81e020 100644
--- a/go/analysis/analysistest/analysistest.go
+++ b/go/analysis/analysistest/analysistest.go
@@ -257,6 +257,7 @@
// 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 062d062..a3c2f09 100644
--- a/go/analysis/internal/analysisflags/flags.go
+++ b/go/analysis/internal/analysisflags/flags.go
@@ -323,9 +323,14 @@
// -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 @@
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 4cade77..e3b0506 100644
--- a/go/analysis/internal/checker/checker.go
+++ b/go/analysis/internal/checker/checker.go
@@ -295,7 +295,8 @@
// 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 @@
// 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 3f88a88..bd0cd2d 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -133,7 +133,7 @@
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.
