add option to force using a pointer

This commit is contained in:
Ben Kraft
2021-04-09 18:51:44 -07:00
parent 74411faf52
commit e597cac74c
14 changed files with 351 additions and 65 deletions
+42 -8
View File
@@ -50,7 +50,31 @@ type GenqlientDirective struct {
// value otherwise.
//
// Only applicable to arguments of nullable types.
Omitempty bool
Omitempty *bool
// If set, this argument or field will use a pointer type in Go. Response
// types always use pointers, but otherwise we typically do not.
//
// This can be useful if it's a type you'll need to pass around (and want a
// pointer to save copies) or if you wish to distinguish between the Go
// zero value and null (for nullable fields).
Pointer *bool
}
func (g *GenqlientDirective) GetOmitempty() bool { return g.Omitempty != nil && *g.Omitempty }
func (g *GenqlientDirective) GetPointer() bool { return g.Pointer != nil && *g.Pointer }
func setBool(dst **bool, v *ast.Value) error {
ei, err := v.Value(nil) // no vars allowed
// TODO: here and below, put positions on these errors
if err != nil {
return fmt.Errorf("invalid boolean value %v: %w", v, err)
}
if b, ok := ei.(bool); ok {
*dst = &b
return nil
}
return fmt.Errorf("expected boolean, got non-boolean value %T(%v)", ei, ei)
}
func fromGraphQL(dir *ast.Directive) (*GenqlientDirective, error) {
@@ -61,13 +85,20 @@ func fromGraphQL(dir *ast.Directive) (*GenqlientDirective, error) {
}
var retval GenqlientDirective
var err error
for _, arg := range dir.Arguments {
switch arg.Name {
// TODO: reflect and struct tags?
case "omitempty":
retval.Omitempty = true
err = setBool(&retval.Omitempty, arg.Value)
case "pointer":
err = setBool(&retval.Pointer, arg.Value)
default:
return nil, fmt.Errorf("unknown argument %v for @genqlient", arg.Name)
}
if err != nil {
return nil, err
}
}
return &retval, nil
}
@@ -79,12 +110,12 @@ func (dir *GenqlientDirective) validate(node interface{}) error {
// whatever it is relevant to.
return nil
case *ast.VariableDefinition:
if dir.Omitempty && node.Type.NonNull {
if dir.Omitempty != nil && node.Type.NonNull {
return fmt.Errorf("omitempty may only be used on optional arguments")
}
return nil
case *ast.Field:
if dir.Omitempty {
if dir.Omitempty != nil {
return fmt.Errorf("omitempty is not appilcable to fields")
}
return nil
@@ -94,11 +125,13 @@ func (dir *GenqlientDirective) validate(node interface{}) error {
}
func (dir *GenqlientDirective) merge(other *GenqlientDirective) *GenqlientDirective {
if dir == nil {
return other
retval := *dir
if other.Omitempty != nil {
retval.Omitempty = other.Omitempty
}
if other.Pointer != nil {
retval.Pointer = other.Pointer
}
var retval GenqlientDirective
retval.Omitempty = dir.Omitempty || other.Omitempty
return &retval
}
@@ -106,6 +139,7 @@ func (g *generator) parsePrecedingComment(
node interface{},
pos *ast.Position,
) (comment string, directive *GenqlientDirective, err error) {
directive = new(GenqlientDirective)
var commentLines []string
sourceLines := strings.Split(pos.Src.Input, "\n")
for i := pos.Line - 1; i > 0; i-- {
+5 -9
View File
@@ -61,7 +61,7 @@ type argument struct {
GoName string
GoType string
GraphQLName string
Omitempty bool
Options *GenqlientDirective
}
func newGenerator(config *Config, schema *ast.Schema) *generator {
@@ -117,14 +117,10 @@ func (g *generator) getArgument(
if err != nil {
return argument{}, err
}
directive = operationDirective.merge(directive)
omitempty := false
if directive != nil {
omitempty = directive.Omitempty
}
graphQLName := arg.Variable
goType, err := g.getTypeForInputType(opName, arg.Type)
goType, err := g.getTypeForInputType(
opName, arg.Type, directive, operationDirective)
if err != nil {
return argument{}, err
}
@@ -132,7 +128,7 @@ func (g *generator) getArgument(
GraphQLName: graphQLName,
GoName: lowerFirst(graphQLName),
GoType: goType,
Omitempty: omitempty,
Options: directive,
}, nil
}
@@ -163,7 +159,7 @@ func (g *generator) addOperation(op *ast.OperationDefinition) error {
}
}
responseName, err := g.getTypeForOperation(op)
responseName, err := g.getTypeForOperation(op, directive)
if err != nil {
return err
}
+5 -2
View File
@@ -95,8 +95,11 @@ func TestGenerate(t *testing.T) {
}
if string(content) != expectedContent {
t.Errorf("mismatch in %v\ngot:\n%v\nwant:\n%v\n",
filename, string(content), expectedContent)
t.Errorf("mismatch in %v", filename)
if testing.Verbose() {
t.Errorf("got:\n%v\nwant:\n%v\n",
string(content), expectedContent)
}
if update {
t.Log("Updating testdata dir to match")
err = ioutil.WriteFile(filepath.Join(dataDir, filename), content, 0o644)
+4 -2
View File
@@ -24,11 +24,13 @@ func {{.Name}}(
{{- if .Args -}}
variables := map[string]interface{}{
{{range .Args -}}
"{{.GraphQLName}}": {{if .Omitempty}}nil{{else}}{{.GoName}}{{end}},
{{if not .Options.GetOmitempty -}}
"{{.GraphQLName}}": {{.GoName}},
{{end -}}
{{end}}
}
{{range .Args -}}
{{if .Omitempty -}}
{{if .Options.GetOmitempty -}}
{{/* zero_{{.GoType}} would be a better name, but {{.GoType}} would require
munging since it might be, say, `time.Time`. */}}
var zero_{{.GoName}} {{.GoType}}
+3
View File
@@ -3,9 +3,12 @@ query OmitEmptyQuery(
$query: UserQueryInput,
$dt: DateTime,
$tz: String,
# @genqlient(omitempty: false)
$tzNoOmitEmpty: String,
) {
user(query: $query) {
id
}
maybeConvert(dt: $dt, tz: $tz)
convert2: maybeConvert(dt: $dt, tz: $tzNoOmitEmpty)
}
+8 -19
View File
@@ -12,6 +12,7 @@ import (
type OmitEmptyQueryResponse struct {
User OmitEmptyQueryUser `json:"user"`
MaybeConvert time.Time `json:"maybeConvert"`
Convert2 time.Time `json:"convert2"`
}
type OmitEmptyQueryUser struct {
@@ -38,26 +39,13 @@ func OmitEmptyQuery(
query UserQueryInput,
dt time.Time,
tz string,
tzNoOmitEmpty string,
) (*OmitEmptyQueryResponse, error) {
variables := map[string]interface{}{
"query": nil,
"dt": nil,
"tz": nil,
}
var zero_query UserQueryInput
if query != zero_query {
variables["query"] = query
}
var zero_dt time.Time
if dt != zero_dt {
variables["dt"] = dt
}
var zero_tz string
if tz != zero_tz {
variables["tz"] = tz
"query": query,
"dt": dt,
"tz": tz,
"tzNoOmitEmpty": tzNoOmitEmpty,
}
var retval OmitEmptyQueryResponse
@@ -65,11 +53,12 @@ func OmitEmptyQuery(
nil,
"OmitEmptyQuery",
`
query OmitEmptyQuery ($query: UserQueryInput, $dt: DateTime, $tz: String) {
query OmitEmptyQuery ($query: UserQueryInput, $dt: DateTime, $tz: String, $tzNoOmitEmpty: String) {
user(query: $query) {
id
}
maybeConvert(dt: $dt, tz: $tz)
convert2: maybeConvert(dt: $dt, tz: $tzNoOmitEmpty)
}
`,
&retval,
+1 -1
View File
@@ -2,7 +2,7 @@
"operations": [
{
"operationName": "OmitEmptyQuery",
"query": "\nquery OmitEmptyQuery ($query: UserQueryInput, $dt: DateTime, $tz: String) {\n\tuser(query: $query) {\n\t\tid\n\t}\n\tmaybeConvert(dt: $dt, tz: $tz)\n}\n",
"query": "\nquery OmitEmptyQuery ($query: UserQueryInput, $dt: DateTime, $tz: String, $tzNoOmitEmpty: String) {\n\tuser(query: $query) {\n\t\tid\n\t}\n\tmaybeConvert(dt: $dt, tz: $tz)\n\tconvert2: maybeConvert(dt: $dt, tz: $tzNoOmitEmpty)\n}\n",
"sourceLocation": "testdata/queries/Omitempty.graphql"
}
]
+20
View File
@@ -0,0 +1,20 @@
# @genqlient(pointer: true)
query PointersQuery(
$query: UserQueryInput,
# @genqlient(pointer: false)
$dt: DateTime,
$tz: String,
) {
user(query: $query) {
id
roles
name
emails
# @genqlient(pointer: false)
emailsNoPtr: emails
}
otherUser: user(query: $query) {
id
}
maybeConvert(dt: $dt, tz: $tz)
}
+80
View File
@@ -0,0 +1,80 @@
package test
// Code generated by github.com/Khan/genqlient, DO NOT EDIT.
import (
"time"
"github.com/Khan/genqlient/graphql"
"github.com/me/mypkg"
)
type PointersQueryOtherUser struct {
Id *mypkg.ID `json:"id"`
}
type PointersQueryResponse struct {
User *PointersQueryUser `json:"user"`
OtherUser *PointersQueryOtherUser `json:"otherUser"`
MaybeConvert *time.Time `json:"maybeConvert"`
}
type PointersQueryUser struct {
Id *mypkg.ID `json:"id"`
Roles []*Role `json:"roles"`
Name *string `json:"name"`
Emails []*string `json:"emails"`
EmailsNoPtr []string `json:"emailsNoPtr"`
}
type Role string
const (
RoleStudent Role = "STUDENT"
RoleTeacher Role = "TEACHER"
)
type UserQueryInput struct {
Email *string `json:"email"`
Name *string `json:"name"`
Id *mypkg.ID `json:"id"`
Role *Role `json:"role"`
Names []*string `json:"names"`
}
func PointersQuery(
client graphql.Client,
query UserQueryInput,
dt time.Time,
tz string,
) (*PointersQueryResponse, error) {
variables := map[string]interface{}{
"query": query,
"dt": dt,
"tz": tz,
}
var retval PointersQueryResponse
err := client.MakeRequest(
nil,
"PointersQuery",
`
query PointersQuery ($query: UserQueryInput, $dt: DateTime, $tz: String) {
user(query: $query) {
id
roles
name
emails
emailsNoPtr: emails
}
otherUser: user(query: $query) {
id
}
maybeConvert(dt: $dt, tz: $tz)
}
`,
&retval,
variables,
)
return &retval, err
}
+9
View File
@@ -0,0 +1,9 @@
{
"operations": [
{
"operationName": "PointersQuery",
"query": "\nquery PointersQuery ($query: UserQueryInput, $dt: DateTime, $tz: String) {\n\tuser(query: $query) {\n\t\tid\n\t\troles\n\t\tname\n\t\temails\n\t\temailsNoPtr: emails\n\t}\n\totherUser: user(query: $query) {\n\t\tid\n\t}\n\tmaybeConvert(dt: $dt, tz: $tz)\n}\n",
"sourceLocation": "testdata/queries/Pointers.graphql"
}
]
}
+24
View File
@@ -0,0 +1,24 @@
query PointersQuery(
# @genqlient(pointer: true)
$query: UserQueryInput,
# @genqlient(pointer: true)
$dt: DateTime,
$tz: String,
) {
# @genqlient(pointer: true)
user(query: $query) {
id
roles
# @genqlient(pointer: true)
name
# @genqlient(pointer: true)
emails
# @genqlient(pointer: true)
emailsNoPtr: emails
}
# @genqlient(pointer: true)
otherUser: user(query: $query) {
id
}
maybeConvert(dt: $dt, tz: $tz)
}
+80
View File
@@ -0,0 +1,80 @@
package test
// Code generated by github.com/Khan/genqlient, DO NOT EDIT.
import (
"time"
"github.com/Khan/genqlient/graphql"
"github.com/me/mypkg"
)
type PointersQueryOtherUser struct {
Id mypkg.ID `json:"id"`
}
type PointersQueryResponse struct {
User *PointersQueryUser `json:"user"`
OtherUser *PointersQueryOtherUser `json:"otherUser"`
MaybeConvert time.Time `json:"maybeConvert"`
}
type PointersQueryUser struct {
Id mypkg.ID `json:"id"`
Roles []Role `json:"roles"`
Name *string `json:"name"`
Emails []*string `json:"emails"`
EmailsNoPtr []*string `json:"emailsNoPtr"`
}
type Role string
const (
RoleStudent Role = "STUDENT"
RoleTeacher Role = "TEACHER"
)
type UserQueryInput struct {
Email string `json:"email"`
Name string `json:"name"`
Id mypkg.ID `json:"id"`
Role Role `json:"role"`
Names []string `json:"names"`
}
func PointersQuery(
client graphql.Client,
query *UserQueryInput,
dt *time.Time,
tz string,
) (*PointersQueryResponse, error) {
variables := map[string]interface{}{
"query": query,
"dt": dt,
"tz": tz,
}
var retval PointersQueryResponse
err := client.MakeRequest(
nil,
"PointersQuery",
`
query PointersQuery ($query: UserQueryInput, $dt: DateTime, $tz: String) {
user(query: $query) {
id
roles
name
emails
emailsNoPtr: emails
}
otherUser: user(query: $query) {
id
}
maybeConvert(dt: $dt, tz: $tz)
}
`,
&retval,
variables,
)
return &retval, err
}
+9
View File
@@ -0,0 +1,9 @@
{
"operations": [
{
"operationName": "PointersQuery",
"query": "\nquery PointersQuery ($query: UserQueryInput, $dt: DateTime, $tz: String) {\n\tuser(query: $query) {\n\t\tid\n\t\troles\n\t\tname\n\t\temails\n\t\temailsNoPtr: emails\n\t}\n\totherUser: user(query: $query) {\n\t\tid\n\t}\n\tmaybeConvert(dt: $dt, tz: $tz)\n}\n",
"sourceLocation": "testdata/queries/PointersInline.graphql"
}
]
}
+61 -24
View File
@@ -30,7 +30,7 @@ func (g *generator) baseTypeForOperation(operation ast.Operation) (*ast.Definiti
}
}
func (g *generator) getTypeForOperation(operation *ast.OperationDefinition) (name string, err error) {
func (g *generator) getTypeForOperation(operation *ast.OperationDefinition, queryOptions *GenqlientDirective) (name string, err error) {
// TODO: configure ResponseName format
name = operation.Name + "Response"
@@ -39,7 +39,7 @@ func (g *generator) getTypeForOperation(operation *ast.OperationDefinition) (nam
return "", fmt.Errorf("%s defined twice:\n%s", name, def)
}
fields, err := selections(operation.SelectionSet)
fields, err := selections(g, operation.SelectionSet, queryOptions)
if err != nil {
return "", err
}
@@ -49,7 +49,7 @@ func (g *generator) getTypeForOperation(operation *ast.OperationDefinition) (nam
return "", err
}
return g.addTypeForDefinition(operation.Name, name, baseType, fields)
return g.addTypeForDefinition(operation.Name, name, baseType, fields, queryOptions)
}
var builtinTypes = map[string]string{
@@ -61,7 +61,7 @@ var builtinTypes = map[string]string{
"ID": "string",
}
func (g *generator) addTypeForDefinition(namePrefix, nameOverride string, typ *ast.Definition, fields []field) (name string, err error) {
func (g *generator) addTypeForDefinition(namePrefix, nameOverride string, typ *ast.Definition, fields []field, options *GenqlientDirective) (name string, err error) {
// If this is a builtin type or custom scalar, just refer to it.
goName, ok := g.Config.Scalars[typ.Name]
if ok {
@@ -114,7 +114,7 @@ func (g *generator) addTypeForDefinition(namePrefix, nameOverride string, typ *a
// name.
builder := &typeBuilder{typeName: name, typeNamePrefix: namePrefix, generator: g}
fmt.Fprintf(builder, "type %s ", name)
err = builder.writeTypedef(typ, fields)
err = builder.writeTypedef(typ, fields, options)
if err != nil {
return "", err
}
@@ -124,22 +124,36 @@ func (g *generator) addTypeForDefinition(namePrefix, nameOverride string, typ *a
return name, nil
}
func (g *generator) getTypeForInputType(opName string, typ *ast.Type) (string, error) {
func (g *generator) getTypeForInputType(opName string, typ *ast.Type, options, queryOptions *GenqlientDirective) (string, error) {
// Sort of a hack: case the input type name to match the op-name.
name := matchFirst(typ.Name(), opName)
// TODO: we have to pass name 4 times, yuck
builder := &typeBuilder{typeName: name, typeNamePrefix: name, generator: g}
err := builder.writeType(name, name, typ, selectionsForType(g, typ))
// TODO: passing options is actually kinda wrong, because it means we could
// break the "there is only Go type for each input type" rule. In practice
// it's probably rare that you use the same input type twice in a query and
// want different settings, though, and it just means we choose one or the
// other set of options.
// TODO: it's also awkward because you have no way to pass an option for an
// individual input-type field.
// TODO: should we use pointers by default for input-types if they're
// structs?
err := builder.writeType(name, name, typ, selectionsForType(g, typ, queryOptions), options)
return builder.String(), err
}
type field interface {
Alias() string
Options() (*GenqlientDirective, error)
Type() *ast.Type
SubFields() ([]field, error)
}
type outputField struct{ field *ast.Field }
type outputField struct {
*generator
queryOptions *GenqlientDirective
field *ast.Field
}
func (s outputField) Alias() string {
// gqlparser sets Alias even if the field is not aliased, see e.g.
@@ -147,6 +161,14 @@ func (s outputField) Alias() string {
return s.field.Alias
}
func (s outputField) Options() (*GenqlientDirective, error) {
_, directive, err := s.generator.parsePrecedingComment(s.field, s.field.Position)
if err != nil {
return nil, err
}
return s.queryOptions.merge(directive), nil
}
func (s outputField) Type() *ast.Type {
if s.field.Definition == nil {
return nil
@@ -155,15 +177,15 @@ func (s outputField) Type() *ast.Type {
}
func (s outputField) SubFields() ([]field, error) {
return selections(s.field.SelectionSet)
return selections(s.generator, s.field.SelectionSet, s.queryOptions)
}
func selections(selectionSet ast.SelectionSet) ([]field, error) {
func selections(g *generator, selectionSet ast.SelectionSet, options *GenqlientDirective) ([]field, error) {
retval := make([]field, len(selectionSet))
for i, selection := range selectionSet {
switch selection := selection.(type) {
case *ast.Field:
retval[i] = outputField{selection}
retval[i] = outputField{g, options, selection}
case *ast.FragmentSpread, *ast.InlineFragment:
return nil, fmt.Errorf("not implemented: %T", selection)
default:
@@ -175,21 +197,29 @@ func selections(selectionSet ast.SelectionSet) ([]field, error) {
type inputField struct {
*generator
field *ast.FieldDefinition
field *ast.FieldDefinition
queryOptions *GenqlientDirective
}
func (s inputField) Alias() string { return s.field.Name }
func (s inputField) Alias() string { return s.field.Name }
func (s inputField) Options() (*GenqlientDirective, error) {
_, directive, err := s.generator.parsePrecedingComment(s.field, s.field.Position)
if err != nil {
return nil, err
}
return s.queryOptions.merge(directive), nil
}
func (s inputField) Type() *ast.Type { return s.field.Type }
func (s inputField) SubFields() ([]field, error) {
return selectionsForType(s.generator, s.field.Type), nil
return selectionsForType(s.generator, s.field.Type, s.queryOptions), nil
}
func selectionsForType(g *generator, typ *ast.Type) []field {
func selectionsForType(g *generator, typ *ast.Type, queryOptions *GenqlientDirective) []field {
def := g.schema.Types[typ.Name()]
fields := make([]field, len(def.Fields))
for i, field := range def.Fields {
fields[i] = inputField{g, field}
fields[i] = inputField{g, field, queryOptions}
}
return fields
}
@@ -214,6 +244,11 @@ func (builder *typeBuilder) writeField(field field) error {
return err
}
options, err := field.Options()
if err != nil {
return err
}
err = builder.writeType(
// Note we don't deduplicate suffixes here -- if our prefix is GetUser
// and the field name is User, we do GetUserUser. This is important
@@ -224,7 +259,7 @@ func (builder *typeBuilder) writeField(field field) error {
// `query q { a: f { b }, c: f { d } }` we need separate types for a
// and c, even though they are the same type in GraphQL, because they
// have different fields.
builder.typeNamePrefix+upperFirst(field.Alias()), "", typ, fields)
builder.typeNamePrefix+upperFirst(field.Alias()), "", typ, fields, options)
if err != nil {
return err
}
@@ -239,7 +274,7 @@ func (builder *typeBuilder) writeField(field field) error {
return nil
}
func (builder *typeBuilder) writeType(namePrefix, nameOverride string, typ *ast.Type, fields []field) error {
func (builder *typeBuilder) writeType(namePrefix, nameOverride string, typ *ast.Type, fields []field, options *GenqlientDirective) error {
// gqlgen does slightly different things here, but its implementation may
// be useful to crib from:
// https://github.com/99designs/gqlgen/blob/master/plugin/modelgen/models.go#L113
@@ -248,13 +283,15 @@ func (builder *typeBuilder) writeType(namePrefix, nameOverride string, typ *ast.
builder.WriteString("[]")
typ = typ.Elem
}
// TODO: allow an option to make the Go type a pointer, if you want to do
// optionality that way, or perhaps others
// if !typ.NonNull { builder.WriteString("*") }
if options.GetPointer() {
// TODO: this does []*T, you might in principle want *[]T or
// *[]*T.
builder.WriteString("*")
}
def := builder.schema.Types[typ.Name()]
// Writes a typedef elsewhere (if not already defined)
name, err := builder.addTypeForDefinition(namePrefix, nameOverride, def, fields)
name, err := builder.addTypeForDefinition(namePrefix, nameOverride, def, fields, options)
if err != nil {
return err
}
@@ -263,7 +300,7 @@ func (builder *typeBuilder) writeType(namePrefix, nameOverride string, typ *ast.
return nil
}
func (builder *typeBuilder) writeTypedef(typedef *ast.Definition, fields []field) error {
func (builder *typeBuilder) writeTypedef(typedef *ast.Definition, fields []field, options *GenqlientDirective) error {
switch typedef.Kind {
case ast.Object, ast.InputObject:
builder.WriteString("struct {\n")
@@ -295,7 +332,7 @@ func (builder *typeBuilder) writeTypedef(typedef *ast.Definition, fields []field
// Then, write the implementations.
// TODO(benkraft): Put a doc-comment somewhere with the list.
for _, impldef := range builder.schema.GetPossibleTypes(typedef) {
name, err := builder.addTypeForDefinition(builder.typeNamePrefix, "", impldef, fields)
name, err := builder.addTypeForDefinition(builder.typeNamePrefix, "", impldef, fields, options)
if err != nil {
return err
}