internal: fill struct with initialized values instead of nil values
The previous implementation filled map, slice, channel, signature, and
pointer types with nil values rather than initializing these types.
Change-Id: I558446af31fdd8877db5eb3cd92b4004f5d5ab22
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239039
Run-TryBot: Josh Baum <joshbaum@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/internal/analysisinternal/analysis.go b/internal/analysisinternal/analysis.go
index 2658681..14b96a7 100644
--- a/internal/analysisinternal/analysis.go
+++ b/internal/analysisinternal/analysis.go
@@ -48,7 +48,7 @@
case *types.Chan, *types.Interface, *types.Map, *types.Pointer, *types.Signature, *types.Slice:
return ast.NewIdent("nil")
case *types.Struct:
- texpr := typeExpr(fset, f, pkg, typ) // typ because we want the name here.
+ texpr := TypeExpr(fset, f, pkg, typ) // typ because we want the name here.
if texpr == nil {
return nil
}
@@ -56,7 +56,7 @@
Type: texpr,
}
case *types.Array:
- texpr := typeExpr(fset, f, pkg, u.Elem())
+ texpr := TypeExpr(fset, f, pkg, u.Elem())
if texpr == nil {
return nil
}
@@ -70,7 +70,7 @@
return nil
}
-func typeExpr(fset *token.FileSet, f *ast.File, pkg *types.Package, typ types.Type) ast.Expr {
+func TypeExpr(fset *token.FileSet, f *ast.File, pkg *types.Package, typ types.Type) ast.Expr {
switch t := typ.(type) {
case *types.Basic:
switch t.Kind() {
@@ -101,6 +101,11 @@
X: ast.NewIdent(pkgName),
Sel: ast.NewIdent(t.Obj().Name()),
}
+ case *types.Pointer:
+ return &ast.UnaryExpr{
+ Op: token.MUL,
+ X: TypeExpr(fset, f, pkg, t.Elem()),
+ }
default:
return nil // TODO: anonymous structs, but who does that
}
diff --git a/internal/lsp/analysis/fillstruct/fillstruct.go b/internal/lsp/analysis/fillstruct/fillstruct.go
index 23e1662..8fb4585 100644
--- a/internal/lsp/analysis/fillstruct/fillstruct.go
+++ b/internal/lsp/analysis/fillstruct/fillstruct.go
@@ -111,7 +111,7 @@
continue
}
- value := analysisinternal.ZeroValue(pass.Fset, file, pass.Pkg, field.Type())
+ value := populateValue(pass.Fset, file, pass.Pkg, field.Type())
if value == nil {
continue
}
@@ -144,9 +144,9 @@
return
}
- msg := "Fill struct with default values"
+ msg := "Fill struct"
if name, ok := expr.Type.(*ast.Ident); ok {
- msg = fmt.Sprintf("Fill %s with default values", name)
+ msg = fmt.Sprintf("Fill %s", name)
}
pass.Report(analysis.Diagnostic{
@@ -172,3 +172,136 @@
}
return tok.LineStart(line + 1)
}
+
+// populateValue constructs an expression to fill the value of a struct field.
+//
+// When the type of a struct field is a basic literal or interface, we return
+// default values. For other types, such as maps, slices, and channels, we create
+// expressions rather than using default values.
+//
+// The reasoning here is that users will call fillstruct with the intention of
+// initializing the struct, in which case setting these fields to nil has no effect.
+func populateValue(fset *token.FileSet, f *ast.File, pkg *types.Package, typ types.Type) ast.Expr {
+ under := typ
+ if n, ok := typ.(*types.Named); ok {
+ under = n.Underlying()
+ }
+ switch u := under.(type) {
+ case *types.Basic:
+ switch {
+ case u.Info()&types.IsNumeric != 0:
+ return &ast.BasicLit{Kind: token.INT, Value: "0"}
+ case u.Info()&types.IsBoolean != 0:
+ return &ast.Ident{Name: "false"}
+ case u.Info()&types.IsString != 0:
+ return &ast.BasicLit{Kind: token.STRING, Value: `""`}
+ default:
+ panic("unknown basic type")
+ }
+ case *types.Map:
+ k := analysisinternal.TypeExpr(fset, f, pkg, u.Key())
+ v := analysisinternal.TypeExpr(fset, f, pkg, u.Elem())
+ if k == nil || v == nil {
+ return nil
+ }
+ return &ast.CompositeLit{
+ Type: &ast.MapType{
+ Key: k,
+ Value: v,
+ },
+ }
+ case *types.Slice:
+ s := analysisinternal.TypeExpr(fset, f, pkg, u.Elem())
+ if s == nil {
+ return nil
+ }
+ return &ast.CompositeLit{
+ Type: &ast.ArrayType{
+ Elt: s,
+ },
+ }
+ case *types.Array:
+ a := analysisinternal.TypeExpr(fset, f, pkg, u.Elem())
+ if a == nil {
+ return nil
+ }
+ return &ast.CompositeLit{
+ Type: &ast.ArrayType{
+ Elt: a,
+ Len: &ast.BasicLit{
+ Kind: token.INT, Value: fmt.Sprintf("%v", u.Len())},
+ },
+ }
+ case *types.Chan:
+ v := analysisinternal.TypeExpr(fset, f, pkg, u.Elem())
+ if v == nil {
+ return nil
+ }
+ dir := ast.ChanDir(u.Dir())
+ if u.Dir() == types.SendRecv {
+ dir = ast.SEND | ast.RECV
+ }
+ return &ast.CallExpr{
+ Fun: ast.NewIdent("make"),
+ Args: []ast.Expr{
+ &ast.ChanType{
+ Dir: dir,
+ Value: v,
+ },
+ },
+ }
+ case *types.Struct:
+ s := analysisinternal.TypeExpr(fset, f, pkg, typ)
+ if s == nil {
+ return nil
+ }
+ return &ast.CompositeLit{
+ Type: s,
+ }
+ case *types.Signature:
+ var params []*ast.Field
+ for i := 0; i < u.Params().Len(); i++ {
+ p := analysisinternal.TypeExpr(fset, f, pkg, u.Params().At(i).Type())
+ if p == nil {
+ return nil
+ }
+ params = append(params, &ast.Field{
+ Type: p,
+ Names: []*ast.Ident{
+ {
+ Name: u.Params().At(i).Name(),
+ },
+ },
+ })
+ }
+ var returns []*ast.Field
+ for i := 0; i < u.Results().Len(); i++ {
+ r := analysisinternal.TypeExpr(fset, f, pkg, u.Results().At(i).Type())
+ if r == nil {
+ return nil
+ }
+ returns = append(returns, &ast.Field{
+ Type: r,
+ })
+ }
+ return &ast.FuncLit{
+ Type: &ast.FuncType{
+ Params: &ast.FieldList{
+ List: params,
+ },
+ Results: &ast.FieldList{
+ List: returns,
+ },
+ },
+ Body: &ast.BlockStmt{},
+ }
+ case *types.Pointer:
+ return &ast.UnaryExpr{
+ Op: token.AND,
+ X: populateValue(fset, f, pkg, u.Elem()),
+ }
+ case *types.Interface:
+ return ast.NewIdent("nil")
+ }
+ return nil
+}
diff --git a/internal/lsp/analysis/fillstruct/testdata/src/a/a.go b/internal/lsp/analysis/fillstruct/testdata/src/a/a.go
index 447ed15..8358383 100644
--- a/internal/lsp/analysis/fillstruct/testdata/src/a/a.go
+++ b/internal/lsp/analysis/fillstruct/testdata/src/a/a.go
@@ -6,6 +6,8 @@
import (
data "b"
+ "go/ast"
+ "go/token"
)
type emptyStruct struct{}
@@ -37,3 +39,53 @@
var _ = nestedStruct{} // want ""
var _ = data.B{} // want ""
+
+type typedStruct struct {
+ m map[string]int
+ s []int
+ c chan int
+ c1 <-chan int
+ a [2]string
+}
+
+var _ = typedStruct{} // want ""
+
+type funStruct struct {
+ fn func(i int) int
+}
+
+var _ = funStruct{} // want ""
+
+type funStructCompex struct {
+ fn func(i int, s string) (string, int)
+}
+
+var _ = funStructCompex{} // want ""
+
+type funStructEmpty struct {
+ fn func()
+}
+
+var _ = funStructEmpty{} // want ""
+
+type Foo struct {
+ A int
+}
+
+type Bar struct {
+ X *Foo
+ Y *Foo
+}
+
+var _ = Bar{} // want ""
+
+type importedStruct struct {
+ m map[*ast.CompositeLit]ast.Field
+ s []ast.BadExpr
+ a [3]token.Token
+ c chan ast.EmptyStmt
+ fn func(ast_decl ast.DeclStmt) ast.Ellipsis
+ st ast.CompositeLit
+}
+
+var _ = importedStruct{} // want ""
diff --git a/internal/lsp/analysis/fillstruct/testdata/src/a/a.go.golden b/internal/lsp/analysis/fillstruct/testdata/src/a/a.go.golden
index 3fc564b..2f52bf5 100644
--- a/internal/lsp/analysis/fillstruct/testdata/src/a/a.go.golden
+++ b/internal/lsp/analysis/fillstruct/testdata/src/a/a.go.golden
@@ -6,6 +6,8 @@
import (
data "b"
+ "go/ast"
+ "go/token"
)
type emptyStruct struct{}
@@ -47,3 +49,79 @@
var _ = data.B{
ExportedInt: 0,
} // want ""
+
+type typedStruct struct {
+ m map[string]int
+ s []int
+ c chan int
+ c1 <-chan int
+ a [2]string
+}
+
+var _ = typedStruct{
+ m: map[string]int{},
+ s: []int{},
+ c: make(chan int),
+ c1: make(<-chan int),
+ a: [2]string{},
+} // want ""
+
+type funStruct struct {
+ fn func(i int) int
+}
+
+var _ = funStruct{
+ fn: func(i int) int {
+ },
+} // want ""
+
+type funStructCompex struct {
+ fn func(i int, s string) (string, int)
+}
+
+var _ = funStructCompex{
+ fn: func(i int, s string) (string, int) {
+ },
+} // want ""
+
+type funStructEmpty struct {
+ fn func()
+}
+
+var _ = funStructEmpty{
+ fn: func() {
+ },
+} // want ""
+
+type Foo struct {
+ A int
+}
+
+type Bar struct {
+ X *Foo
+ Y *Foo
+}
+
+var _ = Bar{
+ X: &Foo{},
+ Y: &Foo{},
+} // want ""
+
+type importedStruct struct {
+ m map[*ast.CompositeLit]ast.Field
+ s []ast.BadExpr
+ a [3]token.Token
+ c chan ast.EmptyStmt
+ fn func(ast_decl ast.DeclStmt) ast.Ellipsis
+ st ast.CompositeLit
+}
+
+var _ = importedStruct{
+ m: map[*ast.CompositeLit]ast.Field{},
+ s: []ast.BadExpr{},
+ a: [3]token.Token{},
+ c: make(chan ast.EmptyStmt),
+ fn: func(ast_decl ast.DeclStmt) ast.Ellipsis {
+ },
+ st: ast.CompositeLit{},
+} // want ""
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go.golden b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go.golden
index 162b4c4..7fb2752 100644
--- a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go.golden
@@ -21,8 +21,8 @@
a := StructA{
unexportedIntField: 0,
ExportedIntField: 0,
- MapA: nil,
- Array: nil,
+ MapA: map[int]string{},
+ Array: []int{},
StructB: StructB{},
} //@suggestedfix("}", "refactor.rewrite")
b := StructA2{} //@suggestedfix("}", "refactor.rewrite")
@@ -51,7 +51,7 @@
func fill() {
a := StructA{} //@suggestedfix("}", "refactor.rewrite")
b := StructA2{
- B: nil,
+ B: &StructB{},
} //@suggestedfix("}", "refactor.rewrite")
c := StructA3{} //@suggestedfix("}", "refactor.rewrite")
}
