Skip to content

Commit

Permalink
feat: improve slices reports
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez committed Dec 30, 2024
1 parent 9eb217f commit 6315bee
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 103 deletions.
74 changes: 47 additions & 27 deletions exptostd.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ const (
goDevel = 666
)

// Result is step analysis results.
type Result struct {
shouldKeepImport bool
Diagnostics []analysis.Diagnostic
}

type stdReplacement struct {
MinGo int
Text string
Expand Down Expand Up @@ -114,7 +120,7 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) {

var shouldKeepExpMaps bool

var shouldKeepExpSlices bool
var resultExpSlices Result

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

switch ident.Name {
case "maps":
usage := a.detectPackageUsage(pass, a.mapsPkgReplacements, selExpr, ident, callExpr, "golang.org/x/exp/maps")
diagnostic, usage := a.detectPackageUsage(pass, a.mapsPkgReplacements, selExpr, ident, callExpr, "golang.org/x/exp/maps")
if usage {
pass.Report(diagnostic)
}

shouldKeepExpMaps = shouldKeepExpMaps || !usage

case "slices":
usage := a.detectPackageUsage(pass, a.slicesPkgReplacements, selExpr, ident, callExpr, "golang.org/x/exp/slices")
shouldKeepExpSlices = shouldKeepExpSlices || !usage
diagnostic, usage := a.detectPackageUsage(pass, a.slicesPkgReplacements, selExpr, ident, callExpr, "golang.org/x/exp/slices")

if usage {
resultExpSlices.Diagnostics = append(resultExpSlices.Diagnostics, diagnostic)
}

resultExpSlices.shouldKeepImport = resultExpSlices.shouldKeepImport || !usage
}
})

a.suggestReplaceImport(pass, imports, shouldKeepExpMaps, "golang.org/x/exp/maps")
a.suggestReplaceImport(pass, imports, shouldKeepExpSlices, "golang.org/x/exp/slices")

if resultExpSlices.shouldKeepImport {
for _, diagnostic := range resultExpSlices.Diagnostics {
pass.Report(diagnostic)
}
} else {
a.suggestReplaceImport(pass, imports, resultExpSlices.shouldKeepImport, "golang.org/x/exp/slices")
}

return nil, nil
}
Expand All @@ -162,47 +184,45 @@ func (a *analyzer) detectPackageUsage(pass *analysis.Pass,
replacements map[string]stdReplacement,
selExpr *ast.SelectorExpr, ident *ast.Ident, callExpr *ast.CallExpr,
importPath string,
) bool {
) (analysis.Diagnostic, bool) {
rp, ok := replacements[selExpr.Sel.Name]
if !ok {
return false
return analysis.Diagnostic{}, false
}

if !a.skipGoVersionDetection && rp.MinGo > a.goVersion {
return false
return analysis.Diagnostic{}, false
}

obj := pass.TypesInfo.Uses[ident]
if obj == nil {
return false
return analysis.Diagnostic{}, false
}

pkg, ok := obj.(*types.PkgName)
if !ok {
return false
return analysis.Diagnostic{}, false
}

if pkg.Imported().Path() == importPath {
diagnostic := analysis.Diagnostic{
Pos: callExpr.Pos(),
Message: fmt.Sprintf("%s.%s() can be replaced by %s", importPath, selExpr.Sel.Name, rp.Text),
}

if rp.Suggested != nil {
fix, err := rp.Suggested(callExpr)
if err != nil {
diagnostic.Message = fmt.Sprintf("Suggested fix error: %v", err)
} else {
diagnostic.SuggestedFixes = append(diagnostic.SuggestedFixes, fix)
}
}
if pkg.Imported().Path() != importPath {
return analysis.Diagnostic{}, false
}

pass.Report(diagnostic)
diagnostic := analysis.Diagnostic{
Pos: callExpr.Pos(),
Message: fmt.Sprintf("%s.%s() can be replaced by %s", importPath, selExpr.Sel.Name, rp.Text),
}

return true
if rp.Suggested != nil {
fix, err := rp.Suggested(callExpr)
if err != nil {
diagnostic.Message = fmt.Sprintf("Suggested fix error: %v", err)
} else {
diagnostic.SuggestedFixes = append(diagnostic.SuggestedFixes, fix)
}
}

return false
return diagnostic, true
}

func (a *analyzer) suggestReplaceImport(pass *analysis.Pass, imports map[string]*ast.ImportSpec, shouldKeep bool, importPath string) {
Expand Down
10 changes: 6 additions & 4 deletions testdata/src/expalias/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package expalias

import (
"golang.org/x/exp/maps" // want "Import statement 'golang.org/x/exp/maps' can be replaced by 'maps'"
aliased "golang.org/x/exp/maps"
aliasMaps "golang.org/x/exp/maps"
"golang.org/x/exp/slices" // want "Import statement 'golang.org/x/exp/slices' can be replaced by 'slices'"
aliasSlices "golang.org/x/exp/slices"
)

func _(m map[string]string, a, b []string) {
aliased.Clone(m)
maps.Clone(m) // want `golang.org/x/exp/maps\.Clone\(\) can be replaced by maps\.Clone\(\)`
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
aliasMaps.Clone(m)
maps.Clone(m) // want `golang.org/x/exp/maps\.Clone\(\) can be replaced by maps\.Clone\(\)`
aliasSlices.Equal(a, b)
slices.Equal(a, b)
}
12 changes: 7 additions & 5 deletions testdata/src/expalias/alias.go.golden
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package expalias

import (
"maps" // want "Import statement 'golang.org/x/exp/maps' can be replaced by 'maps'"
aliased "golang.org/x/exp/maps"
aliasMaps "golang.org/x/exp/maps"
aliasSlices "golang.org/x/exp/slices"
"maps" // want "Import statement 'golang.org/x/exp/maps' can be replaced by 'maps'"
"slices" // want "Import statement 'golang.org/x/exp/slices' can be replaced by 'slices'"
)

func _(m map[string]string, a, b []string) {
aliased.Clone(m)
maps.Clone(m) // want `golang.org/x/exp/maps\.Clone\(\) can be replaced by maps\.Clone\(\)`
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
aliasMaps.Clone(m)
maps.Clone(m) // want `golang.org/x/exp/maps\.Clone\(\) can be replaced by maps\.Clone\(\)`
aliasSlices.Equal(a, b)
slices.Equal(a, b)
}
4 changes: 2 additions & 2 deletions testdata/src/expboth/both.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ import (
)

func _(m map[string]string, a, b []string) {
maps.Clone(m) // want `golang.org/x/exp/maps.Clone\(\) can be replaced by maps.Clone\(\)`
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
maps.Clone(m) // want `golang.org/x/exp/maps.Clone\(\) can be replaced by maps.Clone\(\)`
slices.Equal(a, b)
}
5 changes: 2 additions & 3 deletions testdata/src/expboth/both.go.golden
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
)

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

2 changes: 1 addition & 1 deletion testdata/src/expmixed/mixed.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ import (

func _(m map[string]string, a, b []string) {
maps.Clone(m)
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
slices.Equal(a, b)
}
3 changes: 1 addition & 2 deletions testdata/src/expmixed/mixed.go.golden
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,5 @@ import (

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

58 changes: 29 additions & 29 deletions testdata/src/expslices/slices.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,57 +5,57 @@ import (
)

func _(a, b []string) {
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
slices.EqualFunc(a, b, func(_ string, _ string) bool { // want `golang.org/x/exp/slices\.EqualFunc\(\) can be replaced by slices\.EqualFunc\(\)`
slices.Equal(a, b)
slices.EqualFunc(a, b, func(_ string, _ string) bool {
return true
})
slices.Compare(a, b) // want `golang.org/x/exp/slices\.Compare\(\) can be replaced by slices\.Compare\(\)`
slices.CompareFunc(a, b, func(_ string, _ string) int { // want `golang.org/x/exp/slices\.CompareFunc\(\) can be replaced by slices\.CompareFunc\(\)`
slices.Compare(a, b)
slices.CompareFunc(a, b, func(_ string, _ string) int {
return 0
})
slices.Index(a, "a") // want `golang.org/x/exp/slices\.Index\(\) can be replaced by slices\.Index\(\)`
slices.IndexFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.IndexFunc\(\) can be replaced by slices\.IndexFunc\(\)`
slices.Index(a, "a")
slices.IndexFunc(a, func(_ string) bool {
return true
})
slices.Contains(a, "a") // want `golang.org/x/exp/slices\.Contains\(\) can be replaced by slices\.Contains\(\)`
slices.ContainsFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.ContainsFunc\(\) can be replaced by slices\.ContainsFunc\(\)`
slices.Contains(a, "a")
slices.ContainsFunc(a, func(_ string) bool {
return true
})
slices.Insert(a, 0, "a", "b") // want `golang.org/x/exp/slices\.Insert\(\) can be replaced by slices\.Insert\(\)`
slices.Delete(a, 0, 1) // want `golang.org/x/exp/slices\.Delete\(\) can be replaced by slices\.Delete\(\)`
slices.DeleteFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.DeleteFunc\(\) can be replaced by slices\.DeleteFunc\(\)`
slices.Insert(a, 0, "a", "b")
slices.Delete(a, 0, 1)
slices.DeleteFunc(a, func(_ string) bool {
return true
})
slices.Replace(a, 0, 1, "a") // want `golang.org/x/exp/slices\.Replace\(\) can be replaced by slices\.Replace\(\)`
slices.Clone(a) // want `golang.org/x/exp/slices\.Clone\(\) can be replaced by slices\.Clone\(\)`
slices.Compact(a) // want `golang.org/x/exp/slices\.Compact\(\) can be replaced by slices\.Compact\(\)`
slices.CompactFunc(a, func(_ string, _ string) bool { // want `golang.org/x/exp/slices\.CompactFunc\(\) can be replaced by slices\.CompactFunc\(\)`
slices.Replace(a, 0, 1, "a")
slices.Clone(a)
slices.Compact(a)
slices.CompactFunc(a, func(_ string, _ string) bool {
return true
})
slices.Grow(a, 2) // want `golang.org/x/exp/slices\.Grow\(\) can be replaced by slices\.Grow\(\)`
slices.Clip(a) // want `golang.org/x/exp/slices\.Clip\(\) can be replaced by slices\.Clip\(\)`
slices.Reverse(a) // want `golang.org/x/exp/slices\.Reverse\(\) can be replaced by slices\.Reverse\(\)`
slices.Sort(a) // want `golang.org/x/exp/slices\.Sort\(\) can be replaced by slices\.Sort\(\)`
slices.SortFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.SortFunc\(\) can be replaced by slices\.SortFunc\(\)`
slices.Grow(a, 2)
slices.Clip(a)
slices.Reverse(a)
slices.Sort(a)
slices.SortFunc(a, func(_, _ string) int {
return 0
})
slices.SortStableFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.SortStableFunc\(\) can be replaced by slices\.SortStableFunc\(\)`
slices.SortStableFunc(a, func(_, _ string) int {
return 0
})
slices.IsSorted(a) // want `golang.org/x/exp/slices\.IsSorted\(\) can be replaced by slices\.IsSorted\(\)`
slices.IsSortedFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.IsSortedFunc\(\) can be replaced by slices\.IsSortedFunc\(\)`
slices.IsSorted(a)
slices.IsSortedFunc(a, func(_, _ string) int {
return 0
})
slices.Min(a) // want `golang.org/x/exp/slices\.Min\(\) can be replaced by slices\.Min\(\)`
slices.MinFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.MinFunc\(\) can be replaced by slices\.MinFunc\(\)`
slices.Min(a)
slices.MinFunc(a, func(_, _ string) int {
return 0
})
slices.Max(a) // want `golang.org/x/exp/slices\.Max\(\) can be replaced by slices\.Max\(\)`
slices.MaxFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.MaxFunc\(\) can be replaced by slices\.MaxFunc\(\)`
slices.Max(a)
slices.MaxFunc(a, func(_, _ string) int {
return 0
})
slices.BinarySearch(a, "a") // want `golang.org/x/exp/slices\.BinarySearch\(\) can be replaced by slices\.BinarySearch\(\)`
slices.BinarySearchFunc(a, b, func(_ string, _ []string) int { // want `golang.org/x/exp/slices\.BinarySearchFunc\(\) can be replaced by slices\.BinarySearchFunc\(\)`
slices.BinarySearch(a, "a")
slices.BinarySearchFunc(a, b, func(_ string, _ []string) int {
return 0
})
}
59 changes: 29 additions & 30 deletions testdata/src/expslices/slices.go.golden
Original file line number Diff line number Diff line change
Expand Up @@ -5,58 +5,57 @@ import (
)

func _(a, b []string) {
slices.Equal(a, b) // want `golang.org/x/exp/slices\.Equal\(\) can be replaced by slices\.Equal\(\)`
slices.EqualFunc(a, b, func(_ string, _ string) bool { // want `golang.org/x/exp/slices\.EqualFunc\(\) can be replaced by slices\.EqualFunc\(\)`
slices.Equal(a, b)
slices.EqualFunc(a, b, func(_ string, _ string) bool {
return true
})
slices.Compare(a, b) // want `golang.org/x/exp/slices\.Compare\(\) can be replaced by slices\.Compare\(\)`
slices.CompareFunc(a, b, func(_ string, _ string) int { // want `golang.org/x/exp/slices\.CompareFunc\(\) can be replaced by slices\.CompareFunc\(\)`
slices.Compare(a, b)
slices.CompareFunc(a, b, func(_ string, _ string) int {
return 0
})
slices.Index(a, "a") // want `golang.org/x/exp/slices\.Index\(\) can be replaced by slices\.Index\(\)`
slices.IndexFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.IndexFunc\(\) can be replaced by slices\.IndexFunc\(\)`
slices.Index(a, "a")
slices.IndexFunc(a, func(_ string) bool {
return true
})
slices.Contains(a, "a") // want `golang.org/x/exp/slices\.Contains\(\) can be replaced by slices\.Contains\(\)`
slices.ContainsFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.ContainsFunc\(\) can be replaced by slices\.ContainsFunc\(\)`
slices.Contains(a, "a")
slices.ContainsFunc(a, func(_ string) bool {
return true
})
slices.Insert(a, 0, "a", "b") // want `golang.org/x/exp/slices\.Insert\(\) can be replaced by slices\.Insert\(\)`
slices.Delete(a, 0, 1) // want `golang.org/x/exp/slices\.Delete\(\) can be replaced by slices\.Delete\(\)`
slices.DeleteFunc(a, func(_ string) bool { // want `golang.org/x/exp/slices\.DeleteFunc\(\) can be replaced by slices\.DeleteFunc\(\)`
slices.Insert(a, 0, "a", "b")
slices.Delete(a, 0, 1)
slices.DeleteFunc(a, func(_ string) bool {
return true
})
slices.Replace(a, 0, 1, "a") // want `golang.org/x/exp/slices\.Replace\(\) can be replaced by slices\.Replace\(\)`
slices.Clone(a) // want `golang.org/x/exp/slices\.Clone\(\) can be replaced by slices\.Clone\(\)`
slices.Compact(a) // want `golang.org/x/exp/slices\.Compact\(\) can be replaced by slices\.Compact\(\)`
slices.CompactFunc(a, func(_ string, _ string) bool { // want `golang.org/x/exp/slices\.CompactFunc\(\) can be replaced by slices\.CompactFunc\(\)`
slices.Replace(a, 0, 1, "a")
slices.Clone(a)
slices.Compact(a)
slices.CompactFunc(a, func(_ string, _ string) bool {
return true
})
slices.Grow(a, 2) // want `golang.org/x/exp/slices\.Grow\(\) can be replaced by slices\.Grow\(\)`
slices.Clip(a) // want `golang.org/x/exp/slices\.Clip\(\) can be replaced by slices\.Clip\(\)`
slices.Reverse(a) // want `golang.org/x/exp/slices\.Reverse\(\) can be replaced by slices\.Reverse\(\)`
slices.Sort(a) // want `golang.org/x/exp/slices\.Sort\(\) can be replaced by slices\.Sort\(\)`
slices.SortFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.SortFunc\(\) can be replaced by slices\.SortFunc\(\)`
slices.Grow(a, 2)
slices.Clip(a)
slices.Reverse(a)
slices.Sort(a)
slices.SortFunc(a, func(_, _ string) int {
return 0
})
slices.SortStableFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.SortStableFunc\(\) can be replaced by slices\.SortStableFunc\(\)`
slices.SortStableFunc(a, func(_, _ string) int {
return 0
})
slices.IsSorted(a) // want `golang.org/x/exp/slices\.IsSorted\(\) can be replaced by slices\.IsSorted\(\)`
slices.IsSortedFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.IsSortedFunc\(\) can be replaced by slices\.IsSortedFunc\(\)`
slices.IsSorted(a)
slices.IsSortedFunc(a, func(_, _ string) int {
return 0
})
slices.Min(a) // want `golang.org/x/exp/slices\.Min\(\) can be replaced by slices\.Min\(\)`
slices.MinFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.MinFunc\(\) can be replaced by slices\.MinFunc\(\)`
slices.Min(a)
slices.MinFunc(a, func(_, _ string) int {
return 0
})
slices.Max(a) // want `golang.org/x/exp/slices\.Max\(\) can be replaced by slices\.Max\(\)`
slices.MaxFunc(a, func(_, _ string) int { // want `golang.org/x/exp/slices\.MaxFunc\(\) can be replaced by slices\.MaxFunc\(\)`
slices.Max(a)
slices.MaxFunc(a, func(_, _ string) int {
return 0
})
slices.BinarySearch(a, "a") // want `golang.org/x/exp/slices\.BinarySearch\(\) can be replaced by slices\.BinarySearch\(\)`
slices.BinarySearchFunc(a, b, func(_ string, _ []string) int { // want `golang.org/x/exp/slices\.BinarySearchFunc\(\) can be replaced by slices\.BinarySearchFunc\(\)`
slices.BinarySearch(a, "a")
slices.BinarySearchFunc(a, b, func(_ string, _ []string) int {
return 0
})
}

0 comments on commit 6315bee

Please sign in to comment.