Skip to content

Commit

Permalink
Only make cyclical struct fields pointers
Browse files Browse the repository at this point in the history
  • Loading branch information
ianling committed May 20, 2022
1 parent 4cdf702 commit e15da34
Show file tree
Hide file tree
Showing 12 changed files with 521 additions and 24 deletions.
42 changes: 22 additions & 20 deletions codegen/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,22 @@ import (
)

type Config struct {
SchemaFilename StringList `yaml:"schema,omitempty"`
Exec ExecConfig `yaml:"exec"`
Model PackageConfig `yaml:"model,omitempty"`
Federation PackageConfig `yaml:"federation,omitempty"`
Resolver ResolverConfig `yaml:"resolver,omitempty"`
AutoBind []string `yaml:"autobind"`
Models TypeMap `yaml:"models,omitempty"`
StructTag string `yaml:"struct_tag,omitempty"`
Directives map[string]DirectiveConfig `yaml:"directives,omitempty"`
OmitSliceElementPointers bool `yaml:"omit_slice_element_pointers,omitempty"`
SkipValidation bool `yaml:"skip_validation,omitempty"`
SkipModTidy bool `yaml:"skip_mod_tidy,omitempty"`
Sources []*ast.Source `yaml:"-"`
Packages *code.Packages `yaml:"-"`
Schema *ast.Schema `yaml:"-"`
SchemaFilename StringList `yaml:"schema,omitempty"`
Exec ExecConfig `yaml:"exec"`
Model PackageConfig `yaml:"model,omitempty"`
Federation PackageConfig `yaml:"federation,omitempty"`
Resolver ResolverConfig `yaml:"resolver,omitempty"`
AutoBind []string `yaml:"autobind"`
Models TypeMap `yaml:"models,omitempty"`
StructTag string `yaml:"struct_tag,omitempty"`
Directives map[string]DirectiveConfig `yaml:"directives,omitempty"`
OmitSliceElementPointers bool `yaml:"omit_slice_element_pointers,omitempty"`
StructFieldsAlwaysPointers bool `yaml:"struct_fields_always_pointers,omitempty"`
SkipValidation bool `yaml:"skip_validation,omitempty"`
SkipModTidy bool `yaml:"skip_mod_tidy,omitempty"`
Sources []*ast.Source `yaml:"-"`
Packages *code.Packages `yaml:"-"`
Schema *ast.Schema `yaml:"-"`

// Deprecated: use Federation instead. Will be removed next release
Federated bool `yaml:"federated,omitempty"`
Expand All @@ -41,11 +42,12 @@ var cfgFilenames = []string{".gqlgen.yml", "gqlgen.yml", "gqlgen.yaml"}
// DefaultConfig creates a copy of the default config
func DefaultConfig() *Config {
return &Config{
SchemaFilename: StringList{"schema.graphql"},
Model: PackageConfig{Filename: "models_gen.go"},
Exec: ExecConfig{Filename: "generated.go"},
Directives: map[string]DirectiveConfig{},
Models: TypeMap{},
SchemaFilename: StringList{"schema.graphql"},
Model: PackageConfig{Filename: "models_gen.go"},
Exec: ExecConfig{Filename: "generated.go"},
Directives: map[string]DirectiveConfig{},
Models: TypeMap{},
StructFieldsAlwaysPointers: true,
}
}

Expand Down
6 changes: 5 additions & 1 deletion docs/content/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ resolver:
# Optional: turn on to use []Thing instead of []*Thing
# omit_slice_element_pointers: false

# Optional: turn off to make struct-type struct fields not use pointers
# e.g. type Thing struct { FieldA OtherThing } instead of { FieldA *OtherThing }
# struct_fields_always_pointers: true

# Optional: set to speed up generation time by not performing a final validation pass.
# skip_validation: true

Expand Down Expand Up @@ -116,7 +120,7 @@ type User @goModel(model: "github.com/my/app/models.User") {
}
```

The builtin directives `goField`, `goModel` and `goTag` are automatically registered to `skip_runtime`. Any directives registered as `skip_runtime` will not exposed during introspection and are used during code generation only.
The builtin directives `goField`, `goModel` and `goTag` are automatically registered to `skip_runtime`. Any directives registered as `skip_runtime` will not exposed during introspection and are used during code generation only.

If you have created a new code generation plugin using a directive which does not require runtime execution, the directive will need to be set to `skip_runtime`.

Expand Down
4 changes: 4 additions & 0 deletions init-templates/gqlgen.yml.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ resolver:
# Optional: turn on to use []Thing instead of []*Thing
# omit_slice_element_pointers: false

# Optional: turn off to make struct-type struct fields not use pointers
# e.g. type Thing struct { FieldA OtherThing } instead of { FieldA *OtherThing }
# struct_fields_always_pointers: true

# Optional: set to speed up generation time by not performing a final validation pass.
# skip_validation: true

Expand Down
64 changes: 62 additions & 2 deletions plugin/modelgen/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,10 @@ func (m *Plugin) MutateConfig(cfg *config.Config) error {

typ = binder.CopyModifiersFromAst(field.Type, typ)

if isStruct(typ) && (fieldDef.Kind == ast.Object || fieldDef.Kind == ast.InputObject) {
typ = types.NewPointer(typ)
if cfg.StructFieldsAlwaysPointers {
if isStruct(typ) && (fieldDef.Kind == ast.Object || fieldDef.Kind == ast.InputObject) {
typ = types.NewPointer(typ)
}
}

f := &Field{
Expand Down Expand Up @@ -230,6 +232,12 @@ func (m *Plugin) MutateConfig(cfg *config.Config) error {
sort.Slice(b.Models, func(i, j int) bool { return b.Models[i].Name < b.Models[j].Name })
sort.Slice(b.Interfaces, func(i, j int) bool { return b.Interfaces[i].Name < b.Interfaces[j].Name })

// if we are not just turning all struct-type fields in generated structs into pointers, we need to at least
// check for cyclical relationships and recursive structs
if !cfg.StructFieldsAlwaysPointers {
findAndHandleCyclicalRelationships(b)
}

for _, it := range b.Enums {
cfg.Models.Add(it.Name, cfg.Model.ImportPath()+"."+templates.ToGo(it.Name))
}
Expand Down Expand Up @@ -303,3 +311,55 @@ func isStruct(t types.Type) bool {
_, is := t.Underlying().(*types.Struct)
return is
}

// findAndHandleCyclicalRelationships checks for cyclical relationships between generated structs and replaces them
// with pointers. These relationships will produce compilation errors if they are not pointers.
// Also handles recursive structs.
func findAndHandleCyclicalRelationships(b *ModelBuild) {
for ii, structA := range b.Models {
for _, fieldA := range structA.Fields {
if strings.Contains(fieldA.Type.String(), "NotCyclicalA") {
fmt.Print()
}
if !isStruct(fieldA.Type) {
continue
}

// the field Type string will be in the form "github.com/99designs/gqlgen/codegen/testserver/followschema.LoopA"
// we only want the part after the last dot: "LoopA"
// this could lead to false positives, as we are only checking the name of the struct type, but these
// should be extremely rare, if it is even possible at all.
fieldAStructNameParts := strings.Split(fieldA.Type.String(), ".")
fieldAStructName := fieldAStructNameParts[len(fieldAStructNameParts)-1]

// find this struct type amongst the generated structs
for jj, structB := range b.Models {
if structB.Name != fieldAStructName {
continue
}

// check if structB contains a cyclical reference back to structA
var cyclicalReferenceFound bool
for _, fieldB := range structB.Fields {
if !isStruct(fieldB.Type) {
continue
}

fieldBStructNameParts := strings.Split(fieldB.Type.String(), ".")
fieldBStructName := fieldBStructNameParts[len(fieldBStructNameParts)-1]
if fieldBStructName == structA.Name {
cyclicalReferenceFound = true
fieldB.Type = types.NewPointer(fieldB.Type)
// keep looping in case this struct has additional fields of this type
}
}

// if this is a recursive struct (i.e. structA == structB), ensure that we only change this field to a pointer once
if cyclicalReferenceFound && ii != jj {
fieldA.Type = types.NewPointer(fieldA.Type)
break
}
}
}
}
}
67 changes: 67 additions & 0 deletions plugin/modelgen/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"strings"
"testing"

"github.com/99designs/gqlgen/plugin/modelgen/out_struct_pointers"

"github.com/99designs/gqlgen/codegen/config"
"github.com/99designs/gqlgen/plugin/modelgen/out"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -210,6 +212,71 @@ func TestModelGeneration(t *testing.T) {
sort.Strings(gots)
require.Equal(t, wantMethods, gots)
})

t.Run("cyclical struct fields become pointers", func(t *testing.T) {
require.Nil(t, out.CyclicalA{}.FieldOne)
require.Nil(t, out.CyclicalA{}.FieldTwo)
require.Nil(t, out.CyclicalA{}.FieldThree)
require.NotNil(t, out.CyclicalA{}.FieldFour)
require.Nil(t, out.CyclicalB{}.FieldOne)
require.Nil(t, out.CyclicalB{}.FieldTwo)
require.Nil(t, out.CyclicalB{}.FieldThree)
require.Nil(t, out.CyclicalB{}.FieldFour)
require.NotNil(t, out.CyclicalB{}.FieldFive)
})

t.Run("non-cyclical struct fields become pointers", func(t *testing.T) {
require.NotNil(t, out.NotCyclicalB{}.FieldOne)
require.Nil(t, out.NotCyclicalB{}.FieldTwo)
})

t.Run("recursive struct fields become pointers", func(t *testing.T) {
require.Nil(t, out.Recursive{}.FieldOne)
require.Nil(t, out.Recursive{}.FieldTwo)
require.Nil(t, out.Recursive{}.FieldThree)
require.NotNil(t, out.Recursive{}.FieldFour)
})
}

func TestModelGenerationStructFieldPointers(t *testing.T) {
cfg, err := config.LoadConfig("testdata/gqlgen_struct_field_pointers.yml")
require.NoError(t, err)
require.NoError(t, cfg.Init())
p := Plugin{
MutateHook: mutateHook,
FieldHook: defaultFieldMutateHook,
}
require.NoError(t, p.MutateConfig(cfg))

t.Run("no pointer pointers", func(t *testing.T) {
generated, err := ioutil.ReadFile("./out_struct_pointers/generated.go")
require.NoError(t, err)
require.NotContains(t, string(generated), "**")
})

t.Run("cyclical struct fields become pointers", func(t *testing.T) {
require.Nil(t, out_struct_pointers.CyclicalA{}.FieldOne)
require.Nil(t, out_struct_pointers.CyclicalA{}.FieldTwo)
require.Nil(t, out_struct_pointers.CyclicalA{}.FieldThree)
require.NotNil(t, out_struct_pointers.CyclicalA{}.FieldFour)
require.Nil(t, out_struct_pointers.CyclicalB{}.FieldOne)
require.Nil(t, out_struct_pointers.CyclicalB{}.FieldTwo)
require.Nil(t, out_struct_pointers.CyclicalB{}.FieldThree)
require.Nil(t, out_struct_pointers.CyclicalB{}.FieldFour)
require.NotNil(t, out_struct_pointers.CyclicalB{}.FieldFive)
})

t.Run("non-cyclical struct fields do not become pointers", func(t *testing.T) {
require.NotNil(t, out_struct_pointers.NotCyclicalB{}.FieldOne)
require.NotNil(t, out_struct_pointers.NotCyclicalB{}.FieldTwo)
})

t.Run("recursive struct fields become pointers", func(t *testing.T) {
require.Nil(t, out_struct_pointers.Recursive{}.FieldOne)
require.Nil(t, out_struct_pointers.Recursive{}.FieldTwo)
require.Nil(t, out_struct_pointers.Recursive{}.FieldThree)
require.NotNil(t, out_struct_pointers.Recursive{}.FieldFour)
})
}

func mutateHook(b *ModelBuild) *ModelBuild {
Expand Down
32 changes: 32 additions & 0 deletions plugin/modelgen/out/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions plugin/modelgen/out_struct_pointers/existing.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package out_struct_pointers

type ExistingType struct {
Name *string `json:"name"`
Enum *ExistingEnum `json:"enum"`
Int ExistingInterface `json:"int"`
Existing *MissingTypeNullable `json:"existing"`
}

type ExistingModel struct {
Name string
Enum ExistingEnum
Int ExistingInterface
}

type ExistingInput struct {
Name string
Enum ExistingEnum
Int ExistingInterface
}

type ExistingEnum string

type ExistingInterface interface {
IsExistingInterface()
}

type ExistingUnion interface {
IsExistingUnion()
}
Loading

0 comments on commit e15da34

Please sign in to comment.