Skip to content

Commit

Permalink
gopls/internal/lsp: tolerate missing end position in RelatedInformation
Browse files Browse the repository at this point in the history
Fix the panic reported in #56270, allowing RelatedInformation.End to be
missing.

Fixes golang/go#56270

Change-Id: I5f2dc27cf149c324f39ddbb056862434fe38f730
Reviewed-on: https://go-review.googlesource.com/c/tools/+/443337
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
  • Loading branch information
findleyr committed Oct 19, 2022
1 parent d67c3ad commit 502b93c
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 8 deletions.
2 changes: 1 addition & 1 deletion go/analysis/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type Diagnostic struct {
// declaration.
type RelatedInformation struct {
Pos token.Pos
End token.Pos
End token.Pos // optional
Message string
}

Expand Down
6 changes: 5 additions & 1 deletion gopls/internal/lsp/cache/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,11 @@ func relatedInformation(pkg *pkg, fset *token.FileSet, diag *analysis.Diagnostic
if tokFile == nil {
return nil, bug.Errorf("no file for %q diagnostic position", diag.Category)
}
spn, err := span.NewRange(tokFile, related.Pos, related.End).Span()
end := related.End
if !end.IsValid() {
end = related.Pos
}
spn, err := span.NewRange(tokFile, related.Pos, end).Span()
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/lsp/regtest/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
"sync"
"testing"

"golang.org/x/tools/internal/jsonrpc2/servertest"
"golang.org/x/tools/gopls/internal/lsp/fake"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/internal/jsonrpc2/servertest"
)

// Env holds the building blocks of an editor testing environment, providing
Expand Down Expand Up @@ -119,7 +119,7 @@ func (s State) String() string {
for name, params := range s.diagnostics {
fmt.Fprintf(&b, "\t%s (version %d):\n", name, int(params.Version))
for _, d := range params.Diagnostics {
fmt.Fprintf(&b, "\t\t(%d, %d): %s\n", int(d.Range.Start.Line), int(d.Range.Start.Character), d.Message)
fmt.Fprintf(&b, "\t\t(%d, %d) [%s]: %s\n", int(d.Range.Start.Line), int(d.Range.Start.Character), d.Source, d.Message)
}
}
b.WriteString("\n")
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/lsp/source/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -1306,8 +1306,8 @@ func (r *OptionResult) asBoolMap() map[string]bool {
}
m := make(map[string]bool)
for a, enabled := range all {
if enabled, ok := enabled.(bool); ok {
m[a] = enabled
if e, ok := enabled.(bool); ok {
m[a] = e
} else {
r.parseErrorf("invalid type %T for map key %q", enabled, a)
return m
Expand Down
37 changes: 37 additions & 0 deletions gopls/internal/regtest/misc/staticcheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,40 @@ var FooErr error = errors.New("foo")
)
})
}

// Test for golang/go#56270: an analysis with related info should not panic if
// analysis.RelatedInformation.End is not set.
func TestStaticcheckRelatedInfo(t *testing.T) {
testenv.NeedsGo1Point(t, 17) // staticcheck is only supported at Go 1.17+
const files = `
-- go.mod --
module mod.test
go 1.18
-- p.go --
package p
import (
"fmt"
)
func Foo(enabled interface{}) {
if enabled, ok := enabled.(bool); ok {
} else {
_ = fmt.Sprintf("invalid type %T", enabled) // enabled is always bool here
}
}
`

WithOptions(
Settings{"staticcheck": true},
).Run(t, files, func(t *testing.T, env *Env) {
env.OpenFile("p.go")
env.Await(
OnceMet(
env.DoneWithOpen(),
env.DiagnosticAtRegexpFromSource("p.go", ", (enabled)", "SA9008"),
),
)
})
}
6 changes: 4 additions & 2 deletions gopls/internal/span/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ type Range struct {
Start, End token.Pos // both IsValid()
}

// NewRange creates a new Range from a token.File and two valid positions within it.
// NewRange creates a new Range from a token.File and two positions within it.
// The given start position must be valid; if end is invalid, start is used as
// the end position.
//
// (If you only have a token.FileSet, use file = fset.File(start). But
// most callers know exactly which token.File they're dealing with and
Expand All @@ -33,7 +35,7 @@ func NewRange(file *token.File, start, end token.Pos) Range {
panic("invalid start token.Pos")
}
if !end.IsValid() {
panic("invalid end token.Pos")
end = start
}

// TODO(adonovan): ideally we would make this stronger assertion:
Expand Down

0 comments on commit 502b93c

Please sign in to comment.