Skip to content

Commit 6315bee

Browse files
ldezldez
committed
feat: improve slices reports
1 parent 9eb217f commit 6315bee

File tree

9 files changed

+124
-103
lines changed

9 files changed

+124
-103
lines changed

‎exptostd.go

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ const (
2525
goDevel = 666
2626
)
2727

28+
// Result is step analysis results.
29+
type Result struct {
30+
shouldKeepImport bool
31+
Diagnostics []analysis.Diagnostic
32+
}
33+
2834
type stdReplacement struct {
2935
MinGo int
3036
Text string
@@ -114,7 +120,7 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) {
114120

115121
var shouldKeepExpMaps bool
116122

117-
var shouldKeepExpSlices bool
123+
var resultExpSlices Result
118124

119125
insp.Preorder(nodeFilter, func(n ast.Node) {
120126
if importSpec, ok := n.(*ast.ImportSpec); ok {
@@ -143,17 +149,33 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) {
143149

144150
switch ident.Name {
145151
case "maps":
146-
usage := a.detectPackageUsage(pass, a.mapsPkgReplacements, selExpr, ident, callExpr, "golang.org/x/exp/maps")
152+
diagnostic, usage := a.detectPackageUsage(pass, a.mapsPkgReplacements, selExpr, ident, callExpr, "golang.org/x/exp/maps")
153+
if usage {
154+
pass.Report(diagnostic)
155+
}
156+
147157
shouldKeepExpMaps = shouldKeepExpMaps || !usage
148158

149159
case "slices":
150-
usage := a.detectPackageUsage(pass, a.slicesPkgReplacements, selExpr, ident, callExpr, "golang.org/x/exp/slices")
151-
shouldKeepExpSlices = shouldKeepExpSlices || !usage
160+
diagnostic, usage := a.detectPackageUsage(pass, a.slicesPkgReplacements, selExpr, ident, callExpr, "golang.org/x/exp/slices")
161+
162+
if usage {
163+
resultExpSlices.Diagnostics = append(resultExpSlices.Diagnostics, diagnostic)
164+
}
165+
166+
resultExpSlices.shouldKeepImport = resultExpSlices.shouldKeepImport || !usage
152167
}
153168
})
154169

155170
a.suggestReplaceImport(pass, imports, shouldKeepExpMaps, "golang.org/x/exp/maps")
156-
a.suggestReplaceImport(pass, imports, shouldKeepExpSlices, "golang.org/x/exp/slices")
171+
172+
if resultExpSlices.shouldKeepImport {
173+
for _, diagnostic := range resultExpSlices.Diagnostics {
174+
pass.Report(diagnostic)
175+
}
176+
} else {
177+
a.suggestReplaceImport(pass, imports, resultExpSlices.shouldKeepImport, "golang.org/x/exp/slices")
178+
}
157179

158180
return nil, nil
159181
}
@@ -162,47 +184,45 @@ func (a *analyzer) detectPackageUsage(pass *analysis.Pass,
162184
replacements map[string]stdReplacement,
163185
selExpr *ast.SelectorExpr, ident *ast.Ident, callExpr *ast.CallExpr,
164186
importPath string,
165-
) bool {
187+
) (analysis.Diagnostic, bool) {
166188
rp, ok := replacements[selExpr.Sel.Name]
167189
if !ok {
168-
return false
190+
return analysis.Diagnostic{}, false
169191
}
170192

171193
if !a.skipGoVersionDetection && rp.MinGo > a.goVersion {
172-
return false
194+
return analysis.Diagnostic{}, false
173195
}
174196

175197
obj := pass.TypesInfo.Uses[ident]
176198
if obj == nil {
177-
return false
199+
return analysis.Diagnostic{}, false
178200
}
179201

180202
pkg, ok := obj.(*types.PkgName)
181203
if !ok {
182-
return false
204+
return analysis.Diagnostic{}, false
183205
}
184206

185-
if pkg.Imported().Path() == importPath {
186-
diagnostic := analysis.Diagnostic{
187-
Pos: callExpr.Pos(),
188-
Message: fmt.Sprintf("%s.%s() can be replaced by %s", importPath, selExpr.Sel.Name, rp.Text),
189-
}
190-
191-
if rp.Suggested != nil {
192-
fix, err := rp.Suggested(callExpr)
193-
if err != nil {
194-
diagnostic.Message = fmt.Sprintf("Suggested fix error: %v", err)
195-
} else {
196-
diagnostic.SuggestedFixes = append(diagnostic.SuggestedFixes, fix)
197-
}
198-
}
207+
if pkg.Imported().Path() != importPath {
208+
return analysis.Diagnostic{}, false
209+
}
199210

200-
pass.Report(diagnostic)
211+
diagnostic := analysis.Diagnostic{
212+
Pos: callExpr.Pos(),
213+
Message: fmt.Sprintf("%s.%s() can be replaced by %s", importPath, selExpr.Sel.Name, rp.Text),
214+
}
201215

202-
return true
216+
if rp.Suggested != nil {
217+
fix, err := rp.Suggested(callExpr)
218+
if err != nil {
219+
diagnostic.Message = fmt.Sprintf("Suggested fix error: %v", err)
220+
} else {
221+
diagnostic.SuggestedFixes = append(diagnostic.SuggestedFixes, fix)
222+
}
203223
}
204224

205-
return false
225+
return diagnostic, true
206226
}
207227

208228
func (a *analyzer) suggestReplaceImport(pass *analysis.Pass, imports map[string]*ast.ImportSpec, shouldKeep bool, importPath string) {

‎testdata/src/expalias/alias.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@ package expalias
22

33
import (
44
"golang.org/x/exp/maps" // want "Import statement 'golang.org/x/exp/maps' can be replaced by 'maps'"
5-
aliased "golang.org/x/exp/maps"
5+
aliasMaps "golang.org/x/exp/maps"
66
"golang.org/x/exp/slices" // want "Import statement 'golang.org/x/exp/slices' can be replaced by 'slices'"
7+
aliasSlices "golang.org/x/exp/slices"
78
)
89

910
func _(m map[string]string, a, b []string) {
10-
aliased.Clone(m)
11-
maps.Clone(m) // want `golang.org/x/exp/maps\.Clone\(\) can be replaced by maps\.Clone\(\)`
12-
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
11+
aliasMaps.Clone(m)
12+
maps.Clone(m) // want `golang.org/x/exp/maps\.Clone\(\) can be replaced by maps\.Clone\(\)`
13+
aliasSlices.Equal(a, b)
14+
slices.Equal(a, b)
1315
}

‎testdata/src/expalias/alias.go.golden

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package expalias
22

33
import (
4-
"maps" // want "Import statement 'golang.org/x/exp/maps' can be replaced by 'maps'"
5-
aliased "golang.org/x/exp/maps"
4+
aliasMaps "golang.org/x/exp/maps"
5+
aliasSlices "golang.org/x/exp/slices"
6+
"maps" // want "Import statement 'golang.org/x/exp/maps' can be replaced by 'maps'"
67
"slices" // want "Import statement 'golang.org/x/exp/slices' can be replaced by 'slices'"
78
)
89

910
func _(m map[string]string, a, b []string) {
10-
aliased.Clone(m)
11-
maps.Clone(m) // want `golang.org/x/exp/maps\.Clone\(\) can be replaced by maps\.Clone\(\)`
12-
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
11+
aliasMaps.Clone(m)
12+
maps.Clone(m) // want `golang.org/x/exp/maps\.Clone\(\) can be replaced by maps\.Clone\(\)`
13+
aliasSlices.Equal(a, b)
14+
slices.Equal(a, b)
1315
}

‎testdata/src/expboth/both.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ import (
66
)
77

88
func _(m map[string]string, a, b []string) {
9-
maps.Clone(m) // want `golang.org/x/exp/maps.Clone\(\) can be replaced by maps.Clone\(\)`
10-
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
9+
maps.Clone(m) // want `golang.org/x/exp/maps.Clone\(\) can be replaced by maps.Clone\(\)`
10+
slices.Equal(a, b)
1111
}

‎testdata/src/expboth/both.go.golden

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
)
77

88
func _(m map[string]string, a, b []string) {
9-
maps.Clone(m) // want `golang.org/x/exp/maps.Clone\(\) can be replaced by maps.Clone\(\)`
10-
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
9+
maps.Clone(m) // want `golang.org/x/exp/maps.Clone\(\) can be replaced by maps.Clone\(\)`
10+
slices.Equal(a, b)
1111
}
12-

‎testdata/src/expmixed/mixed.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ import (
88

99
func _(m map[string]string, a, b []string) {
1010
maps.Clone(m)
11-
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
11+
slices.Equal(a, b)
1212
}

‎testdata/src/expmixed/mixed.go.golden

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,5 @@ import (
88

99
func _(m map[string]string, a, b []string) {
1010
maps.Clone(m)
11-
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
11+
slices.Equal(a, b)
1212
}
13-

‎testdata/src/expslices/slices.go

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,57 +5,57 @@ import (
55
)
66

77
func _(a, b []string) {
8-
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
9-
slices.EqualFunc(a, b, func(_ string, _ string) bool { // want `golang.org/x/exp/slices\.EqualFunc\(\) can be replaced by slices\.EqualFunc\(\)`
8+
slices.Equal(a, b)
9+
slices.EqualFunc(a, b, func(_ string, _ string) bool {
1010
return true
1111
})
12-
slices.Compare(a, b) // want `golang.org/x/exp/slices\.Compare\(\) can be replaced by slices\.Compare\(\)`
13-
slices.CompareFunc(a, b, func(_ string, _ string) int { // want `golang.org/x/exp/slices\.CompareFunc\(\) can be replaced by slices\.CompareFunc\(\)`
12+
slices.Compare(a, b)
13+
slices.CompareFunc(a, b, func(_ string, _ string) int {
1414
return 0
1515
})
16-
slices.Index(a, "a") // want `golang.org/x/exp/slices\.Index\(\) can be replaced by slices\.Index\(\)`
17-
slices.IndexFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.IndexFunc\(\) can be replaced by slices\.IndexFunc\(\)`
16+
slices.Index(a, "a")
17+
slices.IndexFunc(a, func(_ string) bool {
1818
return true
1919
})
20-
slices.Contains(a, "a") // want `golang.org/x/exp/slices\.Contains\(\) can be replaced by slices\.Contains\(\)`
21-
slices.ContainsFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.ContainsFunc\(\) can be replaced by slices\.ContainsFunc\(\)`
20+
slices.Contains(a, "a")
21+
slices.ContainsFunc(a, func(_ string) bool {
2222
return true
2323
})
24-
slices.Insert(a, 0, "a", "b") // want `golang.org/x/exp/slices\.Insert\(\) can be replaced by slices\.Insert\(\)`
25-
slices.Delete(a, 0, 1) // want `golang.org/x/exp/slices\.Delete\(\) can be replaced by slices\.Delete\(\)`
26-
slices.DeleteFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.DeleteFunc\(\) can be replaced by slices\.DeleteFunc\(\)`
24+
slices.Insert(a, 0, "a", "b")
25+
slices.Delete(a, 0, 1)
26+
slices.DeleteFunc(a, func(_ string) bool {
2727
return true
2828
})
29-
slices.Replace(a, 0, 1, "a") // want `golang.org/x/exp/slices\.Replace\(\) can be replaced by slices\.Replace\(\)`
30-
slices.Clone(a) // want `golang.org/x/exp/slices\.Clone\(\) can be replaced by slices\.Clone\(\)`
31-
slices.Compact(a) // want `golang.org/x/exp/slices\.Compact\(\) can be replaced by slices\.Compact\(\)`
32-
slices.CompactFunc(a, func(_ string, _ string) bool { // want `golang.org/x/exp/slices\.CompactFunc\(\) can be replaced by slices\.CompactFunc\(\)`
29+
slices.Replace(a, 0, 1, "a")
30+
slices.Clone(a)
31+
slices.Compact(a)
32+
slices.CompactFunc(a, func(_ string, _ string) bool {
3333
return true
3434
})
35-
slices.Grow(a, 2) // want `golang.org/x/exp/slices\.Grow\(\) can be replaced by slices\.Grow\(\)`
36-
slices.Clip(a) // want `golang.org/x/exp/slices\.Clip\(\) can be replaced by slices\.Clip\(\)`
37-
slices.Reverse(a) // want `golang.org/x/exp/slices\.Reverse\(\) can be replaced by slices\.Reverse\(\)`
38-
slices.Sort(a) // want `golang.org/x/exp/slices\.Sort\(\) can be replaced by slices\.Sort\(\)`
39-
slices.SortFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.SortFunc\(\) can be replaced by slices\.SortFunc\(\)`
35+
slices.Grow(a, 2)
36+
slices.Clip(a)
37+
slices.Reverse(a)
38+
slices.Sort(a)
39+
slices.SortFunc(a, func(_, _ string) int {
4040
return 0
4141
})
42-
slices.SortStableFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.SortStableFunc\(\) can be replaced by slices\.SortStableFunc\(\)`
42+
slices.SortStableFunc(a, func(_, _ string) int {
4343
return 0
4444
})
45-
slices.IsSorted(a) // want `golang.org/x/exp/slices\.IsSorted\(\) can be replaced by slices\.IsSorted\(\)`
46-
slices.IsSortedFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.IsSortedFunc\(\) can be replaced by slices\.IsSortedFunc\(\)`
45+
slices.IsSorted(a)
46+
slices.IsSortedFunc(a, func(_, _ string) int {
4747
return 0
4848
})
49-
slices.Min(a) // want `golang.org/x/exp/slices\.Min\(\) can be replaced by slices\.Min\(\)`
50-
slices.MinFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.MinFunc\(\) can be replaced by slices\.MinFunc\(\)`
49+
slices.Min(a)
50+
slices.MinFunc(a, func(_, _ string) int {
5151
return 0
5252
})
53-
slices.Max(a) // want `golang.org/x/exp/slices\.Max\(\) can be replaced by slices\.Max\(\)`
54-
slices.MaxFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.MaxFunc\(\) can be replaced by slices\.MaxFunc\(\)`
53+
slices.Max(a)
54+
slices.MaxFunc(a, func(_, _ string) int {
5555
return 0
5656
})
57-
slices.BinarySearch(a, "a") // want `golang.org/x/exp/slices\.BinarySearch\(\) can be replaced by slices\.BinarySearch\(\)`
58-
slices.BinarySearchFunc(a, b, func(_ string, _ []string) int { // want `golang.org/x/exp/slices\.BinarySearchFunc\(\) can be replaced by slices\.BinarySearchFunc\(\)`
57+
slices.BinarySearch(a, "a")
58+
slices.BinarySearchFunc(a, b, func(_ string, _ []string) int {
5959
return 0
6060
})
6161
}

‎testdata/src/expslices/slices.go.golden

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,58 +5,57 @@ import (
55
)
66

77
func _(a, b []string) {
8-
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
9-
slices.EqualFunc(a, b, func(_ string, _ string) bool { // want `golang.org/x/exp/slices\.EqualFunc\(\) can be replaced by slices\.EqualFunc\(\)`
8+
slices.Equal(a, b)
9+
slices.EqualFunc(a, b, func(_ string, _ string) bool {
1010
return true
1111
})
12-
slices.Compare(a, b) // want `golang.org/x/exp/slices\.Compare\(\) can be replaced by slices\.Compare\(\)`
13-
slices.CompareFunc(a, b, func(_ string, _ string) int { // want `golang.org/x/exp/slices\.CompareFunc\(\) can be replaced by slices\.CompareFunc\(\)`
12+
slices.Compare(a, b)
13+
slices.CompareFunc(a, b, func(_ string, _ string) int {
1414
return 0
1515
})
16-
slices.Index(a, "a") // want `golang.org/x/exp/slices\.Index\(\) can be replaced by slices\.Index\(\)`
17-
slices.IndexFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.IndexFunc\(\) can be replaced by slices\.IndexFunc\(\)`
16+
slices.Index(a, "a")
17+
slices.IndexFunc(a, func(_ string) bool {
1818
return true
1919
})
20-
slices.Contains(a, "a") // want `golang.org/x/exp/slices\.Contains\(\) can be replaced by slices\.Contains\(\)`
21-
slices.ContainsFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.ContainsFunc\(\) can be replaced by slices\.ContainsFunc\(\)`
20+
slices.Contains(a, "a")
21+
slices.ContainsFunc(a, func(_ string) bool {
2222
return true
2323
})
24-
slices.Insert(a, 0, "a", "b") // want `golang.org/x/exp/slices\.Insert\(\) can be replaced by slices\.Insert\(\)`
25-
slices.Delete(a, 0, 1) // want `golang.org/x/exp/slices\.Delete\(\) can be replaced by slices\.Delete\(\)`
26-
slices.DeleteFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.DeleteFunc\(\) can be replaced by slices\.DeleteFunc\(\)`
24+
slices.Insert(a, 0, "a", "b")
25+
slices.Delete(a, 0, 1)
26+
slices.DeleteFunc(a, func(_ string) bool {
2727
return true
2828
})
29-
slices.Replace(a, 0, 1, "a") // want `golang.org/x/exp/slices\.Replace\(\) can be replaced by slices\.Replace\(\)`
30-
slices.Clone(a) // want `golang.org/x/exp/slices\.Clone\(\) can be replaced by slices\.Clone\(\)`
31-
slices.Compact(a) // want `golang.org/x/exp/slices\.Compact\(\) can be replaced by slices\.Compact\(\)`
32-
slices.CompactFunc(a, func(_ string, _ string) bool { // want `golang.org/x/exp/slices\.CompactFunc\(\) can be replaced by slices\.CompactFunc\(\)`
29+
slices.Replace(a, 0, 1, "a")
30+
slices.Clone(a)
31+
slices.Compact(a)
32+
slices.CompactFunc(a, func(_ string, _ string) bool {
3333
return true
3434
})
35-
slices.Grow(a, 2) // want `golang.org/x/exp/slices\.Grow\(\) can be replaced by slices\.Grow\(\)`
36-
slices.Clip(a) // want `golang.org/x/exp/slices\.Clip\(\) can be replaced by slices\.Clip\(\)`
37-
slices.Reverse(a) // want `golang.org/x/exp/slices\.Reverse\(\) can be replaced by slices\.Reverse\(\)`
38-
slices.Sort(a) // want `golang.org/x/exp/slices\.Sort\(\) can be replaced by slices\.Sort\(\)`
39-
slices.SortFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.SortFunc\(\) can be replaced by slices\.SortFunc\(\)`
35+
slices.Grow(a, 2)
36+
slices.Clip(a)
37+
slices.Reverse(a)
38+
slices.Sort(a)
39+
slices.SortFunc(a, func(_, _ string) int {
4040
return 0
4141
})
42-
slices.SortStableFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.SortStableFunc\(\) can be replaced by slices\.SortStableFunc\(\)`
42+
slices.SortStableFunc(a, func(_, _ string) int {
4343
return 0
4444
})
45-
slices.IsSorted(a) // want `golang.org/x/exp/slices\.IsSorted\(\) can be replaced by slices\.IsSorted\(\)`
46-
slices.IsSortedFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.IsSortedFunc\(\) can be replaced by slices\.IsSortedFunc\(\)`
45+
slices.IsSorted(a)
46+
slices.IsSortedFunc(a, func(_, _ string) int {
4747
return 0
4848
})
49-
slices.Min(a) // want `golang.org/x/exp/slices\.Min\(\) can be replaced by slices\.Min\(\)`
50-
slices.MinFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.MinFunc\(\) can be replaced by slices\.MinFunc\(\)`
49+
slices.Min(a)
50+
slices.MinFunc(a, func(_, _ string) int {
5151
return 0
5252
})
53-
slices.Max(a) // want `golang.org/x/exp/slices\.Max\(\) can be replaced by slices\.Max\(\)`
54-
slices.MaxFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.MaxFunc\(\) can be replaced by slices\.MaxFunc\(\)`
53+
slices.Max(a)
54+
slices.MaxFunc(a, func(_, _ string) int {
5555
return 0
5656
})
57-
slices.BinarySearch(a, "a") // want `golang.org/x/exp/slices\.BinarySearch\(\) can be replaced by slices\.BinarySearch\(\)`
58-
slices.BinarySearchFunc(a, b, func(_ string, _ []string) int { // want `golang.org/x/exp/slices\.BinarySearchFunc\(\) can be replaced by slices\.BinarySearchFunc\(\)`
57+
slices.BinarySearch(a, "a")
58+
slices.BinarySearchFunc(a, b, func(_ string, _ []string) int {
5959
return 0
6060
})
6161
}
62-

0 commit comments

Comments
 (0)