Skip to content

Commit 084551f

Browse files
kwjwgopherbot
authored andcommitted
go/analysis/passes/maprange: check for redundant Keys/Values calls
Add an analyzer for redundant use of the functions maps.Keys and maps.Values in "for" statements with "range" clauses. Updates golang/go#72908 Change-Id: I19589dc42fa9cc2465c6d5aa1542175af6aaa6ea Reviewed-on: https://go-review.googlesource.com/c/tools/+/658695 Auto-Submit: Alan Donovan <[email protected]> Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 9abefc5 commit 084551f

File tree

6 files changed

+500
-0
lines changed

6 files changed

+500
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// The maprange command applies the golang.org/x/tools/gopls/internal/analysis/maprange
6+
// analysis to the specified packages of Go source code.
7+
package main
8+
9+
import (
10+
"golang.org/x/tools/go/analysis/singlechecker"
11+
"golang.org/x/tools/gopls/internal/analysis/maprange"
12+
)
13+
14+
func main() { singlechecker.Main(maprange.Analyzer) }
+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Package maprange defines an Analyzer that checks for redundant use
6+
// of the functions maps.Keys and maps.Values in "for" statements with
7+
// "range" clauses.
8+
//
9+
// # Analyzer maprange
10+
//
11+
// maprange: checks for unnecessary calls to maps.Keys and maps.Values in range statements
12+
//
13+
// Consider a loop written like this:
14+
//
15+
// for val := range maps.Values(m) {
16+
// fmt.Println(val)
17+
// }
18+
//
19+
// This should instead be written without the call to maps.Values:
20+
//
21+
// for _, val := range m {
22+
// fmt.Println(val)
23+
// }
24+
//
25+
// golang.org/x/exp/maps returns slices for Keys/Values instead of iterators,
26+
// but unnecessary calls should similarly be removed:
27+
//
28+
// for _, key := range maps.Keys(m) {
29+
// fmt.Println(key)
30+
// }
31+
//
32+
// should be rewritten as:
33+
//
34+
// for key := range m {
35+
// fmt.Println(key)
36+
// }
37+
package maprange
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package maprange
6+
7+
import (
8+
_ "embed"
9+
"fmt"
10+
"go/ast"
11+
"golang.org/x/tools/go/analysis"
12+
"golang.org/x/tools/go/analysis/passes/inspect"
13+
"golang.org/x/tools/go/ast/inspector"
14+
"golang.org/x/tools/go/types/typeutil"
15+
"golang.org/x/tools/internal/analysisinternal"
16+
"golang.org/x/tools/internal/versions"
17+
)
18+
19+
//go:embed doc.go
20+
var doc string
21+
22+
var Analyzer = &analysis.Analyzer{
23+
Name: "maprange",
24+
Doc: analysisinternal.MustExtractDoc(doc, "maprange"),
25+
URL: "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/maprange",
26+
Requires: []*analysis.Analyzer{inspect.Analyzer},
27+
Run: run,
28+
}
29+
30+
// This is a variable because the package name is different in Google's code base.
31+
var xmaps = "golang.org/x/exp/maps"
32+
33+
func run(pass *analysis.Pass) (any, error) {
34+
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
35+
36+
switch pass.Pkg.Path() {
37+
case "maps", xmaps:
38+
// These packages know how to use their own APIs.
39+
return nil, nil
40+
}
41+
42+
if !(analysisinternal.Imports(pass.Pkg, "maps") || analysisinternal.Imports(pass.Pkg, xmaps)) {
43+
return nil, nil // fast path
44+
}
45+
46+
inspect.Preorder([]ast.Node{(*ast.RangeStmt)(nil)}, func(n ast.Node) {
47+
rangeStmt, ok := n.(*ast.RangeStmt)
48+
if !ok {
49+
return
50+
}
51+
call, ok := rangeStmt.X.(*ast.CallExpr)
52+
if !ok {
53+
return
54+
}
55+
callee := typeutil.Callee(pass.TypesInfo, call)
56+
if !analysisinternal.IsFunctionNamed(callee, "maps", "Keys", "Values") &&
57+
!analysisinternal.IsFunctionNamed(callee, xmaps, "Keys", "Values") {
58+
return
59+
}
60+
version := pass.Pkg.GoVersion()
61+
pkg, fn := callee.Pkg().Path(), callee.Name()
62+
key, value := rangeStmt.Key, rangeStmt.Value
63+
64+
edits := editRangeStmt(pass, version, pkg, fn, key, value, call)
65+
if len(edits) > 0 {
66+
pass.Report(analysis.Diagnostic{
67+
Pos: call.Pos(),
68+
End: call.End(),
69+
Message: fmt.Sprintf("unnecessary and inefficient call of %s.%s", pkg, fn),
70+
SuggestedFixes: []analysis.SuggestedFix{{
71+
Message: fmt.Sprintf("Remove unnecessary call to %s.%s", pkg, fn),
72+
TextEdits: edits,
73+
}},
74+
})
75+
}
76+
})
77+
78+
return nil, nil
79+
}
80+
81+
// editRangeStmt returns edits to transform a range statement that calls
82+
// maps.Keys or maps.Values (either the stdlib or the x/exp/maps version).
83+
//
84+
// It reports a diagnostic if an edit cannot be made because the Go version is too old.
85+
//
86+
// It returns nil if no edits are needed.
87+
func editRangeStmt(pass *analysis.Pass, version, pkg, fn string, key, value ast.Expr, call *ast.CallExpr) []analysis.TextEdit {
88+
var edits []analysis.TextEdit
89+
90+
// Check if the call to maps.Keys or maps.Values can be removed/replaced.
91+
// Example:
92+
// for range maps.Keys(m)
93+
// ^^^^^^^^^ removeCall
94+
// for i, _ := range maps.Keys(m)
95+
// ^^^^^^^^^ replace with `len`
96+
//
97+
// If we have: for i, k := range maps.Keys(m) (only possible using x/exp/maps)
98+
// or: for i, v = range maps.Values(m)
99+
// do not remove the call.
100+
removeCall := !isSet(key) || !isSet(value)
101+
replace := ""
102+
if pkg == xmaps && isSet(key) && value == nil {
103+
// If we have: for i := range maps.Keys(m) (using x/exp/maps),
104+
// Replace with: for i := range len(m)
105+
replace = "len"
106+
}
107+
if removeCall {
108+
edits = append(edits, analysis.TextEdit{
109+
Pos: call.Fun.Pos(),
110+
End: call.Fun.End(),
111+
NewText: []byte(replace)})
112+
}
113+
// Check if the key of the range statement should be removed.
114+
// Example:
115+
// for _, k := range maps.Keys(m)
116+
// ^^^ removeKey ^^^^^^^^^ removeCall
117+
removeKey := pkg == xmaps && fn == "Keys" && !isSet(key) && isSet(value)
118+
if removeKey {
119+
edits = append(edits, analysis.TextEdit{
120+
Pos: key.Pos(),
121+
End: value.Pos(),
122+
})
123+
}
124+
// Check if a key should be inserted to the range statement.
125+
// Example:
126+
// for _, v := range maps.Values(m)
127+
// ^^^ addKey ^^^^^^^^^^^ removeCall
128+
addKey := pkg == "maps" && fn == "Values" && isSet(key)
129+
if addKey {
130+
edits = append(edits, analysis.TextEdit{
131+
Pos: key.Pos(),
132+
End: key.Pos(),
133+
NewText: []byte("_, "),
134+
})
135+
}
136+
137+
// Range over int was added in Go 1.22.
138+
// If the Go version is too old, report a diagnostic but do not make any edits.
139+
if replace == "len" && versions.Before(pass.Pkg.GoVersion(), versions.Go1_22) {
140+
pass.Report(analysis.Diagnostic{
141+
Pos: call.Pos(),
142+
End: call.End(),
143+
Message: fmt.Sprintf("likely incorrect use of %s.%s (returns a slice)", pkg, fn),
144+
})
145+
return nil
146+
}
147+
148+
return edits
149+
}
150+
151+
// isSet reports whether an ast.Expr is a non-nil expression that is not the blank identifier.
152+
func isSet(expr ast.Expr) bool {
153+
ident, ok := expr.(*ast.Ident)
154+
return expr != nil && (!ok || ident.Name != "_")
155+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package maprange_test
6+
7+
import (
8+
"golang.org/x/tools/go/analysis/analysistest"
9+
"golang.org/x/tools/gopls/internal/analysis/maprange"
10+
"golang.org/x/tools/internal/testfiles"
11+
"path/filepath"
12+
"testing"
13+
)
14+
15+
func TestBasic(t *testing.T) {
16+
dir := testfiles.ExtractTxtarFileToTmp(t, filepath.Join(analysistest.TestData(), "basic.txtar"))
17+
analysistest.RunWithSuggestedFixes(t, dir, maprange.Analyzer, "maprange")
18+
}
19+
20+
func TestOld(t *testing.T) {
21+
dir := testfiles.ExtractTxtarFileToTmp(t, filepath.Join(analysistest.TestData(), "old.txtar"))
22+
analysistest.RunWithSuggestedFixes(t, dir, maprange.Analyzer, "maprange")
23+
}

0 commit comments

Comments
 (0)