Skip to content

Commit c21ae4c

Browse files
committed
gopls/internal/golang: don't suggest removeparam when there are errors
Update the canRemoveParameter logic to reflect that we don't try to remove parameters when there are parse or type errors. Change-Id: Ia8239a383b56763d100c69c9a3633c87025b81cf Reviewed-on: https://go-review.googlesource.com/c/tools/+/572297 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent f3fccee commit c21ae4c

File tree

3 files changed

+19
-1
lines changed

3 files changed

+19
-1
lines changed

gopls/internal/golang/change_signature.go

+3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ func RemoveUnusedParameter(ctx context.Context, fh file.Handle, rng protocol.Ran
4545
if err != nil {
4646
return nil, err
4747
}
48+
49+
// Changes to our heuristics for whether we can remove a parameter must also
50+
// be reflected in the canRemoveParameter helper.
4851
if perrors, terrors := pkg.ParseErrors(), pkg.TypeErrors(); len(perrors) > 0 || len(terrors) > 0 {
4952
var sample string
5053
if len(perrors) > 0 {

gopls/internal/golang/codeaction.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -401,12 +401,16 @@ func getRewriteCodeActions(ctx context.Context, pkg *cache.Package, snapshot *ca
401401
// indicated by the given [start, end) range.
402402
//
403403
// This is true if:
404+
// - there are no parse or type errors, and
404405
// - [start, end) is contained within an unused field or parameter name
405406
// - ... of a non-method function declaration.
406407
//
407408
// (Note that the unusedparam analyzer also computes this property, but
408409
// much more precisely, allowing it to report its findings as diagnostics.)
409410
func canRemoveParameter(pkg *cache.Package, pgf *parsego.File, rng protocol.Range) bool {
411+
if perrors, terrors := pkg.ParseErrors(), pkg.TypeErrors(); len(perrors) > 0 || len(terrors) > 0 {
412+
return false // can't remove parameters from packages with errors
413+
}
410414
info, err := FindParam(pgf, rng)
411415
if err != nil {
412416
return false // e.g. invalid range
@@ -449,7 +453,7 @@ func getInlineCodeActions(pkg *cache.Package, pgf *parsego.File, rng protocol.Ra
449453
return nil, err
450454
}
451455

452-
// If range is within call expression, offer inline action.
456+
// If range is within call expression, offer to inline the call.
453457
var commands []protocol.Command
454458
if _, fn, err := EnclosingStaticCall(pkg, pgf, start, end); err == nil {
455459
cmd, err := command.NewApplyFixCommand(fmt.Sprintf("Inline call to %s", fn.Name()), command.ApplyFixArgs{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
This test checks that we can't remove parameters for packages with errors.
2+
3+
-- p.go --
4+
package p
5+
6+
func foo(unused int) { //@codeactionerr("unused", "unused", "refactor.rewrite", re"found 0")
7+
}
8+
9+
func _() {
10+
foo("") //@diag(`""`, re"cannot use")
11+
}

0 commit comments

Comments
 (0)