Merge pull request #1277 from 99designs/direct-pointer-binding

Support pointers in un/marshal functions

codegen/config/binder.go

@@ -166,7 +166,8 @@ func (b *Binder) PointerTo(ref *TypeReference) *TypeReference {
 type TypeReference struct {
 	Definition  *ast.Definition
 	GQL         *ast.Type
-	GO          types.Type
+	GO          types.Type  // Type of the field being bound. Could be a pointer or a value type of Target.
+	Target      types.Type  // The actual type that we know how to bind to. May require pointer juggling when traversing to fields.
 	CastType    types.Type  // Before calling marshalling functions cast from/to this base type
 	Marshaler   *types.Func // When using external marshalling functions this will point to the Marshal function
 	Unmarshaler *types.Func // When using external marshalling functions this will point to the Unmarshal function
@@ -177,6 +178,7 @@ func (ref *TypeReference) Elem() *TypeReference {
 	if p, isPtr := ref.GO.(*types.Pointer); isPtr {
 		return &TypeReference{
 			GO:          p.Elem(),
+			Target:      ref.Target,
 			GQL:         ref.GQL,
 			CastType:    ref.CastType,
 			Definition:  ref.Definition,
@@ -189,6 +191,7 @@ func (ref *TypeReference) Elem() *TypeReference {
 	if ref.IsSlice() {
 		return &TypeReference{
 			GO:          ref.GO.(*types.Slice).Elem(),
+			Target:      ref.Target,
 			GQL:         ref.GQL.Elem,
 			CastType:    ref.CastType,
 			Definition:  ref.Definition,
@@ -266,6 +269,10 @@ func (t *TypeReference) UnmarshalFunc() string {
 	return "unmarshal" + t.UniquenessKey()
 }
 
+func (t *TypeReference) IsTargetNilable() bool {
+	return IsNilable(t.Target)
+}
+
 func (b *Binder) PushRef(ret *TypeReference) {
 	b.References = append(b.References, ret)
 }
@@ -350,7 +357,7 @@ func (b *Binder) TypeReference(schemaType *ast.Type, bindTarget types.Type) (ret
 			ref.GO = obj.Type()
 			ref.IsMarshaler = true
 		} else if underlying := basicUnderlying(obj.Type()); def.IsLeafType() && underlying != nil && underlying.Kind() == types.String {
-			// Special case for named types wrapping strings. Used by default enum implementations.
+			// TODO delete before v1. Backwards compatibility case for named types wrapping strings (see #595)
 
 			ref.GO = obj.Type()
 			ref.CastType = underlying
@@ -366,6 +373,7 @@ func (b *Binder) TypeReference(schemaType *ast.Type, bindTarget types.Type) (ret
 			ref.GO = obj.Type()
 		}
 
+		ref.Target = ref.GO
 		ref.GO = b.CopyModifiersFromAst(schemaType, ref.GO)
 
 		if bindTarget != nil {

codegen/testserver/otherpkg/model.go

@@ -7,5 +7,6 @@ type (
 )
 
 type Struct struct {
-	Name string
+	Name Scalar
+	Desc *Scalar
 }

codegen/testserver/resolver.go

@@ -7,6 +7,7 @@ import (
 
 	introspection1 "github.com/99designs/gqlgen/codegen/testserver/introspection"
 	invalid_packagename "github.com/99designs/gqlgen/codegen/testserver/invalid-packagename"
+	"github.com/99designs/gqlgen/codegen/testserver/otherpkg"
 )
 
 type Resolver struct{}
@@ -279,7 +280,7 @@ func (r *queryResolver) WrappedStruct(ctx context.Context) (*WrappedStruct, erro
 	panic("not implemented")
 }
 
-func (r *queryResolver) WrappedScalar(ctx context.Context) (WrappedScalar, error) {
+func (r *queryResolver) WrappedScalar(ctx context.Context) (otherpkg.Scalar, error) {
 	panic("not implemented")
 }
 

codegen/testserver/wrapped_type.go

@@ -2,7 +2,7 @@ package testserver
 
 import "github.com/99designs/gqlgen/codegen/testserver/otherpkg"
 
-type WrappedScalar otherpkg.Scalar
+type WrappedScalar = otherpkg.Scalar
 type WrappedStruct otherpkg.Struct
 type WrappedMap otherpkg.Map
 type WrappedSlice otherpkg.Slice

codegen/testserver/wrapped_type.graphql

@@ -7,7 +7,7 @@ extend type Query {
     wrappedSlice: WrappedSlice!
 }
 
-type WrappedStruct { name: String! }
+type WrappedStruct { name: WrappedScalar!, desc: WrappedScalar }
 scalar WrappedScalar
 type WrappedMap { get(key: String!): String! }
 type WrappedSlice { get(idx: Int!): String! }

codegen/type.go

@@ -8,25 +8,25 @@ import (
 
 func (b *builder) buildTypes() map[string]*config.TypeReference {
 	ret := map[string]*config.TypeReference{}
-	var key string
-	var existing *config.TypeReference
-	var found bool
-
 	for _, ref := range b.Binder.References {
-		for ref != nil {
-			key = ref.UniquenessKey()
-			if existing, found = ret[key]; found {
-				// Simplistic check of content which is obviously different.
-				existingGQL := fmt.Sprintf("%v", existing.GQL)
-				newGQL := fmt.Sprintf("%v", ref.GQL)
-				if existingGQL != newGQL {
-					panic(fmt.Sprintf("non-unique key \"%s\", trying to replace %s with %s", key, existingGQL, newGQL))
-				}
-			}
-			ret[key] = ref
+		processType(ret, ref)
+	}
+	return ret
+}
 
-			ref = ref.Elem()
+func processType(ret map[string]*config.TypeReference, ref *config.TypeReference) {
+	key := ref.UniquenessKey()
+	if existing, found := ret[key]; found {
+		// Simplistic check of content which is obviously different.
+		existingGQL := fmt.Sprintf("%v", existing.GQL)
+		newGQL := fmt.Sprintf("%v", ref.GQL)
+		if existingGQL != newGQL {
+			panic(fmt.Sprintf("non-unique key \"%s\", trying to replace %s with %s", key, existingGQL, newGQL))
 		}
 	}
-	return ret
+	ret[key] = ref
+
+	if ref.IsSlice() {
+		processType(ret, ref.Elem())
+	}
 }

codegen/type.gotpl

@@ -4,10 +4,7 @@
 			{{- if and $type.IsNilable (not $type.GQL.NonNull) }}
 				if v == nil { return nil, nil }
 			{{- end }}
-			{{- if $type.IsPtr }}
-				res, err := ec.{{ $type.Elem.UnmarshalFunc }}(ctx, v)
-				return &res, graphql.WrapErrorWithInputPath(ctx, err)
-			{{- else if $type.IsSlice }}
+			{{- if $type.IsSlice }}
 				var vSlice []interface{}
 				if v != nil {
 					if tmp1, ok := v.([]interface{}); ok {
@@ -30,20 +27,38 @@
 				{{- if $type.Unmarshaler }}
 					{{- if $type.CastType }}
 						tmp, err := {{ $type.Unmarshaler | call }}(v)
-						return {{ $type.GO | ref }}(tmp), graphql.WrapErrorWithInputPath(ctx, err)
+						{{- if $type.IsNilable }}
+							res := {{ $type.Elem.GO | ref }}(tmp)
+						{{- else}}
+							res := {{ $type.GO | ref }}(tmp)
+						{{- end }}
 					{{- else}}
 						res, err := {{ $type.Unmarshaler | call }}(v)
+					{{- end }}
+					{{- if and $type.IsTargetNilable (not $type.IsNilable) }}
+						return *res, graphql.WrapErrorWithInputPath(ctx, err)
+					{{- else if and (not $type.IsTargetNilable) $type.IsNilable }}
+						return &res, graphql.WrapErrorWithInputPath(ctx, err)
+					{{- else}}
 						return res, graphql.WrapErrorWithInputPath(ctx, err)
 					{{- end }}
 				{{- else if eq ($type.GO | ref) "map[string]interface{}" }}
 					return v.(map[string]interface{}), nil
 				{{- else if $type.IsMarshaler }}
-					var res {{ $type.GO | ref }}
+					{{- if $type.IsNilable }}
+						var res = new({{ $type.Elem.GO | ref }})
+					{{- else}}
+						var res {{ $type.GO | ref }}
+					{{- end }}
 					err := res.UnmarshalGQL(v)
 					return res, graphql.WrapErrorWithInputPath(ctx, err)
 				{{- else }}
 					res, err := ec.unmarshalInput{{ $type.GQL.Name }}(ctx, v)
-					return res, graphql.WrapErrorWithInputPath(ctx, err)
+					{{- if $type.IsNilable }}
+						return &res, graphql.WrapErrorWithInputPath(ctx, err)
+					{{- else}}
+						return res, graphql.WrapErrorWithInputPath(ctx, err)
+					{{- end }}
 				{{- end }}
 			{{- end }}
 		}
@@ -110,18 +125,22 @@
 				{{- if $type.IsMarshaler }}
 					return v
 				{{- else if $type.Marshaler }}
-					{{- if $type.IsPtr }}
-						return ec.{{ $type.Elem.MarshalFunc }}(ctx, sel, *v)
-					{{- else if $type.GQL.NonNull }}
-							res := {{ $type.Marshaler | call }}({{- if $type.CastType }}{{ $type.CastType | ref }}(v){{else}}v{{- end }})
+					{{- $v := "v" }}
+					{{- if and $type.IsTargetNilable (not $type.IsNilable) }}
+						{{- $v = "&v" }}
+					{{- else if and (not $type.IsTargetNilable) $type.IsNilable }}
+						{{- $v = "*v" }}
+					{{- end }}
+					{{- if $type.GQL.NonNull }}
+							res := {{ $type.Marshaler | call }}({{- if $type.CastType }}{{ $type.CastType | ref }}({{ $v }}){{else}}{{ $v }}{{- end }})
 							if res == graphql.Null {
 								if !graphql.HasFieldError(ctx, graphql.GetFieldContext(ctx)) {
 									ec.Errorf(ctx, "must not be null")
 								}
 							}
 							return res
 					{{- else }}
-						return {{ $type.Marshaler | call }}({{- if $type.CastType }}{{ $type.CastType | ref }}(v){{else}}v{{- end }})
+						return {{ $type.Marshaler | call }}({{- if $type.CastType }}{{ $type.CastType | ref }}({{ $v }}){{else}}{{ $v }}{{- end }})
 					{{- end }}
 				{{- else }}
 					return ec._{{$type.Definition.Name}}(ctx, sel, {{ if not $type.IsNilable}}&{{end}} v)

docs/content/reference/scalars.md

@@ -60,7 +60,6 @@ package mypkg
 import (
 	"fmt"
 	"io"
-	"strings"
 )
 
 type YesNo bool
@@ -69,7 +68,7 @@ type YesNo bool
 func (y *YesNo) UnmarshalGQL(v interface{}) error {
 	yes, ok := v.(string)
 	if !ok {
-		return fmt.Errorf("points must be strings")
+		return fmt.Errorf("YesNo must be a string")
 	}
 
 	if yes == "yes" {
@@ -90,7 +89,7 @@ func (y YesNo) MarshalGQL(w io.Writer) {
 }
 ```
 
-and then in .gqlgen.yml point to the name without the Marshal|Unmarshal in front:
+and then wire up the type in .gqlgen.yml or via directives like normal:
 
 ```yaml
 models:
@@ -100,8 +99,8 @@ models:
 
 ## Custom scalars with third party types
 
-Sometimes you cant add methods to a type because its in another repo, part of the standard
-library (eg string or time.Time). To do this we can build an external marshaler:
+Sometimes you are unable to add add methods to a type - perhaps you don't own the type, or it is part of the standard
+library (eg string or time.Time). To support this we can build an external marshaler:
 
 ```go
 package mypkg
@@ -147,6 +146,9 @@ models:
     model: github.com/me/mypkg.MyCustomBooleanScalar
 ```
 
+**Note:** you also can un/marshal to pointer types via this approach, simply accept a pointer in your
+`Marshal...` func and return one in your `Unmarshal...` func.
+
 See the [example/scalars](https://github.com/99designs/gqlgen/tree/master/example/scalars) package for more examples.
 
 ## Unmarshaling Errors