Skip to content

Commit b4332e0

Browse files
h9jianggopherbot
h9jiang
authored and
gopherbot
committed
gopls/internal/golang, go/ssa: remove unnamed input parameter
- Unnamed/blank parameters are now assigned zero values, as they are not referenced within the function body. - The `zeroString` function has been moved from `go/ssa` to `internal/typesinternal` for better organization. - Honor the input parameter name from the function signature. - Input parameters from both consutrctor and target functions are flattened into the test case struct. Potential field name duplication is handled by introducing prefixes. For golang/vscode-go#1594 Change-Id: I8b56d8f3e0f0432d4f9fe269cc7ba86ea46decfc Reviewed-on: https://go-review.googlesource.com/c/tools/+/626537 Reviewed-by: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Auto-Submit: Hongxiang Jiang <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent a8d0fa5 commit b4332e0

File tree

9 files changed

+573
-244
lines changed

9 files changed

+573
-244
lines changed

‎go/ssa/const.go

Lines changed: 8 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ import (
1212
"go/token"
1313
"go/types"
1414
"strconv"
15-
"strings"
1615

1716
"golang.org/x/tools/internal/typeparams"
17+
"golang.org/x/tools/internal/typesinternal"
1818
)
1919

2020
// NewConst returns a new constant of the specified value and type.
@@ -78,7 +78,13 @@ func zeroConst(t types.Type) *Const {
7878
func (c *Const) RelString(from *types.Package) string {
7979
var s string
8080
if c.Value == nil {
81-
s = zeroString(c.typ, from)
81+
if _, ok := c.typ.(*types.TypeParam); ok {
82+
// Type parameter's underlying type may be interface that is
83+
// nillable. A better zero value of type parameter is *new(T).
84+
s = typesinternal.ZeroString(c.typ, types.RelativeTo(from))
85+
} else {
86+
s = typesinternal.ZeroString(c.typ.Underlying(), types.RelativeTo(from))
87+
}
8288
} else if c.Value.Kind() == constant.String {
8389
s = constant.StringVal(c.Value)
8490
const max = 20
@@ -93,44 +99,6 @@ func (c *Const) RelString(from *types.Package) string {
9399
return s + ":" + relType(c.Type(), from)
94100
}
95101

96-
// zeroString returns the string representation of the "zero" value of the type t.
97-
func zeroString(t types.Type, from *types.Package) string {
98-
switch t := t.(type) {
99-
case *types.Basic:
100-
switch {
101-
case t.Info()&types.IsBoolean != 0:
102-
return "false"
103-
case t.Info()&types.IsNumeric != 0:
104-
return "0"
105-
case t.Info()&types.IsString != 0:
106-
return `""`
107-
case t.Kind() == types.UnsafePointer:
108-
fallthrough
109-
case t.Kind() == types.UntypedNil:
110-
return "nil"
111-
default:
112-
panic(fmt.Sprint("zeroString for unexpected type:", t))
113-
}
114-
case *types.Pointer, *types.Slice, *types.Interface, *types.Chan, *types.Map, *types.Signature:
115-
return "nil"
116-
case *types.Named, *types.Alias:
117-
return zeroString(t.Underlying(), from)
118-
case *types.Array, *types.Struct:
119-
return relType(t, from) + "{}"
120-
case *types.Tuple:
121-
// Tuples are not normal values.
122-
// We are currently format as "(t[0], ..., t[n])". Could be something else.
123-
components := make([]string, t.Len())
124-
for i := 0; i < t.Len(); i++ {
125-
components[i] = zeroString(t.At(i).Type(), from)
126-
}
127-
return "(" + strings.Join(components, ", ") + ")"
128-
case *types.TypeParam:
129-
return "*new(" + relType(t, from) + ")"
130-
}
131-
panic(fmt.Sprint("zeroString: unexpected ", t))
132-
}
133-
134102
func (c *Const) Name() string {
135103
return c.RelString(nil)
136104
}

‎gopls/internal/golang/addtest.go

Lines changed: 102 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ import (
1414
"go/ast"
1515
"go/token"
1616
"go/types"
17-
"html/template"
1817
"os"
1918
"path/filepath"
2019
"sort"
2120
"strconv"
2221
"strings"
22+
"text/template"
2323
"unicode"
2424

2525
"golang.org/x/tools/go/ast/astutil"
@@ -34,44 +34,34 @@ import (
3434

3535
const testTmplString = `
3636
func {{.TestFuncName}}(t *{{.TestingPackageName}}.T) {
37-
{{- /* Constructor input parameters struct declaration. */}}
38-
{{- if and .Receiver .Receiver.Constructor}}
39-
{{- if gt (len .Receiver.Constructor.Args) 1}}
40-
type constructorArgs struct {
41-
{{- range .Receiver.Constructor.Args}}
42-
{{.Name}} {{.Type}}
43-
{{- end}}
44-
}
45-
{{- end}}
46-
{{- end}}
47-
48-
{{- /* Functions/methods input parameters struct declaration. */}}
49-
{{- if gt (len .Func.Args) 1}}
50-
type args struct {
51-
{{- range .Func.Args}}
52-
{{.Name}} {{.Type}}
53-
{{- end}}
54-
}
55-
{{- end}}
56-
5737
{{- /* Test cases struct declaration and empty initialization. */}}
5838
tests := []struct {
5939
name string // description of this test case
40+
41+
{{- $commentPrinted := false }}
6042
{{- if and .Receiver .Receiver.Constructor}}
61-
{{- if gt (len .Receiver.Constructor.Args) 1}}
62-
constructorArgs constructorArgs
43+
{{- range .Receiver.Constructor.Args}}
44+
{{- if .Name}}
45+
{{- if not $commentPrinted}}
46+
// Named input parameters for receiver constructor.
47+
{{- $commentPrinted = true }}
48+
{{- end}}
49+
{{.Name}} {{.Type}}
6350
{{- end}}
64-
{{- if eq (len .Receiver.Constructor.Args) 1}}
65-
constructorArg {{(index .Receiver.Constructor.Args 0).Type}}
6651
{{- end}}
6752
{{- end}}
6853
69-
{{- if gt (len .Func.Args) 1}}
70-
args args
54+
{{- $commentPrinted := false }}
55+
{{- range .Func.Args}}
56+
{{- if .Name}}
57+
{{- if not $commentPrinted}}
58+
// Named input parameters for target function.
59+
{{- $commentPrinted = true }}
60+
{{- end}}
61+
{{.Name}} {{.Type}}
7162
{{- end}}
72-
{{- if eq (len .Func.Args) 1}}
73-
arg {{(index .Func.Args 0).Type}}
7463
{{- end}}
64+
7565
{{- range $index, $res := .Func.Results}}
7666
{{- if eq $res.Name "gotErr"}}
7767
wantErr bool
@@ -96,7 +86,12 @@ func {{.TestFuncName}}(t *{{.TestingPackageName}}.T) {
9686
{{- .Receiver.Constructor.Name}}
9787
9888
{{- /* Constructor input parameters. */ -}}
99-
({{- if eq (len .Receiver.Constructor.Args) 1}}tt.constructorArg{{end}}{{if gt (len .Func.Args) 1}}{{fieldNames .Receiver.Constructor.Args "tt.constructorArgs."}}{{end}})
89+
(
90+
{{- range $index, $arg := .Receiver.Constructor.Args}}
91+
{{- if ne $index 0}}, {{end}}
92+
{{- if .Name}}tt.{{.Name}}{{else}}{{.Value}}{{end}}
93+
{{- end -}}
94+
)
10095
10196
{{- /* Handles the error return from constructor. */}}
10297
{{- $last := last .Receiver.Constructor.Results}}
@@ -123,7 +118,12 @@ func {{.TestFuncName}}(t *{{.TestingPackageName}}.T) {
123118
{{- end}}{{.Func.Name}}
124119
125120
{{- /* Input parameters. */ -}}
126-
({{- if eq (len .Func.Args) 1}}tt.arg{{end}}{{if gt (len .Func.Args) 1}}{{fieldNames .Func.Args "tt.args."}}{{end}})
121+
(
122+
{{- range $index, $arg := .Func.Args}}
123+
{{- if ne $index 0}}, {{end}}
124+
{{- if .Name}}tt.{{.Name}}{{else}}{{.Value}}{{end}}
125+
{{- end -}}
126+
)
127127
128128
{{- /* Handles the returned error before the rest of return value. */}}
129129
{{- $last := last .Func.Results}}
@@ -155,8 +155,12 @@ func {{.TestFuncName}}(t *{{.TestingPackageName}}.T) {
155155
}
156156
`
157157

158+
// Name is the name of the field this input parameter should reference.
159+
// Value is the expression this input parameter should accept.
160+
//
161+
// Exactly one of Name or Value must be set.
158162
type field struct {
159-
Name, Type string
163+
Name, Type, Value string
160164
}
161165

162166
type function struct {
@@ -191,6 +195,9 @@ type testInfo struct {
191195
var testTmpl = template.Must(template.New("test").Funcs(template.FuncMap{
192196
"add": func(a, b int) int { return a + b },
193197
"last": func(slice []field) field {
198+
if len(slice) == 0 {
199+
return field{}
200+
}
194201
return slice[len(slice)-1]
195202
},
196203
"fieldNames": func(fields []field, qualifier string) (res string) {
@@ -450,36 +457,32 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
450457

451458
errorType := types.Universe.Lookup("error").Type()
452459

453-
// TODO(hxjiang): if input parameter is not named (meaning it's not used),
454-
// pass the zero value to the function call.
455-
// TODO(hxjiang): if the input parameter is named, define the field by using
456-
// the parameter's name instead of in%d.
457460
// TODO(hxjiang): handle special case for ctx.Context input.
458-
for index := range sig.Params().Len() {
459-
var name string
460-
if index == 0 {
461-
name = "in"
461+
for i := range sig.Params().Len() {
462+
param := sig.Params().At(i)
463+
name, typ := param.Name(), param.Type()
464+
f := field{Type: types.TypeString(typ, qf)}
465+
if name == "" || name == "_" {
466+
f.Value = typesinternal.ZeroString(typ, qf)
462467
} else {
463-
name = fmt.Sprintf("in%d", index+1)
468+
f.Name = name
464469
}
465-
data.Func.Args = append(data.Func.Args, field{
466-
Name: name,
467-
Type: types.TypeString(sig.Params().At(index).Type(), qf),
468-
})
470+
data.Func.Args = append(data.Func.Args, f)
469471
}
470472

471-
for index := range sig.Results().Len() {
473+
for i := range sig.Results().Len() {
474+
typ := sig.Results().At(i).Type()
472475
var name string
473-
if index == sig.Results().Len()-1 && types.Identical(sig.Results().At(index).Type(), errorType) {
476+
if i == sig.Results().Len()-1 && types.Identical(typ, errorType) {
474477
name = "gotErr"
475-
} else if index == 0 {
478+
} else if i == 0 {
476479
name = "got"
477480
} else {
478-
name = fmt.Sprintf("got%d", index+1)
481+
name = fmt.Sprintf("got%d", i+1)
479482
}
480483
data.Func.Results = append(data.Func.Results, field{
481484
Name: name,
482-
Type: types.TypeString(sig.Results().At(index).Type(), qf),
485+
Type: types.TypeString(typ, qf),
483486
})
484487
}
485488

@@ -587,25 +590,25 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
587590

588591
if constructor != nil {
589592
data.Receiver.Constructor = &function{Name: constructor.Name()}
590-
for index := range constructor.Signature().Params().Len() {
591-
var name string
592-
if index == 0 {
593-
name = "in"
593+
for i := range constructor.Signature().Params().Len() {
594+
param := constructor.Signature().Params().At(i)
595+
name, typ := param.Name(), param.Type()
596+
f := field{Type: types.TypeString(typ, qf)}
597+
if name == "" || name == "_" {
598+
f.Value = typesinternal.ZeroString(typ, qf)
594599
} else {
595-
name = fmt.Sprintf("in%d", index+1)
600+
f.Name = name
596601
}
597-
data.Receiver.Constructor.Args = append(data.Receiver.Constructor.Args, field{
598-
Name: name,
599-
Type: types.TypeString(constructor.Signature().Params().At(index).Type(), qf),
600-
})
602+
data.Receiver.Constructor.Args = append(data.Receiver.Constructor.Args, f)
601603
}
602-
for index := range constructor.Signature().Results().Len() {
604+
for i := range constructor.Signature().Results().Len() {
605+
typ := constructor.Signature().Results().At(i).Type()
603606
var name string
604-
if index == 0 {
607+
if i == 0 {
605608
// The first return value must be of type T, *T, or a type whose named
606609
// type is the same as named type of T.
607610
name = varName
608-
} else if index == constructor.Signature().Results().Len()-1 && types.Identical(constructor.Signature().Results().At(index).Type(), errorType) {
611+
} else if i == constructor.Signature().Results().Len()-1 && types.Identical(typ, errorType) {
609612
name = "err"
610613
} else {
611614
// Drop any return values beyond the first and the last.
@@ -614,12 +617,48 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
614617
}
615618
data.Receiver.Constructor.Results = append(data.Receiver.Constructor.Results, field{
616619
Name: name,
617-
Type: types.TypeString(constructor.Signature().Results().At(index).Type(), qf),
620+
Type: types.TypeString(typ, qf),
618621
})
619622
}
620623
}
621624
}
622625

626+
// Resolves duplicate parameter names between the function and its
627+
// receiver's constructor. It adds prefix to the constructor's parameters
628+
// until no conflicts remain.
629+
if data.Receiver != nil && data.Receiver.Constructor != nil {
630+
seen := map[string]bool{}
631+
for _, f := range data.Func.Args {
632+
if f.Name == "" {
633+
continue
634+
}
635+
seen[f.Name] = true
636+
}
637+
638+
// "" for no change, "c" for constructor, "i" for input.
639+
for _, prefix := range []string{"", "c", "c_", "i", "i_"} {
640+
conflict := false
641+
for _, f := range data.Receiver.Constructor.Args {
642+
if f.Name == "" {
643+
continue
644+
}
645+
if seen[prefix+f.Name] {
646+
conflict = true
647+
break
648+
}
649+
}
650+
if !conflict {
651+
for i, f := range data.Receiver.Constructor.Args {
652+
if f.Name == "" {
653+
continue
654+
}
655+
data.Receiver.Constructor.Args[i].Name = prefix + data.Receiver.Constructor.Args[i].Name
656+
}
657+
break
658+
}
659+
}
660+
}
661+
623662
// Compute edits to update imports.
624663
//
625664
// If we're adding to an existing test file, we need to adjust existing

‎gopls/internal/golang/completion/postfix_snippets.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ func (a *postfixTmplArgs) TypeName(t types.Type) (string, error) {
442442

443443
// Zero return the zero value representation of type t
444444
func (a *postfixTmplArgs) Zero(t types.Type) string {
445-
return formatZeroValue(t, a.qf)
445+
return typesinternal.ZeroString(t, a.qf)
446446
}
447447

448448
func (a *postfixTmplArgs) IsIdent() bool {

‎gopls/internal/golang/completion/statements.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"golang.org/x/tools/gopls/internal/golang"
1616
"golang.org/x/tools/gopls/internal/golang/completion/snippet"
1717
"golang.org/x/tools/gopls/internal/protocol"
18+
"golang.org/x/tools/internal/typesinternal"
1819
)
1920

2021
// addStatementCandidates adds full statement completion candidates
@@ -294,7 +295,7 @@ func (c *completer) addErrCheck() {
294295
} else {
295296
snip.WriteText("return ")
296297
for i := 0; i < result.Len()-1; i++ {
297-
snip.WriteText(formatZeroValue(result.At(i).Type(), c.qf))
298+
snip.WriteText(typesinternal.ZeroString(result.At(i).Type(), c.qf))
298299
snip.WriteText(", ")
299300
}
300301
snip.WritePlaceholder(func(b *snippet.Builder) {
@@ -404,7 +405,7 @@ func (c *completer) addReturnZeroValues() {
404405
fmt.Fprintf(&label, ", ")
405406
}
406407

407-
zero := formatZeroValue(result.At(i).Type(), c.qf)
408+
zero := typesinternal.ZeroString(result.At(i).Type(), c.qf)
408409
snip.WritePlaceholder(func(b *snippet.Builder) {
409410
b.WriteText(zero)
410411
})

0 commit comments

Comments
 (0)