Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove use of sort functions that take sort.Interface #39937

Merged
merged 10 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions .ci/semgrep/stdlib/sort.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
rules:
- id: prefer-slices-sortfunc
languages: [go]
message: Prefer slices.SortFunc to sort.Slice
pattern: sort.Slice(...)
message: Prefer slices.SortFunc to sort.$FUNC
patterns:
- pattern: sort.$FUNC(...)
- metavariable-pattern:
metavariable: $FUNC
pattern-either:
- pattern: Sort
- pattern: Slice
severity: WARNING

- id: prefer-slices-sortstablefunc
Expand Down Expand Up @@ -38,3 +44,9 @@ rules:
- pattern: StringsAreSorted
fix: slices.IsSorted($X)
severity: WARNING

- id: prefer-slices-issortedfunc
languages: [go]
message: Prefer slices.IsSortedFunc to sort.IsSorted
pattern: sort.IsSorted($X)
severity: WARNING
18 changes: 5 additions & 13 deletions internal/service/dax/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
package dax

import (
"cmp"
"context"
"fmt"
"log"
"reflect"
"sort"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -472,9 +473,9 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, meta int
}

func setClusterNodeData(d *schema.ResourceData, c awstypes.Cluster) error {
sortedNodes := make([]awstypes.Node, len(c.Nodes))
copy(sortedNodes, c.Nodes)
sort.Sort(byNodeId(sortedNodes))
sortedNodes := slices.SortedFunc(slices.Values(c.Nodes), func(a, b awstypes.Node) int {
return cmp.Compare(aws.ToString(a.NodeId), aws.ToString(b.NodeId))
})

nodeData := make([]map[string]interface{}, 0, len(sortedNodes))

Expand All @@ -490,15 +491,6 @@ func setClusterNodeData(d *schema.ResourceData, c awstypes.Cluster) error {
return d.Set("nodes", nodeData)
}

type byNodeId []awstypes.Node

func (b byNodeId) Len() int { return len(b) }
func (b byNodeId) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
func (b byNodeId) Less(i, j int) bool {
return b[i].NodeId != nil && b[j].NodeId != nil &&
aws.ToString(b[i].NodeId) < aws.ToString(b[j].NodeId)
}

func resourceClusterDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
var diags diag.Diagnostics
conn := meta.(*conns.AWSClient).DAXClient(ctx)
Expand Down
18 changes: 4 additions & 14 deletions internal/service/ec2/ebs_volume_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package ec2
import (
"context"
"fmt"
"sort"
"slices"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
Expand Down Expand Up @@ -159,18 +159,8 @@ func dataSourceEBSVolumeRead(ctx context.Context, d *schema.ResourceData, meta i
return diags
}

type volumeSort []awstypes.Volume

func (a volumeSort) Len() int { return len(a) }
func (a volumeSort) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
func (a volumeSort) Less(i, j int) bool {
itime := aws.ToTime(a[i].CreateTime)
jtime := aws.ToTime(a[j].CreateTime)
return itime.Unix() < jtime.Unix()
}

func mostRecentVolume(volumes []awstypes.Volume) awstypes.Volume {
sortedVolumes := volumes
sort.Sort(volumeSort(sortedVolumes))
return sortedVolumes[len(sortedVolumes)-1]
return slices.MaxFunc(volumes, func(a, b awstypes.Volume) int {
return a.CreateTime.Compare(aws.ToTime(b.CreateTime))
})
}
54 changes: 28 additions & 26 deletions internal/service/ec2/vpc_security_group_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ package ec2

import (
"bytes"
"cmp"
"context"
"errors"
"fmt"
"log"
"slices"
"sort"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -173,7 +174,10 @@ func resourceSecurityGroupRuleCreate(ctx context.Context, d *schema.ResourceData

ipPermission := expandIPPermission(d, sg)
ruleType := securityGroupRuleType(d.Get(names.AttrType).(string))
id := securityGroupRuleCreateID(securityGroupID, string(ruleType), &ipPermission)
id, err := securityGroupRuleCreateID(securityGroupID, string(ruleType), &ipPermission)
if err != nil {
return sdkdiag.AppendErrorf(diags, "reading Security Group (%s): %s", securityGroupID, err)
}

switch ruleType {
case securityGroupRuleTypeIngress:
Expand Down Expand Up @@ -307,7 +311,10 @@ func resourceSecurityGroupRuleRead(ctx context.Context, d *schema.ResourceData,

if strings.Contains(d.Id(), securityGroupRuleIDSeparator) {
// import so fix the id
id := securityGroupRuleCreateID(securityGroupID, string(ruleType), &ipPermission)
id, err := securityGroupRuleCreateID(securityGroupID, string(ruleType), &ipPermission)
if err != nil {
return sdkdiag.AppendErrorf(diags, "reading Security Group (%s) Rule (%s): %s", securityGroupID, d.Id(), err)
}
d.SetId(id)
}

Expand Down Expand Up @@ -674,25 +681,7 @@ func findSecurityGroupRuleMatch(p awstypes.IpPermission, securityGroupRules []aw

const securityGroupRuleIDSeparator = "_"

// byGroupPair implements sort.Interface for []*ec2.UserIDGroupPairs based on
// GroupID or GroupName field (only one should be set).
type byGroupPair []awstypes.UserIdGroupPair

func (b byGroupPair) Len() int { return len(b) }
func (b byGroupPair) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
func (b byGroupPair) Less(i, j int) bool {
if b[i].GroupId != nil && b[j].GroupId != nil {
return aws.ToString(b[i].GroupId) < aws.ToString(b[j].GroupId)
}
if b[i].GroupName != nil && b[j].GroupName != nil {
return aws.ToString(b[i].GroupName) < aws.ToString(b[j].GroupName)
}

//lintignore:R009
panic("mismatched security group rules, may be a terraform bug")
}

func securityGroupRuleCreateID(securityGroupID, ruleType string, ip *awstypes.IpPermission) string {
func securityGroupRuleCreateID(securityGroupID, ruleType string, ip *awstypes.IpPermission) (string, error) {
var buf bytes.Buffer

buf.WriteString(fmt.Sprintf("%s-", securityGroupID))
Expand Down Expand Up @@ -744,22 +733,35 @@ func securityGroupRuleCreateID(securityGroupID, ruleType string, ip *awstypes.Ip
}

if len(ip.UserIdGroupPairs) > 0 {
sort.Sort(byGroupPair(ip.UserIdGroupPairs))
var err error
slices.SortFunc(ip.UserIdGroupPairs, func(a, b awstypes.UserIdGroupPair) int {
if a.GroupId != nil && b.GroupId != nil {
return cmp.Compare(aws.ToString(a.GroupId), aws.ToString(b.GroupId))
}
if a.GroupName != nil && b.GroupName != nil {
return cmp.Compare(aws.ToString(a.GroupName), aws.ToString(b.GroupName))
}
err = errors.New("mismatched security group rules: contains both GroupId and GroupName")
return 0
})
if err != nil {
return "", err
}
for _, pair := range ip.UserIdGroupPairs {
if pair.GroupId != nil {
buf.WriteString(fmt.Sprintf("%s-", *pair.GroupId))
buf.WriteString(fmt.Sprintf("%s-", aws.ToString(pair.GroupId)))
} else {
buf.WriteString("-")
}
if pair.GroupName != nil {
buf.WriteString(fmt.Sprintf("%s-", *pair.GroupName))
buf.WriteString(fmt.Sprintf("%s-", aws.ToString(pair.GroupName)))
} else {
buf.WriteString("-")
}
}
}

return fmt.Sprintf("sgrule-%d", create.StringHashcode(buf.String()))
return fmt.Sprintf("sgrule-%d", create.StringHashcode(buf.String())), nil
}

func expandIPPermission(d *schema.ResourceData, sg *awstypes.SecurityGroup) awstypes.IpPermission { // nosemgrep:ci.caps5-in-func-name
Expand Down
6 changes: 4 additions & 2 deletions internal/service/ec2/vpc_security_group_rule_migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ func migrateSGRuleStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceS
}

perm, err := migrateExpandIPPerm(is.Attributes)

if err != nil {
return nil, fmt.Errorf("making new IP Permission in Security Group migration")
}

log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes)
newID := securityGroupRuleCreateID(is.Attributes["security_group_id"], is.Attributes[names.AttrType], perm)
newID, err := securityGroupRuleCreateID(is.Attributes["security_group_id"], is.Attributes[names.AttrType], perm)
if err != nil {
return nil, err
}
is.Attributes[names.AttrID] = newID
is.ID = newID
log.Printf("[DEBUG] Attributes after migration: %#v, new id: %s", is.Attributes, newID)
Expand Down
5 changes: 4 additions & 1 deletion internal/service/ec2/vpc_security_group_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ func TestSecurityGroupRuleCreateID(t *testing.T) {
}

for _, tc := range cases {
actual := tfec2.SecurityGroupRuleCreateID("sg-12345", tc.Type, &tc.Input)
actual, err := tfec2.SecurityGroupRuleCreateID("sg-12345", tc.Type, &tc.Input)
if err != nil {
t.Fatalf("unexpected error: %s", err.Error())
}
if actual != tc.Output {
t.Errorf("input: %s - %#v\noutput: %s", tc.Type, tc.Input, actual)
}
Expand Down
19 changes: 5 additions & 14 deletions internal/service/ec2/vpnsite_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
package ec2

import (
"cmp"
"context"
"encoding/xml"
"fmt"
"log"
"net"
"sort"
"slices"
"strconv"
"time"

Expand Down Expand Up @@ -1565,18 +1566,6 @@ type tunnelInfo struct {
Tunnel2VgwInsideAddress string
}

func (slice xmlVpnConnectionConfig) Len() int {
return len(slice.Tunnels)
}

func (slice xmlVpnConnectionConfig) Less(i, j int) bool {
return slice.Tunnels[i].OutsideAddress < slice.Tunnels[j].OutsideAddress
}

func (slice xmlVpnConnectionConfig) Swap(i, j int) {
slice.Tunnels[i], slice.Tunnels[j] = slice.Tunnels[j], slice.Tunnels[i]
}

// customerGatewayConfigurationToTunnelInfo converts the configuration information for the
// VPN connection's customer gateway (in the native XML format) to a tunnelInfo structure.
// The tunnel1 parameters are optionally used to correctly order tunnel configurations.
Expand Down Expand Up @@ -1616,7 +1605,9 @@ func customerGatewayConfigurationToTunnelInfo(xmlConfig string, tunnel1PreShared
}
}
} else {
sort.Sort(vpnConfig)
slices.SortFunc(vpnConfig.Tunnels, func(a, b xmlIpsecTunnel) int {
return cmp.Compare(a.OutsideAddress, b.OutsideAddress)
})
}

tunnelInfo := &tunnelInfo{
Expand Down
18 changes: 5 additions & 13 deletions internal/service/elasticache/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
package elasticache

import (
"cmp"
"context"
"errors"
"fmt"
"log"
"sort"
"slices"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -944,9 +945,9 @@ func getCacheNodesToRemove(oldNumberOfNodes int, cacheNodesToRemove int) []strin
}

func setCacheNodeData(d *schema.ResourceData, c *awstypes.CacheCluster) error {
sortedCacheNodes := make([]awstypes.CacheNode, len(c.CacheNodes))
copy(sortedCacheNodes, c.CacheNodes)
sort.Sort(byCacheNodeId(sortedCacheNodes))
sortedCacheNodes := slices.SortedFunc(slices.Values(c.CacheNodes), func(a, b awstypes.CacheNode) int {
return cmp.Compare(aws.ToString(a.CacheNodeId), aws.ToString(b.CacheNodeId))
})

cacheNodeData := make([]map[string]interface{}, 0, len(sortedCacheNodes))

Expand All @@ -966,15 +967,6 @@ func setCacheNodeData(d *schema.ResourceData, c *awstypes.CacheCluster) error {
return d.Set("cache_nodes", cacheNodeData)
}

type byCacheNodeId []awstypes.CacheNode

func (b byCacheNodeId) Len() int { return len(b) }
func (b byCacheNodeId) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
func (b byCacheNodeId) Less(i, j int) bool {
return b[i].CacheNodeId != nil && b[j].CacheNodeId != nil &&
aws.ToString(b[i].CacheNodeId) < aws.ToString(b[j].CacheNodeId)
}

func setFromCacheCluster(d *schema.ResourceData, c *awstypes.CacheCluster) error {
d.Set("node_type", c.CacheNodeType)

Expand Down
9 changes: 5 additions & 4 deletions internal/service/iam/policy_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"encoding/json"
"fmt"
"slices"
"sort"
"strconv"

"github.com/YakDriver/regexache"
Expand Down Expand Up @@ -86,7 +85,7 @@ func (s *IAMPolicyDoc) Merge(newDoc *IAMPolicyDoc) {
func (ps IAMPolicyStatementPrincipalSet) MarshalJSON() ([]byte, error) {
raw := map[string]interface{}{}

// Although IAM documentation says, that "*" and {"AWS": "*"} are equivalent
// Although IAM documentation says that "*" and {"AWS": "*"} are equivalent
// (https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_principal.html),
// in practice they are not for IAM roles. IAM will return an error if trust
// policy have "*" or {"*": "*"} as principal, but will accept {"AWS": "*"}.
Expand Down Expand Up @@ -115,7 +114,8 @@ func (ps IAMPolicyStatementPrincipalSet) MarshalJSON() ([]byte, error) {
raw[p.Type] = make([]string, 0, len(i)+1)
raw[p.Type] = append(raw[p.Type].([]string), v)
}
sort.Sort(sort.Reverse(sort.StringSlice(i)))
slices.Sort(i)
slices.Reverse(i)
raw[p.Type] = append(raw[p.Type].([]string), i...)
case string:
switch v := raw[p.Type].(type) {
Expand Down Expand Up @@ -243,7 +243,8 @@ func policyDecodeConfigStringList(lI []interface{}) interface{} {
for i, vI := range lI {
ret[i] = vI.(string)
}
sort.Sort(sort.Reverse(sort.StringSlice(ret)))
slices.Sort(ret)
slices.Reverse(ret)
return ret
}

Expand Down
Loading
Loading