Warn about a field that an object or input type declares twice

src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt

@@ -172,6 +172,8 @@
         val name = objectDefinition.name
         val extensions = typeExtensions.extensionsOf<ObjectTypeExtensionDefinition>(name)
         val directives = sdlDirectives.directivesOf(objectDefinition, extensions)
+        val fieldDefinitions = objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }
+        warnRepeatedFields(name, fieldDefinitions)
         val builder = GraphQLObjectType.newObject()
             .name(name)
             .definition(objectDefinition)
@@ -186,7 +188,7 @@
                 }
             }
             .apply {
-                objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }.forEach { fieldDefinition ->
+                fieldDefinitions.forEach { fieldDefinition ->
                     field { field ->
                         createField(field, fieldDefinition, inputObjects)
                         codeRegistryBuilder.dataFetcher(
@@ -210,6 +212,8 @@
                                   inputObjectsInProgress: MutableSet<String>): GraphQLInputObjectType {
         val extensionDefinitions = typeExtensions.extensionsOf<InputObjectTypeExtensionDefinition>(definition.name)
         val directives = sdlDirectives.directivesOf(definition, extensionDefinitions)
+        val fieldDefinitions = (extensionDefinitions + definition).flatMap { it.inputValueDefinitions }
+        warnRepeatedFields(definition.name, fieldDefinitions)
 
         inputObjectsInProgress.add(definition.name)
 
@@ -220,20 +224,18 @@
             .description(getDocumentation(definition, options))
             .withSdlDirectives(directives, INPUT_OBJECT)
             .apply {
-                (extensionDefinitions + definition).forEach { typeDefinition ->
-                    typeDefinition.inputValueDefinitions.forEach { fieldDefinition ->
-                        field(
-                            GraphQLInputObjectField.newInputObjectField()
-                                .name(fieldDefinition.name)
-                                .definition(fieldDefinition)
-                                .description(getDocumentation(fieldDefinition, options))
-                                .apply { fieldDefinition.defaultValue?.let { v -> defaultValueLiteral(v) } }
-                                .apply { sdlDirectives.getDeprecated(fieldDefinition.directives)?.let { deprecate(it) } }
-                                .type(determineInputType(fieldDefinition.type, inputObjects, inputObjectsInProgress))
-                                .withSdlDirectives(fieldDefinition.directives, INPUT_FIELD_DEFINITION)
-                                .build()
-                        )
-                    }
+                fieldDefinitions.forEach { fieldDefinition ->
+                    field(
+                        GraphQLInputObjectField.newInputObjectField()
+                            .name(fieldDefinition.name)
+                            .definition(fieldDefinition)
+                            .description(getDocumentation(fieldDefinition, options))
+                            .apply { fieldDefinition.defaultValue?.let { v -> defaultValueLiteral(v) } }
+                            .apply { sdlDirectives.getDeprecated(fieldDefinition.directives)?.let { deprecate(it) } }
+                            .type(determineInputType(fieldDefinition.type, inputObjects, inputObjectsInProgress))
+                            .withSdlDirectives(fieldDefinition.directives, INPUT_FIELD_DEFINITION)
+                            .build()
+                    )
                 }
             }
 
@@ -383,6 +385,16 @@
         }
     }
 
+    // TODO throw a SchemaError instead in 15.0.0 (#865), like graphql-java does
+    private fun warnRepeatedFields(typeName: String, fieldDefinitions: List<NamedNode<*>>) {
+        fieldDefinitions.groupBy { it.name }.values.filter { it.size > 1 }.forEach { declarations ->
+            val kept = declarations.last()
+            val location = kept.sourceLocation.describe()
+            val which = if (location.isEmpty()) "one of them" else "the one$location"
+            log.warn("Type '$typeName' declares field '${kept.name}' more than once, only $which is kept.")
+        }
+    }
+
     private fun createField(field: GraphQLFieldDefinition.Builder, fieldDefinition: FieldDefinition, inputObjects: List<GraphQLInputObjectType>): GraphQLFieldDefinition.Builder {
         return field
             .name(fieldDefinition.name)

src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt

@@ -2,10 +2,13 @@ package graphql.kickstart.tools
 
 import graphql.GraphQL
 import graphql.Scalars
+import graphql.language.Definition
 import graphql.language.EnumTypeDefinition
+import graphql.language.FieldDefinition
 import graphql.language.InterfaceTypeDefinition
 import graphql.language.ObjectTypeDefinition
 import graphql.language.ScalarTypeDefinition
+import graphql.language.TypeName
 import graphql.language.UnionTypeDefinition
 import graphql.schema.*
 import graphql.schema.idl.SchemaDirectiveWiring
@@ -393,6 +396,135 @@ class TypeExtensionTest {
         assertEquals((schema.getType("FooBar") as GraphQLUnionType).types.map { it.name }, listOf("Foo", "Bar", "Baz"))
     }
 
+    @Test
+    fun `repeated fields of objects and inputs keep building and keep the last one`() {
+        assertRepeatedObjectField(
+            """
+            type Item implements Node {
+                id: ID!
+                "From the type"
+                name: String
+            }
+            extend type Item {
+                "From the extension"
+                name: String
+            }
+            """,
+            location = "at line 9, column 5",
+            kept = "From the extension"
+        )
+        assertRepeatedObjectField(
+            """
+            type Item implements Node { id: ID! }
+            extend type Item {
+                "From the first extension"
+                name: String
+            }
+            extend type Item {
+                "From the second extension"
+                name: String
+            }
+            """,
+            location = "at line 9, column 5",
+            kept = "From the second extension"
+        )
+        assertRepeatedObjectField(
+            """
+            type Item implements Node {
+                id: ID!
+                "The first"
+                name: String
+                "The second"
+                name: String
+            }
+            """,
+            location = "at line 7, column 5",
+            kept = "The second"
+        )
+
+        assertRepeatedInputField(
+            "Filter",
+            """
+            input Filter {
+                "From the type"
+                id: ID
+            }
+            extend input Filter {
+                "From the extension"
+                id: ID
+            }
+            """,
+            location = "at line 3, column 5",
+            kept = "From the type"
+        )
+        assertRepeatedInputField(
+            "Filter",
+            """
+            input Filter { term: String }
+            extend input Filter {
+                "From the first extension"
+                id: ID
+            }
+            extend input Filter {
+                "From the second extension"
+                id: ID
+            }
+            """,
+            location = "at line 8, column 5",
+            kept = "From the second extension"
+        )
+        assertRepeatedInputField(
+            "Filter",
+            """
+            input Filter {
+                "The first"
+                id: ID
+                "The second"
+                id: ID
+            }
+            """,
+            location = "at line 5, column 5",
+            kept = "The second"
+        )
+        assertRepeatedInputField(
+            "Inner",
+            """
+            input Filter {
+                inner: Inner
+            }
+            input Inner {
+                "The first"
+                id: ID
+                "The second"
+                id: ID
+            }
+            """,
+            location = "at line 8, column 5",
+            kept = "The second"
+        )
+    }
+
+    @Test
+    fun `a repeated field without a source location says only that one is kept`() {
+        fun field(name: String) = FieldDefinition.newFieldDefinition().name(name).type(TypeName("String")).build()
+        val item = ObjectTypeDefinition.newObjectTypeDefinition()
+            .name("Item")
+            .implementz(TypeName("Node"))
+            .fieldDefinitions(listOf(field("id"), field("name"), field("name")))
+            .build()
+
+        val (_, warnings) = withWarnings {
+            parser("type Query { node: Node }\ninterface Node { id: String }") {
+                dictionary(Item::class)
+                options(SchemaParserOptions.newOptions().typeDefinitionFactory(object : TypeDefinitionFactory {
+                    override fun create(existing: MutableList<Definition<*>>) = listOf(item)
+                }).build())
+            }.makeExecutableSchema()
+        }
+
+        assertEquals(warnings, listOf("Type 'Item' declares field 'name' more than once, only one of them is kept."))
+    }
+
     @Test
     fun `extensions of a different kind than the type are ignored`() {
         val (schema, warnings) = withWarnings {
@@ -473,6 +605,26 @@ class TypeExtensionTest {
         assertEquals(schema.getSchemaAppliedDirectives("link").map { it.getArgument("for")?.getValue<String>() }, listOf("EXECUTION"))
     }
 
+    private fun assertRepeatedObjectField(item: String, location: String, kept: String) {
+        val (schema, warnings) = withWarnings {
+            parser("type Query { node: Node }\ninterface Node { id: ID! }\n${item.trimIndent()}") {
+                dictionary(Item::class)
+            }.makeExecutableSchema()
+        }
+
+        assertEquals(warnings, listOf("Type 'Item' declares field 'name' more than once, only the one $location is kept."))
+        assertEquals((schema.getType("Item") as GraphQLObjectType).getFieldDefinition("name").description, kept)
+    }
+
+    private fun assertRepeatedInputField(repeating: String, filter: String, location: String, kept: String) {
+        val (schema, warnings) = withWarnings {
+            parser("type Query { search(filter: Filter): String }\n${filter.trimIndent()}").makeExecutableSchema()
+        }
+
+        assertEquals(warnings, listOf("Type '$repeating' declares field 'id' more than once, only the one $location is kept."))
+        assertEquals((schema.getType(repeating) as GraphQLInputObjectType).getField("id").description, kept)
+    }
+
     private val fooBarBaz = """
         type Foo { foo: String }
         type Bar { bar: String }
@@ -502,7 +654,9 @@ class TypeExtensionTest {
 
     class Item(val id: String, val name: String?, val fooBar: Foo? = null) : Node
 
-    class Filter(val id: String?)
+    class Filter(val id: String?, val term: String? = null, val inner: Inner? = null)
+
+    class Inner(val id: String?)
 
     enum class Color { RED, GREEN }