go/analysis/passes/modernize: reflecttypefor: remove orphaned var
This CL fixes a bug in reflecttypefor that caused it to leave
a declaration for a local variable that was no longer used,
which is not legal. Now, it deletes the variable:
- var zero T
- reflect.TypeOf(zero)
+ reflect.TypeFor[T]()
The bookkeeping is done by the new refactor.DeleteUnusedVars function.
Fixes golang/go#75767
Change-Id: If51af1500a63c968098b4a518b941315e826fcbf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/710656
Reviewed-by: Madeline Kalil <mkalil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: shuang cui <imcusg@gmail.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
diff --git a/go/analysis/passes/modernize/modernize_test.go b/go/analysis/passes/modernize/modernize_test.go
index 0ca72ca..64bf60c 100644
--- a/go/analysis/passes/modernize/modernize_test.go
+++ b/go/analysis/passes/modernize/modernize_test.go
@@ -10,6 +10,7 @@
. "golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/modernize"
"golang.org/x/tools/internal/goplsexport"
+ "golang.org/x/tools/internal/testenv"
)
func TestAppendClipped(t *testing.T) {
@@ -61,6 +62,7 @@
}
func TestReflectTypeFor(t *testing.T) {
+ testenv.NeedsGo1Point(t, 25) // requires go1.25 types.Var.Kind
RunWithSuggestedFixes(t, TestData(), modernize.ReflectTypeForAnalyzer, "reflecttypefor")
}
diff --git a/go/analysis/passes/modernize/reflect.go b/go/analysis/passes/modernize/reflect.go
index 1a4e3eb..c9b0fa4 100644
--- a/go/analysis/passes/modernize/reflect.go
+++ b/go/analysis/passes/modernize/reflect.go
@@ -18,6 +18,7 @@
"golang.org/x/tools/internal/analysisinternal/generated"
typeindexanalyzer "golang.org/x/tools/internal/analysisinternal/typeindex"
"golang.org/x/tools/internal/astutil"
+ "golang.org/x/tools/internal/refactor"
"golang.org/x/tools/internal/typesinternal"
"golang.org/x/tools/internal/typesinternal/typeindex"
"golang.org/x/tools/internal/versions"
@@ -66,16 +67,15 @@
obj := typeutil.Callee(info, call2)
if typesinternal.IsMethodNamed(obj, "reflect", "Type", "Elem") {
if ptr, ok := t.(*types.Pointer); ok {
- // Have: reflect.TypeOf(...*T value...).Elem()
- // => reflect.TypeFor[T]()
+ // Have: TypeOf(expr).Elem() where expr : *T
t = ptr.Elem()
- edits = []analysis.TextEdit{
- {
- // delete .Elem()
- Pos: call.End(),
- End: call2.End(),
- },
- }
+ // reflect.TypeOf(expr).Elem()
+ // -------
+ // reflect.TypeOf(expr)
+ edits = []analysis.TextEdit{{
+ Pos: call.End(),
+ End: call2.End(),
+ }}
}
}
}
@@ -92,6 +92,7 @@
if versions.Before(info.FileVersions[file], "go1.22") {
continue // TypeFor requires go1.22
}
+ tokFile := pass.Fset.File(file.Pos())
// Format the type as valid Go syntax.
// TODO(adonovan): FileQualifier needs to respect
@@ -105,6 +106,14 @@
continue // e.g. reflect was dot-imported
}
+ // If the call argument contains the last use
+ // of a variable, as in:
+ // var zero T
+ // reflect.TypeOf(zero)
+ // remove the declaration of that variable.
+ curArg0 := curCall.ChildAt(edge.CallExpr_Args, 0)
+ edits = append(edits, refactor.DeleteUnusedVars(index, info, tokFile, curArg0)...)
+
pass.Report(analysis.Diagnostic{
Pos: call.Fun.Pos(),
End: call.Fun.End(),
diff --git a/go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go b/go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go
index a1042de..6979aa9 100644
--- a/go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go
+++ b/go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go
@@ -19,3 +19,13 @@
_ = reflect.TypeOf(time.Time{}) // want "reflect.TypeOf call can be simplified using TypeFor"
_ = reflect.TypeOf(time.Duration(0)) // want "reflect.TypeOf call can be simplified using TypeFor"
)
+
+// Eliminate local var if we deleted its last use.
+func _() {
+ var zero string
+ _ = reflect.TypeOf(zero) // want "reflect.TypeOf call can be simplified using TypeFor"
+
+ var z2 string
+ _ = reflect.TypeOf(z2) // want "reflect.TypeOf call can be simplified using TypeFor"
+ _ = z2 // z2 has multiple uses
+}
diff --git a/go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go.golden b/go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go.golden
index a3e7c07..3f137a5 100644
--- a/go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go.golden
+++ b/go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go.golden
@@ -20,3 +20,11 @@
_ = reflect.TypeFor[time.Duration]() // want "reflect.TypeOf call can be simplified using TypeFor"
)
+// Eliminate local var if we deleted its last use.
+func _() {
+ _ = reflect.TypeFor[string]() // want "reflect.TypeOf call can be simplified using TypeFor"
+
+ var z2 string
+ _ = reflect.TypeFor[string]() // want "reflect.TypeOf call can be simplified using TypeFor"
+ _ = z2 // z2 has multiple uses
+}
diff --git a/internal/refactor/delete.go b/internal/refactor/delete.go
index aa8ba5a..19c3af1 100644
--- a/internal/refactor/delete.go
+++ b/internal/refactor/delete.go
@@ -18,6 +18,7 @@
"golang.org/x/tools/go/ast/inspector"
"golang.org/x/tools/internal/astutil"
"golang.org/x/tools/internal/typesinternal"
+ "golang.org/x/tools/internal/typesinternal/typeindex"
)
// DeleteVar returns edits to delete the declaration of a variable
@@ -431,3 +432,34 @@
}
return []analysis.TextEdit{edit}
}
+
+// DeleteUnusedVars computes the edits required to delete the
+// declarations of any local variables whose last uses are in the
+// curDelend subtree, which is about to be deleted.
+func DeleteUnusedVars(index *typeindex.Index, info *types.Info, tokFile *token.File, curDelend inspector.Cursor) []analysis.TextEdit {
+ // TODO(adonovan): we might want to generalize this by
+ // splitting the two phases below, so that we can gather
+ // across a whole sequence of deletions then finally compute the
+ // set of variables that are no longer wanted.
+
+ // Count number of deletions of each var.
+ delcount := make(map[*types.Var]int)
+ for curId := range curDelend.Preorder((*ast.Ident)(nil)) {
+ id := curId.Node().(*ast.Ident)
+ if v, ok := info.Uses[id].(*types.Var); ok &&
+ typesinternal.GetVarKind(v) == typesinternal.LocalVar { // always false before go1.25
+ delcount[v]++
+ }
+ }
+
+ // Delete declaration of each var that became unused.
+ var edits []analysis.TextEdit
+ for v, count := range delcount {
+ if len(slices.Collect(index.Uses(v))) == count {
+ if curDefId, ok := index.Def(v); ok {
+ edits = append(edits, DeleteVar(tokFile, info, curDefId)...)
+ }
+ }
+ }
+ return edits
+}
