Skip to content

Commit

Permalink
Add a new option to treat an interface like an object (#97)
Browse files Browse the repository at this point in the history
## Summary:
The basic idea here is if you only request interface fields (no
fragments) you may not care about the concrete type, and so we could
just generate a struct as if it were an object.  I don't think it's a
good idea to do that by default, because then if you later add a
fragment all your code totally changes, but it's quite reasonable as an
option!

Most of the code involved is just wiring and validation; the
core implementation is literally just: treat it like an object.

Issue: #85

## Test plan:
make check


Author: benjaminjkraft

Reviewers: csilvers, StevenACoffman, benjaminjkraft, aberkan, dnerdy, jvoll, mahtabsabet, MiguelCastillo

Required Reviewers: 

Approved By: StevenACoffman

Checks: ⌛ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Lint, ⌛ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Lint

Pull Request URL: #97
  • Loading branch information
benjaminjkraft authored Sep 16, 2021
1 parent 463cffd commit 5211442
Show file tree
Hide file tree
Showing 14 changed files with 477 additions and 9 deletions.
22 changes: 22 additions & 0 deletions docs/FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,28 @@ if novel, ok := resp.Favorite.(*GetBooksFavoriteNovel); ok {

The interface-type's GoDoc will include a list of its implementations, for your convenience.

If you only want to request shared fields of the interface (i.e. no fragments), this may seem like a lot of ceremony. If you prefer, you can instead add `# @genqlient(struct: true)` to the field, and genqlient will just generate a struct, like it does for GraphQL object types. For example, given:

```graphql
query GetBooks {
# @genqlient(struct: true)
favorite {
title
}
}
```

genqlient will generate just:

```go
type GetBooksFavoriteBook struct {
Title string
}
```

Keep in mind that if you later want to add fragments to your selection, you won't be able to use `struct` anymore; when you remove it you may need to update your code to replace `.Title` with `.GetTitle()` and so on.


### … documentation on the output types?

For any GraphQL types or fields with documentation in the GraphQL schema, genqlient automatically includes that documentation in the generated code's GoDoc. To add additional information to genqlient entrypoints, you can put comments in the GraphQL source:
Expand Down
16 changes: 16 additions & 0 deletions docs/genqlient_directive.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,22 @@ directive genqlient(
# zero value and null (for nullable fields).
pointer: Boolean

# If set, this field will use a struct type in Go, even if it's an interface.
#
# This is useful when you have a query like
# query MyQuery {
# myInterface { myField }
# }
# where you are requesting only shared fields of an interface. By default,
# genqlient still generates an interface type, for consistency. But this
# isn't necessary: a struct would do just fine since there are no
# type-specific fields. Setting `struct: true` tells genqlient to do that.
#
# Note that this is only allowed when there are no fragments in play, such
# that all fields are on the interface type. Note that if you later add a
# fragment, you'll have to remove this option, and the types will change.
struct: Boolean

# If set, this argument or field will use the given Go type instead of a
# genqlient-generated type.
#
Expand Down
11 changes: 9 additions & 2 deletions generate/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,14 @@ func (g *generator) convertDefinition(
GraphQLName: def.Name,
}

switch def.Kind {
// The struct option basically means "treat this as if it were an object".
// (It only applies if valid; this is important if you said the whole
// query should have `struct: true`.)
kind := def.Kind
if options.GetStruct() && validateStructOption(def, selectionSet, pos) == nil {
kind = ast.Object
}
switch kind {
case ast.Object:
name := makeTypeName(namePrefix, def.Name)

Expand Down Expand Up @@ -463,7 +470,7 @@ func (g *generator) convertInlineFragment(
containingTypedef *ast.Definition,
queryOptions *genqlientDirective,
) ([]*goStructField, error) {
// You might think fragmentTypedef would be a fragment.ObjectDefinition, but
// You might think fragmentTypedef is just fragment.ObjectDefinition, but
// actually that's the type into which the fragment is spread.
fragmentTypedef := g.schema.Types[fragment.TypeCondition]
if !fragmentMatches(containingTypedef, fragmentTypedef) {
Expand Down
2 changes: 2 additions & 0 deletions generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ func (g *generator) preprocessQueryDocument(doc *ast.QueryDocument) {
// needed). Note this does mean abstract-typed fragments spread into
// object-typed scope will *not* have access to `__typename`, but they
// indeed don't need it, since we do know the type in that context.
// TODO(benkraft): We should omit __typename if you asked for
// `# @genqlient(struct: true)`.
observers.OnField(func(_ *validator.Walker, field *ast.Field) {
// We are interested in a field from the query like
// field { subField ... }
Expand Down
66 changes: 59 additions & 7 deletions generate/genqlient_directive.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ type genqlientDirective struct {
pos *ast.Position
Omitempty *bool
Pointer *bool
Struct *bool
Bind string
}

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 {
ei, err := v.Value(nil) // no vars allowed
Expand All @@ -44,15 +46,15 @@ func setString(dst *string, v *ast.Value) error {
return errorf(v.Position, "expected string, got non-string value %T(%v)", ei, ei)
}

func fromGraphQL(dir *ast.Directive) (*genqlientDirective, error) {
func fromGraphQL(dir *ast.Directive, pos *ast.Position) (*genqlientDirective, error) {
if dir.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(dir.Position, "the only valid comment-directive is @genqlient, got %v", dir.Name)
return nil, errorf(pos, "the only valid comment-directive is @genqlient, got %v", dir.Name)
}

var retval genqlientDirective
retval.pos = dir.Position
retval.pos = pos

var err error
for _, arg := range dir.Arguments {
Expand All @@ -62,10 +64,12 @@ func fromGraphQL(dir *ast.Directive) (*genqlientDirective, error) {
err = setBool(&retval.Omitempty, arg.Value)
case "pointer":
err = setBool(&retval.Pointer, arg.Value)
case "struct":
err = setBool(&retval.Struct, arg.Value)
case "bind":
err = setString(&retval.Bind, arg.Value)
default:
return nil, errorf(arg.Position, "unknown argument %v for @genqlient", arg.Name)
return nil, errorf(pos, "unknown argument %v for @genqlient", arg.Name)
}
if err != nil {
return nil, err
Expand All @@ -74,7 +78,7 @@ func fromGraphQL(dir *ast.Directive) (*genqlientDirective, error) {
return &retval, nil
}

func (dir *genqlientDirective) validate(node interface{}) error {
func (dir *genqlientDirective) validate(node interface{}, schema *ast.Schema) error {
switch node := node.(type) {
case *ast.OperationDefinition:
if dir.Bind != "" {
Expand All @@ -90,24 +94,69 @@ func (dir *genqlientDirective) validate(node interface{}) error {
return errorf(dir.pos, "bind is not implemented for named fragments")
}

if dir.Struct != nil {
return errorf(dir.pos, "struct is only applicable to fields")
}

// Like operations, anything else will just apply to the entire
// fragment.
return nil
case *ast.VariableDefinition:
if dir.Omitempty != nil && node.Type.NonNull {
return errorf(dir.pos, "omitempty may only be used on optional arguments")
}

if dir.Struct != nil {
return errorf(dir.pos, "struct is only applicable to fields")
}

return nil
case *ast.Field:
if dir.Omitempty != nil {
return errorf(dir.pos, "omitempty is not applicable to fields")
}

if dir.Struct != nil {
typ := schema.Types[node.Definition.Type.Name()]
if err := validateStructOption(typ, node.SelectionSet, dir.pos); err != nil {
return err
}
}

return nil
default:
return errorf(dir.pos, "invalid @genqlient directive location: %T", node)
}
}

func validateStructOption(
typ *ast.Definition,
selectionSet ast.SelectionSet,
pos *ast.Position,
) error {
if typ.Kind != ast.Interface && typ.Kind != ast.Union {
return errorf(pos, "struct is only applicable to interface-typed fields")
}

// Make sure that all the requested fields apply to the interface itself
// (not just certain implementations).
for _, selection := range selectionSet {
switch selection.(type) {
case *ast.Field:
// fields are fine.
case *ast.InlineFragment, *ast.FragmentSpread:
// Fragments aren't allowed. In principle we could allow them under
// the condition that the fragment applies to the whole interface
// (not just one implementation; and so on recursively), and for
// fragment spreads additionally that the fragment has the same
// option applied to it, but it seems more trouble than it's worth
// right now.
return errorf(pos, "struct is not allowed for types with fragments")
}
}
return nil
}

func (dir *genqlientDirective) merge(other *genqlientDirective) *genqlientDirective {
retval := *dir
if other.Omitempty != nil {
Expand All @@ -116,6 +165,9 @@ func (dir *genqlientDirective) merge(other *genqlientDirective) *genqlientDirect
if other.Pointer != nil {
retval.Pointer = other.Pointer
}
if other.Struct != nil {
retval.Struct = other.Struct
}
if other.Bind != "" {
retval.Bind = other.Bind
}
Expand All @@ -141,11 +193,11 @@ func (g *generator) parsePrecedingComment(
if err != nil {
return "", nil, err
}
genqlientDirective, err := fromGraphQL(graphQLDirective)
genqlientDirective, err := fromGraphQL(graphQLDirective, pos)
if err != nil {
return "", nil, err
}
err = genqlientDirective.validate(node)
err = genqlientDirective.validate(node, g.schema)
if err != nil {
return "", nil, err
}
Expand Down
6 changes: 6 additions & 0 deletions generate/testdata/errors/StructOptionOnObject.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
query StructOptionOnObject {
# @genqlient(struct: true)
myObject {
f
}
}
7 changes: 7 additions & 0 deletions generate/testdata/errors/StructOptionOnObject.schema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
type Query {
myObject: MyObject
}

type MyObject {
f: String!
}
9 changes: 9 additions & 0 deletions generate/testdata/errors/StructOptionWithFragments.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
query StructOptionOnObject {
# @genqlient(struct: true)
myInterface {
f
... on MyObject {
g
}
}
}
16 changes: 16 additions & 0 deletions generate/testdata/errors/StructOptionWithFragments.schema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
type Query {
myInterface: MyInterface
}

interface MyInterface {
f: String!
}

type MyObject implements MyInterface {
f: String!
g: String!
}

type OtherObject implements MyInterface {
f: String!
}
24 changes: 24 additions & 0 deletions generate/testdata/queries/StructOption.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
fragment VideoFields on Video { duration }

# @genqlient(struct: true)
query StructOption {
root {
id
children {
id
parent {
id
children {
id
}
# (it won't apply to this)
interfaceChildren: children {
id
...VideoFields
}
}
}
}
# (nor this)
user { roles }
}
Loading

0 comments on commit 5211442

Please sign in to comment.