Skip to content

Commit 84007fb

Browse files
tadeleshtadelesh
tadelesh
and
tadelesh
authored
Take param name into account for func and interface diff detection (#25604)
* take param name into account for func and interface diff detection * fix with review --------- Co-authored-by: tadelesh <[email protected]>
1 parent 3ad86d1 commit 84007fb

File tree

5 files changed

+204
-25
lines changed

5 files changed

+204
-25
lines changed

‎eng/tools/internal/delta/delta.go‎

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package delta
55

66
import (
77
"sort"
8+
"strings"
89

910
"github.com/Azure/azure-sdk-for-go/eng/tools/internal/exports"
1011
)
@@ -243,15 +244,19 @@ func GetFuncSigChanges(lhs, rhs exports.Content) map[string]FuncSig {
243244
continue
244245
}
245246
sig := FuncSig{}
246-
if !safeStrCmp(lhs.Funcs[rhsKey].Params, rhsValue.Params) {
247+
lhsValue := lhs.Funcs[rhsKey]
248+
249+
// Check for parameter changes
250+
if !paramsEqual(lhsValue.Params, rhsValue.Params) {
247251
sig.Params = &Signature{
248-
From: safeFuncSig(lhs.Funcs[rhsKey].Params),
249-
To: safeFuncSig(rhsValue.Params),
252+
From: paramsToString(lhsValue.Params),
253+
To: paramsToString(rhsValue.Params),
250254
}
251255
}
252-
if !safeStrCmp(lhs.Funcs[rhsKey].Returns, rhsValue.Returns) {
256+
257+
if !safeStrCmp(lhsValue.Returns, rhsValue.Returns) {
253258
sig.Returns = &Signature{
254-
From: safeFuncSig(lhs.Funcs[rhsKey].Returns),
259+
From: safeFuncSig(lhsValue.Returns),
255260
To: safeFuncSig(rhsValue.Returns),
256261
}
257262
}
@@ -285,10 +290,10 @@ func GetInterfaceMethodSigChanges(lhs, rhs exports.Content) map[string]Interface
285290
continue
286291
}
287292
sig := FuncSig{}
288-
if !safeStrCmp(lhs.Interfaces[rhsKey].Methods[rhsMethod].Params, rhsSig.Params) {
293+
if !paramsEqual(lhs.Interfaces[rhsKey].Methods[rhsMethod].Params, rhsSig.Params) {
289294
sig.Params = &Signature{
290-
From: safeFuncSig(lhs.Interfaces[rhsKey].Methods[rhsMethod].Params),
291-
To: safeFuncSig(rhsSig.Params),
295+
From: paramsToString(lhs.Interfaces[rhsKey].Methods[rhsMethod].Params),
296+
To: paramsToString(rhsSig.Params),
292297
}
293298
}
294299
if !safeStrCmp(lhs.Interfaces[rhsKey].Methods[rhsMethod].Returns, rhsSig.Returns) {
@@ -368,3 +373,42 @@ func safeStrCmp(lhs, rhs *string) bool {
368373
}
369374
return *lhs == *rhs
370375
}
376+
377+
// paramsEqual checks if two parameter lists are equal.
378+
// Parameters are considered equal only if they have the same types in the same order
379+
// with the same names in the same order.
380+
func paramsEqual(lhs, rhs []exports.Param) bool {
381+
if len(lhs) != len(rhs) {
382+
return false
383+
}
384+
385+
for i := range lhs {
386+
// Types must match exactly
387+
if lhs[i].Type != rhs[i].Type {
388+
return false
389+
}
390+
// Names must match exactly
391+
if lhs[i].Name != rhs[i].Name {
392+
return false
393+
}
394+
}
395+
396+
return true
397+
}
398+
399+
// paramsToString converts a parameter list to a comma-delimited string with names and types.
400+
func paramsToString(params []exports.Param) string {
401+
if len(params) == 0 {
402+
return None
403+
}
404+
405+
var parts []string
406+
for _, p := range params {
407+
if p.Name != "" {
408+
parts = append(parts, p.Name+" "+p.Type)
409+
} else {
410+
parts = append(parts, p.Type)
411+
}
412+
}
413+
return strings.Join(parts, ", ")
414+
}

‎eng/tools/internal/delta/delta_test.go‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
package delta
5+
6+
import (
7+
"testing"
8+
9+
"github.com/Azure/azure-sdk-for-go/eng/tools/internal/exports"
10+
)
11+
12+
func TestParamsEqual(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
lhs []exports.Param
16+
rhs []exports.Param
17+
expected bool
18+
}{
19+
{
20+
name: "Simple order change",
21+
lhs: []exports.Param{
22+
{Name: "resourceGroupName", Type: "string"},
23+
{Name: "serviceName", Type: "string"},
24+
{Name: "options", Type: "*Options"},
25+
},
26+
rhs: []exports.Param{
27+
{Name: "serviceName", Type: "string"},
28+
{Name: "resourceGroupName", Type: "string"},
29+
{Name: "options", Type: "*Options"},
30+
},
31+
expected: false,
32+
},
33+
{
34+
name: "Three params order change",
35+
lhs: []exports.Param{
36+
{Name: "resourceGroupName", Type: "string"},
37+
{Name: "serviceName", Type: "string"},
38+
{Name: "subscriptionID", Type: "string"},
39+
},
40+
rhs: []exports.Param{
41+
{Name: "serviceName", Type: "string"},
42+
{Name: "subscriptionID", Type: "string"},
43+
{Name: "resourceGroupName", Type: "string"},
44+
},
45+
expected: false,
46+
},
47+
{
48+
name: "No change",
49+
lhs: []exports.Param{
50+
{Name: "ctx", Type: "context.Context"},
51+
{Name: "name", Type: "string"},
52+
{Name: "value", Type: "int"},
53+
},
54+
rhs: []exports.Param{
55+
{Name: "ctx", Type: "context.Context"},
56+
{Name: "name", Type: "string"},
57+
{Name: "value", Type: "int"},
58+
},
59+
expected: true,
60+
},
61+
{
62+
name: "Name change only",
63+
lhs: []exports.Param{
64+
{Name: "oldName", Type: "string"},
65+
{Name: "newName", Type: "string"},
66+
},
67+
rhs: []exports.Param{
68+
{Name: "firstName", Type: "string"},
69+
{Name: "lastName", Type: "string"},
70+
},
71+
expected: false,
72+
},
73+
{
74+
name: "Type change",
75+
lhs: []exports.Param{
76+
{Name: "value", Type: "int"},
77+
},
78+
rhs: []exports.Param{
79+
{Name: "value", Type: "string"},
80+
},
81+
expected: false,
82+
},
83+
{
84+
name: "Different number of params",
85+
lhs: []exports.Param{
86+
{Name: "a", Type: "string"},
87+
{Name: "b", Type: "string"},
88+
},
89+
rhs: []exports.Param{
90+
{Name: "a", Type: "string"},
91+
},
92+
expected: false,
93+
},
94+
}
95+
96+
for _, tt := range tests {
97+
t.Run(tt.name, func(t *testing.T) {
98+
result := paramsEqual(tt.lhs, tt.rhs)
99+
if result != tt.expected {
100+
t.Errorf("paramsEqual() = %v, expected %v", result, tt.expected)
101+
t.Logf("lhs: %+v", tt.lhs)
102+
t.Logf("rhs: %+v", tt.rhs)
103+
}
104+
})
105+
}
106+
}

‎eng/tools/internal/exports/exports.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,19 @@ type Const struct {
4747
Value string `json:"value"`
4848
}
4949

50+
// Param represents a function parameter with its name and type.
51+
type Param struct {
52+
// Name is the parameter name, may be empty for unnamed parameters
53+
Name string `json:"name,omitempty"`
54+
55+
// Type is the parameter type
56+
Type string `json:"type"`
57+
}
58+
5059
// Func contains parameter and return types of a function/method.
5160
type Func struct {
52-
// a comma-delimited list of the param types
53-
Params *string `json:"params,omitempty"`
61+
// Params is the list of function parameters
62+
Params []Param `json:"params,omitempty"`
5463

5564
// a comma-delimited list of the return types
5665
Returns *string `json:"returns,omitempty"`

‎eng/tools/internal/exports/package.go‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,17 @@ func (pkg Package) getText(start token.Pos, end token.Pos) string {
157157
// name can be nil, e.g. anonymous fields in structs, unnamed return types etc.
158158
func (pkg Package) translateFieldList(fl []*ast.Field, cb func(*string, string, *ast.Field)) {
159159
for _, f := range fl {
160-
var name *string
160+
t := pkg.getText(f.Type.Pos(), f.Type.End())
161161
if f.Names != nil {
162-
n := pkg.getText(f.Names[0].Pos(), f.Names[0].End())
163-
name = &n
162+
// Handle multiple parameter names with the same type (e.g., func Foo(a, b string))
163+
for _, name := range f.Names {
164+
n := pkg.getText(name.Pos(), name.End())
165+
cb(&n, t, f)
166+
}
167+
} else {
168+
// Unnamed parameter
169+
cb(nil, t, f)
164170
}
165-
t := pkg.getText(f.Type.Pos(), f.Type.End())
166-
cb(name, t, f)
167171
}
168172
}
169173

@@ -178,19 +182,21 @@ func (pkg Package) buildFunc(ft *ast.FuncType) (f Func) {
178182
return s
179183
}
180184

181-
// build the params type list
185+
// build the params list
182186
if ft.Params.List != nil {
183-
p := ""
184-
pkg.translateFieldList(ft.Params.List, func(n *string, t string, f *ast.Field) {
185-
p = appendString(p, t)
187+
pkg.translateFieldList(ft.Params.List, func(n *string, t string, field *ast.Field) {
188+
param := Param{Type: t}
189+
if n != nil {
190+
param.Name = *n
191+
}
192+
f.Params = append(f.Params, param)
186193
})
187-
f.Params = &p
188194
}
189195

190196
// build the return types list
191197
if ft.Results != nil {
192198
r := ""
193-
pkg.translateFieldList(ft.Results.List, func(n *string, t string, f *ast.Field) {
199+
pkg.translateFieldList(ft.Results.List, func(n *string, t string, field *ast.Field) {
194200
r = appendString(r, t)
195201
})
196202
f.Returns = &r

‎eng/tools/internal/report/report.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,7 @@ func writeFuncs(funcs map[string]exports.Func, subheader string, md *markdown.Wr
297297
items := make([]string, len(funcs))
298298
i := 0
299299
for k, v := range funcs {
300-
params := ""
301-
if v.Params != nil {
302-
params = *v.Params
303-
}
300+
params := formatParams(v.Params)
304301
returns := ""
305302
if v.Returns != nil {
306303
returns = *v.Returns
@@ -318,6 +315,23 @@ func writeFuncs(funcs map[string]exports.Func, subheader string, md *markdown.Wr
318315
}
319316
}
320317

318+
// formatParams converts a parameter list to a comma-delimited string with names and types
319+
func formatParams(params []exports.Param) string {
320+
if len(params) == 0 {
321+
return ""
322+
}
323+
324+
var parts []string
325+
for _, p := range params {
326+
if p.Name != "" {
327+
parts = append(parts, p.Name+" "+p.Type)
328+
} else {
329+
parts = append(parts, p.Type)
330+
}
331+
}
332+
return strings.Join(parts, ", ")
333+
}
334+
321335
// writes out struct information
322336
// sheader1 is for added/removed struct types formatted as TypeName
323337
// sheader2 is for added/removed struct fields formatted as TypeName.FieldName

0 commit comments

Comments
 (0)