Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for abstract-typed named fragments #79

Merged
merged 2 commits into from
Sep 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
go: [ '1.13', '1.14', '1.15', '1.16', '1.17' ]
go: [ '1.14', '1.15', '1.16', '1.17' ]

steps:
- name: Set up Go
Expand Down
151 changes: 113 additions & 38 deletions generate/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ func (g *generator) convertOperation(

goType := &goStructType{
GoName: name,
Description: fmt.Sprintf(
"%v is returned by %v on success.", name, operation.Name),
GraphQLName: baseType.Name,
Fields: fields,
Incomplete: false,
descriptionInfo: descriptionInfo{
CommentOverride: fmt.Sprintf(
"%v is returned by %v on success.", name, operation.Name),
GraphQLName: baseType.Name,
// omit the GraphQL description for baseType; it's uninteresting.
},
Fields: fields,
}
g.typeMap[name] = goType

Expand Down Expand Up @@ -167,6 +169,12 @@ func (g *generator) convertDefinition(
return &goOpaqueType{goBuiltinName}, nil
}

desc := descriptionInfo{
// TODO(benkraft): Copy any comment above this selection-set?
GraphQLDescription: def.Description,
GraphQLName: def.Name,
}

switch def.Kind {
case ast.Object:
name := makeTypeName(namePrefix, def.Name)
Expand All @@ -178,11 +186,9 @@ func (g *generator) convertDefinition(
}

goType := &goStructType{
GoName: name,
Description: def.Description,
GraphQLName: def.Name,
Fields: fields,
Incomplete: true,
GoName: name,
Fields: fields,
descriptionInfo: desc,
}
g.typeMap[name] = goType
return goType, nil
Expand All @@ -195,10 +201,10 @@ func (g *generator) convertDefinition(
name := upperFirst(def.Name)

goType := &goStructType{
GoName: name,
Description: def.Description,
GraphQLName: def.Name,
Fields: make([]*goStructField, len(def.Fields)),
GoName: name,
Fields: make([]*goStructField, len(def.Fields)),
descriptionInfo: desc,
IsInput: true,
}
g.typeMap[name] = goType

Expand Down Expand Up @@ -240,10 +246,9 @@ func (g *generator) convertDefinition(
implementationTypes := g.schema.GetPossibleTypes(def)
goType := &goInterfaceType{
GoName: name,
Description: def.Description,
GraphQLName: def.Name,
SharedFields: sharedFields,
Implementations: make([]*goStructType, len(implementationTypes)),
descriptionInfo: desc,
}
g.typeMap[name] = goType

Expand Down Expand Up @@ -354,14 +359,31 @@ func (g *generator) convertSelectionSet(
// { id, id, id, ... on SubType { id } }
// (which, yes, is legal) we'll treat that as just { id }.
uniqFields := make([]*goStructField, 0, len(selectionSet))
fragmentNames := make(map[string]bool, len(selectionSet))
fieldNames := make(map[string]bool, len(selectionSet))
for _, field := range fields {
// If you embed a field twice via a named fragment, we keep both, even
// if there are complicated overlaps, since they are separate types to
// us. (See also the special handling for IsEmbedded in
// unmarshal.go.tmpl.)
//
// But if you spread the samenamed fragment twice, e.g.
// { ...MyFragment, ... on SubType { ...MyFragment } }
// we'll still deduplicate that.
if field.JSONName == "" {
name := field.GoType.Reference()
if fragmentNames[name] {
continue
}
uniqFields = append(uniqFields, field)
fragmentNames[name] = true
continue
}

// GraphQL (and, effectively, JSON) requires that all fields with the
// same alias (JSON-name) must be the same (i.e. refer to the same
// field), so that's how we deduplicate.
// It's fine to have duplicate embeds (i.e. via named fragments), even
// ones with complicated overlaps, since they are separate types to us.
if field.JSONName != "" && fieldNames[field.JSONName] {
if fieldNames[field.JSONName] {
// GraphQL (and, effectively, JSON) forbids you from having two
// fields with the same alias (JSON-name) that refer to different
// GraphQL fields. But it does allow you to have the same field
Expand Down Expand Up @@ -475,46 +497,99 @@ func (g *generator) convertFragmentSpread(
}
}

iface, ok := typ.(*goInterfaceType)
if ok && containingTypedef.Kind == ast.Object {
// If the containing type is concrete, and the fragment spread is
// abstract, refer directly to the appropriate implementation, to save
// the caller having to do type-assertions that will always succeed.
//
// That is, if you do
// fragment F on I { ... }
// query Q { a { ...F } }
// for the fragment we generate
// type F interface { ... }
// type FA struct { ... }
// // (other implementations)
// when you spread F into a context of type A, we embed FA, not F.
for _, impl := range iface.Implementations {
if impl.GraphQLName == containingTypedef.Name {
typ = impl
}
}
}

return &goStructField{GoName: "" /* i.e. embedded */, GoType: typ}, nil
}

// convertNamedFragment converts a single GraphQL named fragment-definition
// (`fragment MyFragment on MyType { ... }`) into a Go struct.
func (g *generator) convertNamedFragment(fragment *ast.FragmentDefinition) (goType, error) {
typ := g.schema.Types[fragment.TypeCondition]
if !g.Config.AllowBrokenFeatures &&
(typ.Kind == ast.Interface || typ.Kind == ast.Union) {
return nil, errorf(fragment.Position, "not implemented: abstract-typed fragments")
}

description, directive, err := g.parsePrecedingComment(fragment, fragment.Position)
comment, directive, err := g.parsePrecedingComment(fragment, fragment.Position)
if err != nil {
return nil, err
}

// If the user included a comment, use that. Else make up something
// generic; there's not much to say though.
if description == "" {
description = fmt.Sprintf(
"%v includes the GraphQL fields of %v requested by the fragment %v.",
fragment.Name, fragment.TypeCondition, fragment.Name)
desc := descriptionInfo{
CommentOverride: comment,
GraphQLName: typ.Name,
GraphQLDescription: typ.Description,
FragmentName: fragment.Name,
}

// The rest basically follows how we convert a definition, except that
// things like type-names are a bit different.

fields, err := g.convertSelectionSet(
newPrefixList(fragment.Name), fragment.SelectionSet, typ, directive)
if err != nil {
return nil, err
}

goType := &goStructType{
GoName: fragment.Name,
Description: description,
GraphQLName: fragment.TypeCondition,
Fields: fields,
Incomplete: false,
switch typ.Kind {
case ast.Object:
goType := &goStructType{
GoName: fragment.Name,
Fields: fields,
descriptionInfo: desc,
}
g.typeMap[fragment.Name] = goType
return goType, nil
case ast.Interface, ast.Union:
implementationTypes := g.schema.GetPossibleTypes(typ)
goType := &goInterfaceType{
GoName: fragment.Name,
SharedFields: fields,
Implementations: make([]*goStructType, len(implementationTypes)),
descriptionInfo: desc,
}
g.typeMap[fragment.Name] = goType

for i, implDef := range implementationTypes {
implFields, err := g.convertSelectionSet(
newPrefixList(fragment.Name), fragment.SelectionSet, implDef, directive)
if err != nil {
return nil, err
}

implDesc := desc
implDesc.GraphQLName = implDef.Name

implTyp := &goStructType{
GoName: fragment.Name + upperFirst(implDef.Name),
Fields: implFields,
descriptionInfo: implDesc,
}
goType.Implementations[i] = implTyp
g.typeMap[implTyp.GoName] = implTyp
}

return goType, nil
default:
return nil, errorf(fragment.Position, "invalid type for fragment: %v is a %v",
fragment.TypeCondition, typ.Kind)
}
g.typeMap[fragment.Name] = goType
return goType, nil
}

// convertField converts a single GraphQL operation-field into a Go
Expand Down
81 changes: 81 additions & 0 deletions generate/description.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package generate

// Code relating to generating GoDoc from GraphQL descriptions.
//
// For fields, and types where we just copy the "whole" type (enum and
// input-object), this is easy: we just use the GraphQL description. But for
// struct types, there are often more useful things we can say.

import (
"fmt"
"strings"
)

// descriptionInfo is embedded in types whose descriptions may be more complex
// than just a copy of the GraphQL doc.
type descriptionInfo struct {
// user-specified comment for this type
CommentOverride string
// name of the corresponding GraphQL type
GraphQLName string
// GraphQL description of the type .GraphQLName, if any
GraphQLDescription string
// name of the corresponding GraphQL fragment (on .GraphQLName), if any
FragmentName string
}

func maybeAddTypeDescription(info descriptionInfo, description string) string {
if info.GraphQLDescription == "" {
return description
}
return fmt.Sprintf(
"%v\nThe GraphQL type's documentation follows.\n\n%v",
description, info.GraphQLDescription)
}

func fragmentDescription(info descriptionInfo) string {
return maybeAddTypeDescription(info, fmt.Sprintf(
"%v includes the GraphQL fields of %v requested by the fragment %v.",
info.FragmentName, info.GraphQLName, info.FragmentName))
}

func structDescription(typ *goStructType) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the current "comment" and "description" terminology a tad confusing. For example, this function returns the struct comment, which comes from the Comment field (if populated), or it generates a "description", which possibly includes the GraphQL description, if one is present.

What do you think about using the term "comment" for the code that generates Go comments, renaming Description to GraphQLDescription, and also renaming Comment to CommentOverride (or something similar)? I'm not quite sure how that jibes with your TODO to include any selection set commit, but I think it reflects how the current code works?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I like CommentOverride, will do!

switch {
case typ.CommentOverride != "":
return typ.CommentOverride
case typ.IsInput:
// Input types have all their fields, just use the GraphQL description.
return typ.GraphQLDescription
case typ.FragmentName != "":
return fragmentDescription(typ.descriptionInfo)
default:
// For types where we only have some fields, note that, along with
// the GraphQL documentation (if any). We don't want to just use
// the GraphQL documentation, since it may refer to fields we
// haven't selected, say.
return maybeAddTypeDescription(typ.descriptionInfo, fmt.Sprintf(
"%v includes the requested fields of the GraphQL type %v.",
typ.GoName, typ.GraphQLName))
}
}

func interfaceDescription(typ *goInterfaceType) string {
goImplNames := make([]string, len(typ.Implementations))
for i, impl := range typ.Implementations {
goImplNames[i] = impl.Reference()
}
implementationList := fmt.Sprintf(
"\n\n%v is implemented by the following types:\n\t%v",
typ.GoName, strings.Join(goImplNames, "\n\t"))

switch {
case typ.CommentOverride != "":
return typ.CommentOverride + implementationList
case typ.FragmentName != "":
return fragmentDescription(typ.descriptionInfo) + implementationList
default:
return maybeAddTypeDescription(typ.descriptionInfo, fmt.Sprintf(
"%v includes the requested fields of the GraphQL interface %v.%v",
typ.GoName, typ.GraphQLName, implementationList))
}
}
49 changes: 49 additions & 0 deletions generate/errors.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package generate

import (
"bytes"
"errors"
"fmt"
"go/scanner"
"math"
"strconv"
"strings"

Expand Down Expand Up @@ -130,3 +133,49 @@ func errorf(pos *ast.Position, msg string, args ...interface{}) error {
wrapped: wrapped,
}
}

// goSourceError processes the error(s) returned by go tooling (gofmt, etc.)
// into a nice error message.
//
// In practice, such errors are genqlient internal errors, but it's still
// useful to format them nicely for debugging.
func goSourceError(
failedOperation string, // e.g. "gofmt", for the error message
source []byte,
err error,
) error {
var errTexts []string
var scanErrs scanner.ErrorList
var scanErr *scanner.Error
var badLines map[int]bool

if errors.As(err, &scanErrs) {
errTexts = make([]string, len(scanErrs))
badLines = make(map[int]bool, len(scanErrs))
for i, scanErr := range scanErrs {
errTexts[i] = err.Error()
badLines[scanErr.Pos.Line] = true
}
} else if errors.As(err, &scanErr) {
errTexts = []string{scanErr.Error()}
badLines = map[int]bool{scanErr.Pos.Line: true}
} else {
errTexts = []string{err.Error()}
}

lines := bytes.SplitAfter(source, []byte("\n"))
lineNoWidth := int(math.Ceil(math.Log10(float64(len(lines) + 1))))
for i, line := range lines {
prefix := " "
if badLines[i] {
prefix = "> "
}
lineNo := strconv.Itoa(i + 1)
padding := strings.Repeat(" ", lineNoWidth-len(lineNo))
lines[i] = []byte(fmt.Sprintf("%s%s%s | %s", prefix, padding, lineNo, line))
}

return errorf(nil,
"genqlient internal error: failed to %s code:\n\t%s---source code---\n%s",
failedOperation, strings.Join(errTexts, "\n\t"), bytes.Join(lines, nil))
}
6 changes: 2 additions & 4 deletions generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,13 +382,11 @@ func Generate(config *Config) (map[string][]byte, error) {
unformatted := buf.Bytes()
formatted, err := format.Source(unformatted)
if err != nil {
return nil, errorf(nil, "could not gofmt code: %v\n---unformatted code---\n%v",
err, string(unformatted))
return nil, goSourceError("gofmt", unformatted, err)
}
importsed, err := imports.Process(config.Generated, formatted, nil)
if err != nil {
return nil, errorf(nil, "could not goimports code: %v\n---unimportsed code---\n%v",
err, string(formatted))
return nil, goSourceError("goimports", formatted, err)
}

retval := map[string][]byte{
Expand Down
Loading