Skip to content

Commit

Permalink
gopls/internal/lsp/source: address problems detected by staticcheck
Browse files Browse the repository at this point in the history
While working on performance I disabled staticcheck. Reenabling it
today turned up several suggestions, and a few real bugs.

Change-Id: I032d0cfb12ec8fda2573a2663faec4126b321afc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/557496
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
findleyr committed Jan 22, 2024
1 parent 94d99e3 commit e2ca594
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 8 deletions.
6 changes: 5 additions & 1 deletion gopls/internal/lsp/source/change_signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func RemoveUnusedParameter(ctx context.Context, fh file.Handle, rng protocol.Ran
if err != nil {
return nil, err
}

// Finally, rewrite the original declaration. We do this after inlining all
// calls, as there may be calls in the same file as the declaration. But none
// of the inlining should have changed the location of the original
Expand All @@ -149,7 +150,10 @@ func RemoveUnusedParameter(ctx context.Context, fh file.Handle, rng protocol.Ran
src = pgf.Src
}
fset := tokeninternal.FileSetFor(pgf.Tok)
src, err = rewriteSignature(fset, idx, src, newDecl)
src, err := rewriteSignature(fset, idx, src, newDecl)
if err != nil {
return nil, err
}
newContent[pgf.URI] = src
}

Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/lsp/source/codeaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
return nil, err
}
if want[protocol.RefactorRewrite] {
rewrites, err := getRewriteCodeActions(snapshot, pkg, pgf, fh, rng, snapshot.Options())
rewrites, err := getRewriteCodeActions(pkg, pgf, fh, rng, snapshot.Options())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -253,7 +253,7 @@ func newCodeAction(title string, kind protocol.CodeActionKind, cmd *protocol.Com
}

// getRewriteCodeActions returns refactor.rewrite code actions available at the specified range.
func getRewriteCodeActions(snapshot *cache.Snapshot, pkg *cache.Package, pgf *ParsedGoFile, fh file.Handle, rng protocol.Range, options *settings.Options) (_ []protocol.CodeAction, rerr error) {
func getRewriteCodeActions(pkg *cache.Package, pgf *ParsedGoFile, fh file.Handle, rng protocol.Range, options *settings.Options) (_ []protocol.CodeAction, rerr error) {
// golang/go#61693: code actions were refactored to run outside of the
// analysis framework, but as a result they lost their panic recovery.
//
Expand Down
8 changes: 6 additions & 2 deletions gopls/internal/lsp/source/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,10 @@ func builtinDecl(ctx context.Context, snapshot *cache.Snapshot, obj types.Object
if err != nil {
return nil, nil, err
}

decl, err = getDecl(pgf.File, obj.Name())
if err != nil {
return nil, nil, err
}
} else {
// pseudo-package "builtin":
// use parsed $GOROOT/src/builtin/builtin.go
Expand All @@ -163,7 +165,9 @@ func builtinDecl(ctx context.Context, snapshot *cache.Snapshot, obj types.Object
if obj.Parent() == types.Universe {
// built-in function or type
decl, err = getDecl(pgf.File, obj.Name())

if err != nil {
return nil, nil, err
}
} else if obj.Name() == "Error" {
// error.Error method
decl, err = getDecl(pgf.File, "error")
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/source/invertifcondition.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func invertIfCondition(fset *token.FileSet, start, end token.Pos, src []byte, fi
func endsWithReturn(elseBranch ast.Stmt) (bool, error) {
elseBlock, isBlockStatement := elseBranch.(*ast.BlockStmt)
if !isBlockStatement {
return false, fmt.Errorf("Unable to figure out whether this ends with return: %T", elseBranch)
return false, fmt.Errorf("unable to figure out whether this ends with return: %T", elseBranch)
}

if len(elseBlock.List) == 0 {
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/lsp/source/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, f file.Handle
// transitive rdeps. (The exportedness of the field's struct
// or method's receiver is irrelevant.)
transitive := false
switch obj.(type) {
switch obj := obj.(type) {
case *types.TypeName:
// Renaming an exported package-level type
// requires us to inspect all transitive rdeps
Expand All @@ -418,7 +418,7 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, f file.Handle
}

case *types.Var:
if obj.(*types.Var).IsField() {
if obj.IsField() {
transitive = true // field
}

Expand Down

0 comments on commit e2ca594

Please sign in to comment.