Add support for interfaces, part 3: automatically add __typename (#56)

## Summary:
In #52, I added support for interface types, but with the simplifying
restriction (among others) that the user must request the field
`__typename`.  In this commit, I remove this restriction.

The basic idea is simple: we preprocess the query to add `__typename`.
The implementation isn't much more complicated!  Although it required
some new wiring in a few places.

Issue: https://github.com/Khan/genqlient/issues/8

## Test plan:
make check


Author: benjaminjkraft

Reviewers: dnerdy, aberkan, MiguelCastillo

Required Reviewers: 

Approved by: dnerdy

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

Pull request URL: https://github.com/Khan/genqlient/pull/56

generate/convert.go

@@ -259,19 +259,6 @@ func (g *generator) convertDefinition(
 			return nil, errorf(pos, "not implemented: %v", def.Kind)
 		}
 
-		// We need to request __typename so we know which concrete type to use.
-		hasTypename := false
-		for _, selection := range selectionSet {
-			field, ok := selection.(*ast.Field)
-			if ok && field.Name == "__typename" {
-				hasTypename = true
-			}
-		}
-		if !hasTypename {
-			// TODO(benkraft): Instead, modify the query to add __typename.
-			return nil, errorf(pos, "union/interface type %s must request __typename", def.Name)
-		}
-
 		implementationTypes := g.schema.GetPossibleTypes(def)
 		goType := &goInterfaceType{
 			GoName:          name,
@@ -283,6 +270,11 @@ func (g *generator) convertDefinition(
 
 		for i, implDef := range implementationTypes {
 			implName, implNamePrefix := g.typeName(namePrefix, implDef)
+			// TODO(benkraft): In principle we should skip generating a Go
+			// field for __typename each of these impl-defs if you didn't
+			// request it (and it was automatically added by
+			// preprocessQueryDocument).  But in practice it doesn't really
+			// hurt, and would be extra work to avoid, so we just leave it.
 			implTyp, err := g.convertDefinition(
 				implName, implNamePrefix, implDef, pos, selectionSet, queryOptions)
 			if err != nil {

generate/generate.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/vektah/gqlparser/v2/ast"
 	"github.com/vektah/gqlparser/v2/formatter"
+	"github.com/vektah/gqlparser/v2/validator"
 	"golang.org/x/tools/imports"
 )
 
@@ -140,17 +141,88 @@ func (g *generator) getArgument(
 	}, nil
 }
 
+// Preprocess each query to make any changes that genqlient needs.
+//
+// At present, the only change is that we add __typename, if not already
+// requested, to each field of interface type, so we can use the right types
+// when unmarshaling.
+func (g *generator) preprocessQueryDocument(doc *ast.QueryDocument) {
+	var observers validator.Events
+	// We want to ensure that everywhere you ask for some list of fields (a
+	// selection-set) from an interface (or union) type, you ask for its
+	// __typename field.  There are four places we might find a selection-set:
+	// at the toplevel of a query, on a field, or in an inline or named
+	// fragment.  The toplevel of a query must be an object type, so we don't
+	// need to consider that.
+	// TODO(benkraft): Once we support fragments, figure out whether we need to
+	// traverse inline/named fragments here too.
+	observers.OnField(func(_ *validator.Walker, field *ast.Field) {
+		// We are interested in a field from the query like
+		//	field { subField ... }
+		// where the schema looks like
+		//	type ... {       # or interface/union
+		//		field: FieldType    # or [FieldType!]! etc.
+		//	}
+		//	interface FieldType {   # or union
+		//		subField: ...
+		//	}
+		// If FieldType is an interface/union, and none of the subFields is
+		// __typename, we want to change the query to
+		//	field { __typename subField ... }
+
+		fieldType := g.schema.Types[field.Definition.Type.Name()]
+		if fieldType.Kind != ast.Interface && fieldType.Kind != ast.Union {
+			return // a concrete type
+		}
+
+		hasTypename := false
+		for _, selection := range field.SelectionSet {
+			// Check if we already selected __typename. We ignore fragments,
+			// because we want __typename as a toplevel field.
+			subField, ok := selection.(*ast.Field)
+			if ok && subField.Name == "__typename" {
+				hasTypename = true
+			}
+		}
+		if !hasTypename {
+			// Ok, we need to add the field!
+			field.SelectionSet = append(ast.SelectionSet{
+				&ast.Field{
+					Alias: "__typename", Name: "__typename",
+					// Fake definition for the magic field __typename cribbed
+					// from gqlparser's validator/walk.go, equivalent to
+					//	__typename: String
+					// TODO(benkraft): This should in principle be
+					//	__typename: String!
+					// But genqlient doesn't care, so we just match gqlparser.
+					Definition: &ast.FieldDefinition{
+						Name: "__typename",
+						Type: ast.NamedType("String", nil /* pos */),
+					},
+					// Definition of the object that contains this field, i.e.
+					// FieldType.
+					ObjectDefinition: fieldType,
+				},
+			}, field.SelectionSet...)
+		}
+	})
+	validator.Walk(g.schema, doc, &observers)
+}
+
 func (g *generator) addOperation(op *ast.OperationDefinition) error {
 	if op.Name == "" {
 		return errorf(op.Position, "operations must have operation-names")
 	}
 
-	var builder strings.Builder
+	queryDoc := &ast.QueryDocument{
-	f := formatter.NewFormatter(&builder)
-	f.FormatQueryDocument(&ast.QueryDocument{
 		Operations: ast.OperationList{op},
 		// TODO: handle fragments
-	})
+	}
+	g.preprocessQueryDocument(queryDoc)
+
+	var builder strings.Builder
+	f := formatter.NewFormatter(&builder)
+	f.FormatQueryDocument(queryDoc)
 
 	commentLines, directive, err := g.parsePrecedingComment(op, op.Position)
 	if err != nil {

generate/genqlient_directive.go

@@ -146,6 +146,10 @@ func (g *generator) parsePrecedingComment(
 	pos *ast.Position,
 ) (comment string, directive *GenqlientDirective, err error) {
 	directive = new(GenqlientDirective)
+	if pos == nil || pos.Src == nil { // node was added by genqlient itself
+		return "", directive, nil // treated as if there were no comment
+	}
+
 	var commentLines []string
 	sourceLines := strings.Split(pos.Src.Input, "\n")
 	for i := pos.Line - 1; i > 0; i-- {

generate/testdata/errors/MissingTypeName.go

@@ -1,5 +0,0 @@
-package errors
-
-const _ = `# @genqlient
-	query MyQuery { i { f } }
-`

generate/testdata/errors/MissingTypeName.graphql

@@ -1 +0,0 @@
-query MyQuery { i { f } }

generate/testdata/errors/MissingTypeName.schema.graphql

@@ -1,11 +0,0 @@
-type Query {
-  i: I
-}
-
-type T implements I {
-  f: String!
-}
-
-interface I {
-  f: String!
-}

generate/testdata/queries/InterfaceListField.graphql

@@ -3,7 +3,6 @@ query InterfaceListField {
     id
     name
     children {
-      __typename
       id
       name
     }
@@ -13,7 +12,6 @@ query InterfaceListField {
     id
     name
     children {
-      __typename
       id
       name
     }

generate/testdata/queries/InterfaceListOfListsOfListsField.graphql

@@ -1,5 +1,5 @@
 query InterfaceListOfListOfListsField {
-  listOfListsOfListsOfContent { __typename id name }
+  listOfListsOfListsOfContent { id name }
   # @genqlient(pointer: true)
-  withPointer: listOfListsOfListsOfContent { __typename id name }
+  withPointer: listOfListsOfListsOfContent { id name }
 }

generate/testdata/queries/InterfaceNesting.graphql

@@ -2,13 +2,10 @@ query InterfaceNesting {
   root {
     id
     children {
-      __typename
       id
       parent {
-        __typename
         id
         children {
-          __typename
           id
         }
       }

generate/testdata/queries/InterfaceNoFragments.graphql

@@ -1,6 +1,7 @@
 query InterfaceNoFragmentsQuery {
   root { id name }       # (make sure sibling fields work)
-  randomItem { __typename id name }
+  randomItem { id name }
+  randomItemWithTypeName: randomItem { __typename id name }
   # @genqlient(pointer: true)
-  withPointer: randomItem { __typename id name }
+  withPointer: randomItem { id name }
 }

generate/testdata/snapshots/TestGenerate-InterfaceListOfListsOfListsField.graphql-InterfaceListOfListsOfListsField.graphql.go

@@ -160,7 +160,7 @@ func (v *InterfaceListOfListOfListsFieldResponse) UnmarshalJSON(b []byte) error
 
 // InterfaceListOfListOfListsFieldWithPointerArticle includes the requested fields of the GraphQL type Article.
 type InterfaceListOfListOfListsFieldWithPointerArticle struct {
-	Typename *string `json:"__typename"`
+	Typename string `json:"__typename"`
 	// ID is the identifier of the content.
 	Id   *testutil.ID `json:"id"`
 	Name *string      `json:"name"`
@@ -212,7 +212,7 @@ func __unmarshalInterfaceListOfListOfListsFieldWithPointerContent(v *InterfaceLi
 
 // InterfaceListOfListOfListsFieldWithPointerTopic includes the requested fields of the GraphQL type Topic.
 type InterfaceListOfListOfListsFieldWithPointerTopic struct {
-	Typename *string `json:"__typename"`
+	Typename string `json:"__typename"`
 	// ID is the identifier of the content.
 	Id   *testutil.ID `json:"id"`
 	Name *string      `json:"name"`
@@ -220,7 +220,7 @@ type InterfaceListOfListOfListsFieldWithPointerTopic struct {
 
 // InterfaceListOfListOfListsFieldWithPointerVideo includes the requested fields of the GraphQL type Video.
 type InterfaceListOfListOfListsFieldWithPointerVideo struct {
-	Typename *string `json:"__typename"`
+	Typename string `json:"__typename"`
 	// ID is the identifier of the content.
 	Id   *testutil.ID `json:"id"`
 	Name *string      `json:"name"`

generate/testdata/snapshots/TestGenerate-InterfaceNesting.graphql-InterfaceNesting.graphql.go

@@ -65,7 +65,6 @@ type InterfaceNestingRootTopicChildrenArticle struct {
 
 // InterfaceNestingRootTopicChildrenArticleParentTopic includes the requested fields of the GraphQL type Topic.
 type InterfaceNestingRootTopicChildrenArticleParentTopic struct {
-	Typename string `json:"__typename"`
 	// ID is documented in the Content interface.
 	Id       testutil.ID                                                          `json:"id"`
 	Children []InterfaceNestingRootTopicChildrenArticleParentTopicChildrenContent `json:"-"`
@@ -223,7 +222,6 @@ type InterfaceNestingRootTopicChildrenTopic struct {
 
 // InterfaceNestingRootTopicChildrenTopicParentTopic includes the requested fields of the GraphQL type Topic.
 type InterfaceNestingRootTopicChildrenTopicParentTopic struct {
-	Typename string `json:"__typename"`
 	// ID is documented in the Content interface.
 	Id       testutil.ID                                                        `json:"id"`
 	Children []InterfaceNestingRootTopicChildrenTopicParentTopicChildrenContent `json:"-"`
@@ -337,7 +335,6 @@ type InterfaceNestingRootTopicChildrenVideo struct {
 
 // InterfaceNestingRootTopicChildrenVideoParentTopic includes the requested fields of the GraphQL type Topic.
 type InterfaceNestingRootTopicChildrenVideoParentTopic struct {
-	Typename string `json:"__typename"`
 	// ID is documented in the Content interface.
 	Id       testutil.ID                                                        `json:"id"`
 	Children []InterfaceNestingRootTopicChildrenVideoParentTopicChildrenContent `json:"-"`
@@ -456,7 +453,6 @@ query InterfaceNesting {
 			__typename
 			id
 			parent {
-				__typename
 				id
 				children {
 					__typename

generate/testdata/snapshots/TestGenerate-InterfaceNesting.graphql-InterfaceNesting.graphql.json

@@ -2,7 +2,7 @@
   "operations": [
     {
       "operationName": "InterfaceNesting",
-      "query": "\nquery InterfaceNesting {\n\troot {\n\t\tid\n\t\tchildren {\n\t\t\t__typename\n\t\t\tid\n\t\t\tparent {\n\t\t\t\t__typename\n\t\t\t\tid\n\t\t\t\tchildren {\n\t\t\t\t\t__typename\n\t\t\t\t\tid\n\t\t\t\t}\n\t\t\t}\n\t\t}\n\t}\n}\n",
+      "query": "\nquery InterfaceNesting {\n\troot {\n\t\tid\n\t\tchildren {\n\t\t\t__typename\n\t\t\tid\n\t\t\tparent {\n\t\t\t\tid\n\t\t\t\tchildren {\n\t\t\t\t\t__typename\n\t\t\t\t\tid\n\t\t\t\t}\n\t\t\t}\n\t\t}\n\t}\n}\n",
       "sourceLocation": "testdata/queries/InterfaceNesting.graphql"
     }
   ]

generate/testdata/snapshots/TestGenerate-InterfaceNoFragments.graphql-InterfaceNoFragments.graphql.go

@@ -78,11 +78,80 @@ type InterfaceNoFragmentsQueryRandomItemVideo struct {
 	Name string      `json:"name"`
 }
 
+// InterfaceNoFragmentsQueryRandomItemWithTypeNameArticle includes the requested fields of the GraphQL type Article.
+type InterfaceNoFragmentsQueryRandomItemWithTypeNameArticle struct {
+	Typename string `json:"__typename"`
+	// ID is the identifier of the content.
+	Id   testutil.ID `json:"id"`
+	Name string      `json:"name"`
+}
+
+// InterfaceNoFragmentsQueryRandomItemWithTypeNameContent includes the requested fields of the GraphQL type Content.
+// The GraphQL type's documentation follows.
+//
+// Content is implemented by various types like Article, Video, and Topic.
+type InterfaceNoFragmentsQueryRandomItemWithTypeNameContent interface {
+	implementsGraphQLInterfaceInterfaceNoFragmentsQueryRandomItemWithTypeNameContent()
+}
+
+func (v *InterfaceNoFragmentsQueryRandomItemWithTypeNameArticle) implementsGraphQLInterfaceInterfaceNoFragmentsQueryRandomItemWithTypeNameContent() {
+}
+func (v *InterfaceNoFragmentsQueryRandomItemWithTypeNameVideo) implementsGraphQLInterfaceInterfaceNoFragmentsQueryRandomItemWithTypeNameContent() {
+}
+func (v *InterfaceNoFragmentsQueryRandomItemWithTypeNameTopic) implementsGraphQLInterfaceInterfaceNoFragmentsQueryRandomItemWithTypeNameContent() {
+}
+
+func __unmarshalInterfaceNoFragmentsQueryRandomItemWithTypeNameContent(v *InterfaceNoFragmentsQueryRandomItemWithTypeNameContent, m json.RawMessage) error {
+	if string(m) == "null" {
+		return nil
+	}
+
+	var tn struct {
+		TypeName string `json:"__typename"`
+	}
+	err := json.Unmarshal(m, &tn)
+	if err != nil {
+		return err
+	}
+
+	switch tn.TypeName {
+	case "Article":
+		*v = new(InterfaceNoFragmentsQueryRandomItemWithTypeNameArticle)
+		return json.Unmarshal(m, *v)
+	case "Video":
+		*v = new(InterfaceNoFragmentsQueryRandomItemWithTypeNameVideo)
+		return json.Unmarshal(m, *v)
+	case "Topic":
+		*v = new(InterfaceNoFragmentsQueryRandomItemWithTypeNameTopic)
+		return json.Unmarshal(m, *v)
+	default:
+		return fmt.Errorf(
+			`Unexpected concrete type for InterfaceNoFragmentsQueryRandomItemWithTypeNameContent: "%v"`, tn.TypeName)
+	}
+}
+
+// InterfaceNoFragmentsQueryRandomItemWithTypeNameTopic includes the requested fields of the GraphQL type Topic.
+type InterfaceNoFragmentsQueryRandomItemWithTypeNameTopic struct {
+	Typename string `json:"__typename"`
+	// ID is the identifier of the content.
+	Id   testutil.ID `json:"id"`
+	Name string      `json:"name"`
+}
+
+// InterfaceNoFragmentsQueryRandomItemWithTypeNameVideo includes the requested fields of the GraphQL type Video.
+type InterfaceNoFragmentsQueryRandomItemWithTypeNameVideo struct {
+	Typename string `json:"__typename"`
+	// ID is the identifier of the content.
+	Id   testutil.ID `json:"id"`
+	Name string      `json:"name"`
+}
+
 // InterfaceNoFragmentsQueryResponse is returned by InterfaceNoFragmentsQuery on success.
 type InterfaceNoFragmentsQueryResponse struct {
-	Root        InterfaceNoFragmentsQueryRootTopic           `json:"root"`
+	Root                   InterfaceNoFragmentsQueryRootTopic                     `json:"root"`
-	RandomItem  InterfaceNoFragmentsQueryRandomItemContent   `json:"-"`
+	RandomItem             InterfaceNoFragmentsQueryRandomItemContent             `json:"-"`
-	WithPointer *InterfaceNoFragmentsQueryWithPointerContent `json:"-"`
+	RandomItemWithTypeName InterfaceNoFragmentsQueryRandomItemWithTypeNameContent `json:"-"`
+	WithPointer            *InterfaceNoFragmentsQueryWithPointerContent           `json:"-"`
 }
 
 func (v *InterfaceNoFragmentsQueryResponse) UnmarshalJSON(b []byte) error {
@@ -91,8 +160,9 @@ func (v *InterfaceNoFragmentsQueryResponse) UnmarshalJSON(b []byte) error {
 
 	var firstPass struct {
 		*InterfaceNoFragmentsQueryResponseWrapper
-		RandomItem  json.RawMessage `json:"randomItem"`
+		RandomItem             json.RawMessage `json:"randomItem"`
-		WithPointer json.RawMessage `json:"withPointer"`
+		RandomItemWithTypeName json.RawMessage `json:"randomItemWithTypeName"`
+		WithPointer            json.RawMessage `json:"withPointer"`
 	}
 	firstPass.InterfaceNoFragmentsQueryResponseWrapper = (*InterfaceNoFragmentsQueryResponseWrapper)(v)
 
@@ -110,6 +180,15 @@ func (v *InterfaceNoFragmentsQueryResponse) UnmarshalJSON(b []byte) error {
 			return err
 		}
 	}
+	{
+		target := &v.RandomItemWithTypeName
+		raw := firstPass.RandomItemWithTypeName
+		err = __unmarshalInterfaceNoFragmentsQueryRandomItemWithTypeNameContent(
+			target, raw)
+		if err != nil {
+			return err
+		}
+	}
 	{
 		target := &v.WithPointer
 		raw := firstPass.WithPointer
@@ -132,7 +211,7 @@ type InterfaceNoFragmentsQueryRootTopic struct {
 
 // InterfaceNoFragmentsQueryWithPointerArticle includes the requested fields of the GraphQL type Article.
 type InterfaceNoFragmentsQueryWithPointerArticle struct {
-	Typename *string `json:"__typename"`
+	Typename string `json:"__typename"`
 	// ID is the identifier of the content.
 	Id   *testutil.ID `json:"id"`
 	Name *string      `json:"name"`
@@ -184,7 +263,7 @@ func __unmarshalInterfaceNoFragmentsQueryWithPointerContent(v *InterfaceNoFragme
 
 // InterfaceNoFragmentsQueryWithPointerTopic includes the requested fields of the GraphQL type Topic.
 type InterfaceNoFragmentsQueryWithPointerTopic struct {
-	Typename *string `json:"__typename"`
+	Typename string `json:"__typename"`
 	// ID is the identifier of the content.
 	Id   *testutil.ID `json:"id"`
 	Name *string      `json:"name"`
@@ -192,7 +271,7 @@ type InterfaceNoFragmentsQueryWithPointerTopic struct {
 
 // InterfaceNoFragmentsQueryWithPointerVideo includes the requested fields of the GraphQL type Video.
 type InterfaceNoFragmentsQueryWithPointerVideo struct {
-	Typename *string `json:"__typename"`
+	Typename string `json:"__typename"`
 	// ID is the identifier of the content.
 	Id   *testutil.ID `json:"id"`
 	Name *string      `json:"name"`
@@ -216,6 +295,11 @@ query InterfaceNoFragmentsQuery {
 		id
 		name
 	}
+	randomItemWithTypeName: randomItem {
+		__typename
+		id
+		name
+	}
 	withPointer: randomItem {
 		__typename
 		id

generate/testdata/snapshots/TestGenerate-InterfaceNoFragments.graphql-InterfaceNoFragments.graphql.json

@@ -2,7 +2,7 @@
   "operations": [
     {
       "operationName": "InterfaceNoFragmentsQuery",
-      "query": "\nquery InterfaceNoFragmentsQuery {\n\troot {\n\t\tid\n\t\tname\n\t}\n\trandomItem {\n\t\t__typename\n\t\tid\n\t\tname\n\t}\n\twithPointer: randomItem {\n\t\t__typename\n\t\tid\n\t\tname\n\t}\n}\n",
+      "query": "\nquery InterfaceNoFragmentsQuery {\n\troot {\n\t\tid\n\t\tname\n\t}\n\trandomItem {\n\t\t__typename\n\t\tid\n\t\tname\n\t}\n\trandomItemWithTypeName: randomItem {\n\t\t__typename\n\t\tid\n\t\tname\n\t}\n\twithPointer: randomItem {\n\t\t__typename\n\t\tid\n\t\tname\n\t}\n}\n",
       "sourceLocation": "testdata/queries/InterfaceNoFragments.graphql"
     }
   ]

internal/integration/integration_test.go

@@ -59,7 +59,7 @@ func TestVariables(t *testing.T) {
 func TestInterfaceNoFragments(t *testing.T) {
 	_ = `# @genqlient
 	query queryWithInterfaceNoFragments($id: ID!) {
-		being(id: $id) { __typename id name }
+		being(id: $id) { id name }
 		me { id name }
 	}`
 
@@ -102,7 +102,7 @@ func TestInterfaceNoFragments(t *testing.T) {
 func TestInterfaceListField(t *testing.T) {
 	_ = `# @genqlient
 	query queryWithInterfaceListField($ids: [ID!]!) {
-		beings(ids: $ids) { __typename id name }
+		beings(ids: $ids) { id name }
 	}`
 
 	ctx := context.Background()