Improve type assertion method argument validation (require zero) (graph-gophers#516)

Improve type assertion method argument validation (require zero)

It's tempting to include a context argument (or think it's allowed), but
not discover that this will fail until a query is executed. Validating
the resolver during schema parsing reduces the chance of inadvertant
errors here.

Signed-off-by: Evan Owen <kainosnoema@gmail.com>

Author: kainosnoema

graphql_test.go

@@ -3666,6 +3666,106 @@ func TestErrorPropagation(t *testing.T) {
 	})
 }
 
+type assertionResolver struct{}
+
+func (r *assertionResolver) ToHuman() (*struct{ Name string }, bool) {
+	return &struct{ Name string }{Name: "Luke Skywalker"}, true
+}
+
+type assertionQueryResolver struct{}
+
+func (*assertionQueryResolver) Character() *assertionResolver {
+	return &assertionResolver{}
+}
+
+type badAssertionResolver struct{}
+
+func (r *badAssertionResolver) ToHuman(ctx context.Context) (*struct{ Name string }, bool) {
+	return &struct{ Name string }{Name: "Luke Skywalker"}, true
+}
+
+type badAssertionQueryResolver struct{}
+
+func (*badAssertionQueryResolver) Character() *badAssertionResolver {
+	return &badAssertionResolver{}
+}
+
+func TestTypeAssertions(t *testing.T) {
+	assertionSchema := `
+		schema {
+			query: Query
+		}
+
+		type Query {
+			character: Character!
+		}
+
+		type Human {
+			name: String!
+		}
+
+		union Character = Human
+	`
+	query := `
+		query {
+			character {
+				... on Human {
+					name
+				}
+			}
+		}
+	`
+
+	gqltesting.RunTests(t, []*gqltesting.Test{
+		{
+			Schema: graphql.MustParseSchema(assertionSchema, &assertionQueryResolver{}, graphql.UseFieldResolvers()),
+			Query:  query,
+			ExpectedResult: `
+				{
+					"character": {
+						"name": "Luke Skywalker"
+					}
+				}
+			`,
+		},
+	})
+}
+
+func TestPanicTypeAssertionArguments(t *testing.T) {
+	panicMessage := `*graphql_test.badAssertionResolver does not resolve "Character": method "ToHuman" should't have any arguments
+	used by (*graphql_test.badAssertionQueryResolver).Character`
+
+	defer func() {
+		r := recover()
+		if r == nil {
+			t.Fatal("expected schema parse to panic")
+		}
+
+		if r.(error).Error() != panicMessage {
+			t.Logf("got:  %s", r)
+			t.Logf("want: %s", panicMessage)
+			t.Fail()
+		}
+	}()
+
+	schema := `
+		schema {
+			query: Query
+		}
+
+		type Query {
+			character: Character!
+		}
+
+		type Human {
+			name: String!
+		}
+
+		union Character = Human
+	`
+	graphql.MustParseSchema(schema, &badAssertionQueryResolver{}, graphql.UseFieldResolvers())
+}
+
 type ambiguousResolver struct {
 	Name string // ambiguous
 	University

internal/exec/resolvable/resolvable.go

@@ -277,7 +277,15 @@ func (b *execBuilder) makeObjectExec(typeName string, fields types.FieldsDefinit
 			if methodIndex == -1 {
 				return nil, fmt.Errorf("%s does not resolve %q: missing method %q to convert to %q", resolverType, typeName, "To"+impl.Name, impl.Name)
 			}
-			if resolverType.Method(methodIndex).Type.NumOut() != 2 {
+			m := resolverType.Method(methodIndex)
+			expectedIn := 0
+			if methodHasReceiver {
+				expectedIn = 1
+			}
+			if m.Type.NumIn() != expectedIn {
+				return nil, fmt.Errorf("%s does not resolve %q: method %q should't have any arguments", resolverType, typeName, "To"+impl.Name)
+			}
+			if m.Type.NumOut() != 2 {
 				return nil, fmt.Errorf("%s does not resolve %q: method %q should return a value and a bool indicating success", resolverType, typeName, "To"+impl.Name)
 			}
 			a := &TypeAssertion{
@@ -324,7 +332,7 @@ func (b *execBuilder) makeFieldExec(typeName string, f *types.FieldDefinition, m
 
 		if len(f.Arguments) > 0 {
 			if len(in) == 0 {
-				return nil, fmt.Errorf("must have parameter for field arguments")
+				return nil, fmt.Errorf("must have `args struct { ... }` argument for field arguments")
 			}
 			var err error
 			argsPacker, err = b.packerBuilder.MakeStructPacker(f.Arguments, in[0])
@@ -335,7 +343,7 @@ func (b *execBuilder) makeFieldExec(typeName string, f *types.FieldDefinition, m
 		}
 
 		if len(in) > 0 {
-			return nil, fmt.Errorf("too many parameters")
+			return nil, fmt.Errorf("too many arguments")
 		}
 
 		maxNumOfReturns := 2