Skip to content

Commit

Permalink
gopls/internal: don't bug.Report when given an invalid position
Browse files Browse the repository at this point in the history
It's possible that the client sent us a bad protocol.Range:
that shouldn't trigger a bug report. (It's also possible that
AST fixing is perturbing positions so as to cause a valid
range not to match a parameter, but we can't easily distinguish
that case.)

Fixes golang/go#64544

Change-Id: I28f29716e8c98cf57d44cddfad5b46b1311808bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/548738
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
adonovan authored and gopherbot committed Dec 11, 2023
1 parent 90c60a2 commit 0640701
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 22 deletions.
3 changes: 1 addition & 2 deletions gopls/internal/lsp/source/change_quote.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ import (
func ConvertStringLiteral(pgf *ParsedGoFile, fh file.Handle, rng protocol.Range) (protocol.CodeAction, bool) {
startPos, endPos, err := pgf.RangePos(rng)
if err != nil {
bug.Reportf("(file=%v).RangePos(%v) failed: %v", pgf.URI, rng, err)
return protocol.CodeAction{}, false
return protocol.CodeAction{}, false // e.g. invalid range
}
path, _ := astutil.PathEnclosingInterval(pgf.File, startPos, endPos)
lit, ok := path[0].(*ast.BasicLit)
Expand Down
26 changes: 14 additions & 12 deletions gopls/internal/lsp/source/change_signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ func RemoveUnusedParameter(ctx context.Context, fh file.Handle, rng protocol.Ran
return nil, fmt.Errorf("can't change signatures for packages with parse or type errors: (e.g. %s)", sample)
}

info := FindParam(pgf, rng)
if info.Decl == nil {
return nil, fmt.Errorf("failed to find declaration")
info, err := FindParam(pgf, rng)
if err != nil {
return nil, err // e.g. invalid range
}
if info.Decl.Recv != nil {
return nil, fmt.Errorf("can't change signature of methods (yet)")
Expand Down Expand Up @@ -242,20 +242,18 @@ func rewriteSignature(fset *token.FileSet, declIdx int, src0 []byte, newDecl *as

// ParamInfo records information about a param identified by a position.
type ParamInfo struct {
Decl *ast.FuncDecl // enclosing func decl, or nil
Decl *ast.FuncDecl // enclosing func decl (non-nil)
FieldIndex int // index of Field in Decl.Type.Params, or -1
Field *ast.Field // enclosing field of Decl, or nil
Field *ast.Field // enclosing field of Decl, or nil if range not among parameters
NameIndex int // index of Name in Field.Names, or nil
Name *ast.Ident // indicated name (either enclosing, or Field.Names[0] if len(Field.Names) == 1)
}

// FindParam finds the parameter information spanned by the given range.
func FindParam(pgf *ParsedGoFile, rng protocol.Range) ParamInfo {
info := ParamInfo{FieldIndex: -1, NameIndex: -1}
func FindParam(pgf *ParsedGoFile, rng protocol.Range) (*ParamInfo, error) {
start, end, err := pgf.RangePos(rng)
if err != nil {
bug.Reportf("(file=%v).RangePos(%v) failed: %v", pgf.URI, rng, err)
return info
return nil, err
}

path, _ := astutil.PathEnclosingInterval(pgf.File, start, end)
Expand All @@ -278,9 +276,13 @@ func FindParam(pgf *ParsedGoFile, rng protocol.Range) ParamInfo {
}
// Check the conditions described in the docstring.
if decl == nil {
return info
return nil, fmt.Errorf("range is not within a function declaration")
}
info := &ParamInfo{
FieldIndex: -1,
NameIndex: -1,
Decl: decl,
}
info.Decl = decl
for fi, f := range decl.Type.Params.List {
if f == field {
info.FieldIndex = fi
Expand All @@ -299,7 +301,7 @@ func FindParam(pgf *ParsedGoFile, rng protocol.Range) ParamInfo {
break
}
}
return info
return info, nil
}

// signatureRewrite defines a rewritten function signature.
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/source/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func addEmbedImport(ctx context.Context, snapshot *cache.Snapshot, fh file.Handl
for _, e := range protoEdits {
start, end, err := pgf.RangePos(e.Range)
if err != nil {
return nil, fmt.Errorf("map range: %w", err)
return nil, err // e.g. invalid range
}
edits = append(edits, analysis.TextEdit{
Pos: start,
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/source/inline_all.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func inlineAllCalls(ctx context.Context, logf func(string, ...any), snapshot *ca
}
start, end, err := pgf.RangePos(ref.Range)
if err != nil {
return nil, bug.Errorf("RangePos(ref): %v", err)
return nil, err // e.g. invalid range
}

// Look for the surrounding call expression.
Expand Down
11 changes: 6 additions & 5 deletions gopls/internal/server/code_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,15 +543,16 @@ func refactorRewrite(snapshot *cache.Snapshot, pkg *cache.Package, pgf *source.P
// - [start, end) is contained within an unused field or parameter name
// - ... of a non-method function declaration.
func canRemoveParameter(pkg *cache.Package, pgf *source.ParsedGoFile, rng protocol.Range) bool {
info := source.FindParam(pgf, rng)
if info.Decl == nil || info.Field == nil {
return false
info, err := source.FindParam(pgf, rng)
if err != nil {
return false // e.g. invalid range
}
if info.Field == nil {
return false // range does not span a parameter
}

if info.Decl.Body == nil {
return false // external function
}

if len(info.Field.Names) == 0 {
return true // no names => field is unused
}
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/server/semantic.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (s *server) semanticTokens(ctx context.Context, td protocol.TextDocumentIde
var err error
start, end, err = pgf.RangePos(*rng)
if err != nil {
return nil, fmt.Errorf("range span (%w) error for %s", err, pgf.File.Name)
return nil, err // e.g. invalid range
}
} else {
tok := pgf.Tok
Expand Down

0 comments on commit 0640701

Please sign in to comment.