diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d49a2be..5344d9c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -28,6 +28,7 @@ When releasing a new version: ### New features: - The new `bindings.marshaler` and `bindings.unmarshaler` options in `genqlient.yaml` allow binding to a type without using its standard JSON serialization; see the [documentation](genqlient.yaml) for details. +- Multiple genqlient directives may now be applied to the same node, as long as they don't conflict; see the [directive documentation](genqlient_directive.graphql) for details. ### Bug fixes: diff --git a/docs/genqlient_directive.graphql b/docs/genqlient_directive.graphql index 615a66e..d00a8fe 100644 --- a/docs/genqlient_directive.graphql +++ b/docs/genqlient_directive.graphql @@ -21,15 +21,25 @@ # # @genqlient(n: "c") # query MyQuery(arg1: String, # # @genqlient(n: "d") -# arg2: String, arg3: String, +# arg2: String, arg3: MyInput, # arg4: String, # ) { # # @genqlient(n: "e") # field1, field2 -# field3 +# # @genqlient(n: "f") +# field3 { +# field4 +# } # } # the directive "a" is ignored, "b" and "c" apply to all relevant nodes in the -# query, "d" applies to arg2 and arg3, and "e" applies to field1 and field2. +# query, "d" applies to arg2 and arg3, "e" applies to field1 and field2, and +# "f" applies to field3. +# +# Except as noted below, directives on nodes take precedence over ones on the +# entire query (so "d", "e", and "f" take precedence over "b" and "c"), and +# multiple directives on the same node ("b" and "c") must not conflict. Note +# that directives on nodes do *not* apply to their "children", so "d" does not +# apply to the fields of MyInput, and "f" does not apply to field4. directive genqlient( # If set, this argument will be omitted if it has an empty value, defined @@ -125,7 +135,9 @@ directive genqlient( # down to all child fields (which would cause conflicts). typename: String -) on +# Multiple genqlient directives are allowed in the same location, as long as +# they don't have conflicting options. +) repeatable on # genqlient directives can go almost anywhere, although some options are only # applicable in certain locations as described above. | QUERY diff --git a/generate/convert.go b/generate/convert.go index 1084e10..889d18e 100644 --- a/generate/convert.go +++ b/generate/convert.go @@ -290,7 +290,7 @@ func (g *generator) convertDefinition( if options.TypeName != "" { // If the user specified a name, use it! name = options.TypeName - if namePrefix.head == name && namePrefix.tail == nil { + if namePrefix != nil && namePrefix.head == name && namePrefix.tail == nil { // Special case: if this name is also the only component of the // name-prefix, append the type-name anyway. This happens when you // assign a type name to an interface type, and we are generating @@ -367,10 +367,12 @@ func (g *generator) convertDefinition( for i, field := range def.Fields { goName := upperFirst(field.Name) - // Several of the arguments don't really make sense here + // There are no field-specific options for inputs (yet, see #14), + // but we still need to merge with an empty directive to clear out + // any query-options that shouldn't apply here (namely "typename"). + fieldOptions := queryOptions.merge(newGenqlientDirective(pos)) + // Several of the arguments don't really make sense here: // (note field.Type is necessarily a scalar, input, or enum) - // - no field-specific options can apply, because this is - // a field in the type, not in the query (see also #14). // - namePrefix is ignored for input types and enums (see // names.go) and for scalars (they use client-specified // names) @@ -381,7 +383,7 @@ func (g *generator) convertDefinition( // will be ignored? We know field.Type is a scalar, enum, or input // type. But plumbing that is a bit tricky in practice. fieldGoType, err := g.convertType( - namePrefix, field.Type, nil, queryOptions, queryOptions) + namePrefix, field.Type, nil, fieldOptions, queryOptions) if err != nil { return nil, err } diff --git a/generate/genqlient_directive.go b/generate/genqlient_directive.go index ca00aad..15ed1c8 100644 --- a/generate/genqlient_directive.go +++ b/generate/genqlient_directive.go @@ -19,66 +19,84 @@ type genqlientDirective struct { TypeName string } +func newGenqlientDirective(pos *ast.Position) *genqlientDirective { + return &genqlientDirective{ + pos: pos, + } +} + func (dir *genqlientDirective) GetOmitempty() bool { return dir.Omitempty != nil && *dir.Omitempty } func (dir *genqlientDirective) GetPointer() bool { return dir.Pointer != nil && *dir.Pointer } func (dir *genqlientDirective) GetStruct() bool { return dir.Struct != nil && *dir.Struct } -func setBool(dst **bool, v *ast.Value) error { +func setBool(optionName string, dst **bool, v *ast.Value, pos *ast.Position) error { + if *dst != nil { + return errorf(pos, "conflicting values for %v", optionName) + } ei, err := v.Value(nil) // no vars allowed if err != nil { - return errorf(v.Position, "invalid boolean value %v: %v", v, err) + return errorf(pos, "invalid boolean value %v: %v", v, err) } if b, ok := ei.(bool); ok { *dst = &b return nil } - return errorf(v.Position, "expected boolean, got non-boolean value %T(%v)", ei, ei) + return errorf(pos, "expected boolean, got non-boolean value %T(%v)", ei, ei) } -func setString(dst *string, v *ast.Value) error { +func setString(optionName string, dst *string, v *ast.Value, pos *ast.Position) error { + if *dst != "" { + return errorf(pos, "conflicting values for %v", optionName) + } ei, err := v.Value(nil) // no vars allowed if err != nil { - return errorf(v.Position, "invalid string value %v: %v", v, err) + return errorf(pos, "invalid string value %v: %v", v, err) } if b, ok := ei.(string); ok { *dst = b return nil } - return errorf(v.Position, "expected string, got non-string value %T(%v)", ei, ei) + return errorf(pos, "expected string, got non-string value %T(%v)", ei, ei) } -func fromGraphQL(dir *ast.Directive, pos *ast.Position) (*genqlientDirective, error) { - if dir.Name != "genqlient" { +// add adds to this genqlientDirective struct the settings from then given +// GraphQL directive. +// +// If there are multiple genqlient directives are applied to the same node, +// e.g. +// # @genqlient(...) +// # @genqlient(...) +// add will be called several times. In this case, conflicts between the +// options are an error. +func (dir *genqlientDirective) add(graphQLDirective *ast.Directive, pos *ast.Position) error { + if graphQLDirective.Name != "genqlient" { // Actually we just won't get here; we only get here if the line starts // with "# @genqlient", unless there's some sort of bug. - return nil, errorf(pos, "the only valid comment-directive is @genqlient, got %v", dir.Name) + return errorf(pos, "the only valid comment-directive is @genqlient, got %v", graphQLDirective.Name) } - var retval genqlientDirective - retval.pos = pos - var err error - for _, arg := range dir.Arguments { + for _, arg := range graphQLDirective.Arguments { switch arg.Name { - // TODO: reflect and struct tags? + // TODO(benkraft): Use reflect and struct tags? case "omitempty": - err = setBool(&retval.Omitempty, arg.Value) + err = setBool("omitempty", &dir.Omitempty, arg.Value, pos) case "pointer": - err = setBool(&retval.Pointer, arg.Value) + err = setBool("pointer", &dir.Pointer, arg.Value, pos) case "struct": - err = setBool(&retval.Struct, arg.Value) + err = setBool("struct", &dir.Struct, arg.Value, pos) case "bind": - err = setString(&retval.Bind, arg.Value) + err = setString("bind", &dir.Bind, arg.Value, pos) case "typename": - err = setString(&retval.TypeName, arg.Value) + err = setString("typename", &dir.TypeName, arg.Value, pos) default: - return nil, errorf(pos, "unknown argument %v for @genqlient", arg.Name) + return errorf(pos, "unknown argument %v for @genqlient", arg.Name) } if err != nil { - return nil, err + return err } } - return &retval, nil + return nil } func (dir *genqlientDirective) validate(node interface{}, schema *ast.Schema) error { @@ -185,11 +203,16 @@ func (dir *genqlientDirective) merge(other *genqlientDirective) *genqlientDirect return &retval } +// parsePrecedingComment looks at the comment right before this node, and +// returns the genqlient directive applied to it (or an empty one if there is +// none), the remaining human-readable comment (or "" if there is none), and an +// error if the directive is invalid. func (g *generator) parsePrecedingComment( node interface{}, pos *ast.Position, ) (comment string, directive *genqlientDirective, err error) { - directive = new(genqlientDirective) + directive = newGenqlientDirective(pos) + hasDirective := false if pos == nil || pos.Src == nil { // node was added by genqlient itself return "", directive, nil // treated as if there were no comment } @@ -200,19 +223,16 @@ func (g *generator) parsePrecedingComment( line := strings.TrimSpace(sourceLines[i-1]) trimmed := strings.TrimSpace(strings.TrimPrefix(line, "#")) if strings.HasPrefix(line, "# @genqlient") { - graphQLDirective, err := parseDirective(trimmed, pos) + hasDirective = true + var graphQLDirective *ast.Directive + graphQLDirective, err = parseDirective(trimmed, pos) if err != nil { return "", nil, err } - genqlientDirective, err := fromGraphQL(graphQLDirective, pos) + err = directive.add(graphQLDirective, pos) if err != nil { return "", nil, err } - err = genqlientDirective.validate(node, g.schema) - if err != nil { - return "", nil, err - } - directive = directive.merge(genqlientDirective) } else if strings.HasPrefix(line, "#") { commentLines = append(commentLines, trimmed) } else { @@ -220,6 +240,13 @@ func (g *generator) parsePrecedingComment( } } + if hasDirective { // (else directive is empty) + err = directive.validate(node, g.schema) + if err != nil { + return "", nil, err + } + } + reverse(commentLines) return strings.TrimSpace(strings.Join(commentLines, "\n")), directive, nil diff --git a/generate/testdata/errors/ConflictingDirectiveArguments.graphql b/generate/testdata/errors/ConflictingDirectiveArguments.graphql new file mode 100644 index 0000000..28f9976 --- /dev/null +++ b/generate/testdata/errors/ConflictingDirectiveArguments.graphql @@ -0,0 +1,2 @@ +# @genqlient(pointer: true, pointer: false) +query ConflictingDirectiveArguments { f } diff --git a/generate/testdata/errors/ConflictingDirectiveArguments.schema.graphql b/generate/testdata/errors/ConflictingDirectiveArguments.schema.graphql new file mode 100644 index 0000000..314f707 --- /dev/null +++ b/generate/testdata/errors/ConflictingDirectiveArguments.schema.graphql @@ -0,0 +1,3 @@ +type Query { + f: String +} diff --git a/generate/testdata/errors/ConflictingDirectives.graphql b/generate/testdata/errors/ConflictingDirectives.graphql new file mode 100644 index 0000000..762da3a --- /dev/null +++ b/generate/testdata/errors/ConflictingDirectives.graphql @@ -0,0 +1,3 @@ +# @genqlient(pointer: true) +# @genqlient(pointer: false) +query ConflictingDirectives { f } diff --git a/generate/testdata/errors/ConflictingDirectives.schema.graphql b/generate/testdata/errors/ConflictingDirectives.schema.graphql new file mode 100644 index 0000000..314f707 --- /dev/null +++ b/generate/testdata/errors/ConflictingDirectives.schema.graphql @@ -0,0 +1,3 @@ +type Query { + f: String +} diff --git a/generate/testdata/queries/MultipleDirectives.graphql b/generate/testdata/queries/MultipleDirectives.graphql new file mode 100644 index 0000000..ac38e0e --- /dev/null +++ b/generate/testdata/queries/MultipleDirectives.graphql @@ -0,0 +1,12 @@ +# @genqlient(typename: "MyMultipleDirectivesResponse") +# @genqlient(omitempty: true) +# @genqlient(pointer: true) +query MultipleDirectives( + # @genqlient(pointer: false) + # @genqlient(typename: "MyInput") + $query: UserQueryInput, + $queries: [UserQueryInput], +) { + user(query: $query) { id } + users(query: $queries) { id } +} diff --git a/generate/testdata/snapshots/TestGenerate-MultipleDirectives.graphql-MultipleDirectives.graphql.go b/generate/testdata/snapshots/TestGenerate-MultipleDirectives.graphql-MultipleDirectives.graphql.go new file mode 100644 index 0000000..6f76b1b --- /dev/null +++ b/generate/testdata/snapshots/TestGenerate-MultipleDirectives.graphql-MultipleDirectives.graphql.go @@ -0,0 +1,178 @@ +package test + +// Code generated by github.com/Khan/genqlient, DO NOT EDIT. + +import ( + "encoding/json" + "fmt" + "time" + + "github.com/Khan/genqlient/graphql" + "github.com/Khan/genqlient/internal/testutil" +) + +// UserQueryInput is the argument to Query.users. +// +// Ideally this would support anything and everything! +// Or maybe ideally it wouldn't. +// Really I'm just talking to make this documentation longer. +type MyInput struct { + Email *string `json:"email"` + Name *string `json:"name"` + // id looks the user up by ID. It's a great way to look up users. + Id *testutil.ID `json:"id"` + Role *Role `json:"role"` + Names []*string `json:"names"` + HasPokemon *testutil.Pokemon `json:"hasPokemon"` + Birthdate *time.Time `json:"-"` +} + +func (v *MyInput) MarshalJSON() ([]byte, error) { + + var fullObject struct { + *MyInput + Birthdate json.RawMessage `json:"birthdate"` + graphql.NoUnmarshalJSON + } + fullObject.MyInput = v + + { + + dst := &fullObject.Birthdate + src := v.Birthdate + var err error + *dst, err = testutil.MarshalDate( + src) + if err != nil { + return nil, fmt.Errorf( + "Unable to marshal MyInput.Birthdate: %w", err) + } + } + + return json.Marshal(&fullObject) +} + +// MyMultipleDirectivesResponse is returned by MultipleDirectives on success. +type MyMultipleDirectivesResponse struct { + // user looks up a user by some stuff. + // + // See UserQueryInput for what stuff is supported. + // If query is null, returns the current user. + User *MyMultipleDirectivesResponseUser `json:"user"` + Users []*MyMultipleDirectivesResponseUsersUser `json:"users"` +} + +// MyMultipleDirectivesResponseUser includes the requested fields of the GraphQL type User. +// The GraphQL type's documentation follows. +// +// A User is a user! +type MyMultipleDirectivesResponseUser struct { + // id is the user's ID. + // + // It is stable, unique, and opaque, like all good IDs. + Id *testutil.ID `json:"id"` +} + +// MyMultipleDirectivesResponseUsersUser includes the requested fields of the GraphQL type User. +// The GraphQL type's documentation follows. +// +// A User is a user! +type MyMultipleDirectivesResponseUsersUser struct { + // id is the user's ID. + // + // It is stable, unique, and opaque, like all good IDs. + Id *testutil.ID `json:"id"` +} + +// Role is a type a user may have. +type Role string + +const ( + // What is a student? + // + // A student is primarily a person enrolled in a school or other educational institution and who is under learning with goals of acquiring knowledge, developing professions and achieving employment at desired field. In the broader sense, a student is anyone who applies themselves to the intensive intellectual engagement with some matter necessary to master it as part of some practical affair in which such mastery is basic or decisive. + // + // (from [Wikipedia](https://en.wikipedia.org/wiki/Student)) + RoleStudent Role = "STUDENT" + // Teacher is a teacher, who teaches the students. + RoleTeacher Role = "TEACHER" +) + +// UserQueryInput is the argument to Query.users. +// +// Ideally this would support anything and everything! +// Or maybe ideally it wouldn't. +// Really I'm just talking to make this documentation longer. +type UserQueryInput struct { + Email *string `json:"email"` + Name *string `json:"name"` + // id looks the user up by ID. It's a great way to look up users. + Id *testutil.ID `json:"id"` + Role *Role `json:"role"` + Names []*string `json:"names"` + HasPokemon *testutil.Pokemon `json:"hasPokemon"` + Birthdate *time.Time `json:"-"` +} + +func (v *UserQueryInput) MarshalJSON() ([]byte, error) { + + var fullObject struct { + *UserQueryInput + Birthdate json.RawMessage `json:"birthdate"` + graphql.NoUnmarshalJSON + } + fullObject.UserQueryInput = v + + { + + dst := &fullObject.Birthdate + src := v.Birthdate + var err error + *dst, err = testutil.MarshalDate( + src) + if err != nil { + return nil, fmt.Errorf( + "Unable to marshal UserQueryInput.Birthdate: %w", err) + } + } + + return json.Marshal(&fullObject) +} + +// __MultipleDirectivesInput is used internally by genqlient +type __MultipleDirectivesInput struct { + Query MyInput `json:"query,omitempty"` + Queries []*UserQueryInput `json:"queries,omitempty"` +} + +func MultipleDirectives( + client graphql.Client, + query MyInput, + queries []*UserQueryInput, +) (*MyMultipleDirectivesResponse, error) { + __input := __MultipleDirectivesInput{ + Query: query, + Queries: queries, + } + var err error + + var retval MyMultipleDirectivesResponse + err = client.MakeRequest( + nil, + "MultipleDirectives", + ` +query MultipleDirectives ($query: UserQueryInput, $queries: [UserQueryInput]) { + user(query: $query) { + id + } + users(query: $queries) { + id + } +} +`, + &retval, + &__input, + ) + return &retval, err +} + diff --git a/generate/testdata/snapshots/TestGenerate-MultipleDirectives.graphql-MultipleDirectives.graphql.json b/generate/testdata/snapshots/TestGenerate-MultipleDirectives.graphql-MultipleDirectives.graphql.json new file mode 100644 index 0000000..7e6bbf3 --- /dev/null +++ b/generate/testdata/snapshots/TestGenerate-MultipleDirectives.graphql-MultipleDirectives.graphql.json @@ -0,0 +1,9 @@ +{ + "operations": [ + { + "operationName": "MultipleDirectives", + "query": "\nquery MultipleDirectives ($query: UserQueryInput, $queries: [UserQueryInput]) {\n\tuser(query: $query) {\n\t\tid\n\t}\n\tusers(query: $queries) {\n\t\tid\n\t}\n}\n", + "sourceLocation": "testdata/queries/MultipleDirectives.graphql" + } + ] +} diff --git a/generate/testdata/snapshots/TestGenerateErrors-ConflictingDirectiveArguments-graphql b/generate/testdata/snapshots/TestGenerateErrors-ConflictingDirectiveArguments-graphql new file mode 100644 index 0000000..a3cce72 --- /dev/null +++ b/generate/testdata/snapshots/TestGenerateErrors-ConflictingDirectiveArguments-graphql @@ -0,0 +1 @@ +testdata/errors/ConflictingDirectiveArguments.graphql:2: conflicting values for pointer diff --git a/generate/testdata/snapshots/TestGenerateErrors-ConflictingDirectives-graphql b/generate/testdata/snapshots/TestGenerateErrors-ConflictingDirectives-graphql new file mode 100644 index 0000000..f838c05 --- /dev/null +++ b/generate/testdata/snapshots/TestGenerateErrors-ConflictingDirectives-graphql @@ -0,0 +1 @@ +testdata/errors/ConflictingDirectives.graphql:3: conflicting values for pointer