Fix line numbers in schema syntax errors and name the source

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

@@ -2,6 +2,8 @@ package graphql.kickstart.tools
 
 import graphql.language.Definition
 import graphql.language.Document
+import graphql.language.SourceLocation
+import graphql.parser.InvalidSyntaxException
 import graphql.parser.MultiSourceReader
 import graphql.parser.Parser
 import graphql.parser.ParserEnvironment
@@ -11,6 +13,7 @@ import graphql.schema.idl.RuntimeWiring
 import graphql.schema.idl.SchemaDirectiveWiring
 import org.antlr.v4.runtime.RecognitionException
 import org.antlr.v4.runtime.misc.ParseCancellationException
+import java.text.MessageFormat
 import kotlin.Int.Companion.MAX_VALUE
 import kotlin.reflect.KClass
 
@@ -20,7 +23,7 @@ import kotlin.reflect.KClass
 class SchemaParserBuilder {
 
     private val dictionary = SchemaParserDictionary()
-    private val schemaString = StringBuilder()
+    private val schemaStrings = mutableListOf<Pair<String, String?>>()
     private val files = mutableListOf<String>()
     private val resolvers = mutableListOf<GraphQLResolver<*>>()
     private val scalars = mutableListOf<GraphQLScalarType>()
@@ -49,10 +52,14 @@ class SchemaParserBuilder {
      * Add a GraphQL schema string directly.
      */
     fun schemaString(string: String) = this.apply {
-        if (schemaString.isNotEmpty()) {
-            schemaString.append("\n")
-        }
-        schemaString.append(string)
+        schemaStrings.add(string to null)
+    }
+
+    /**
+     * Add a GraphQL schema string directly, naming its source in parse errors and source locations.
+     */
+    fun schemaString(string: String, sourceName: String) = this.apply {
+        schemaStrings.add(string to sourceName)
     }
 
     /**
@@ -172,10 +179,10 @@ class SchemaParserBuilder {
 
     private fun parseDocuments(): List<Document> {
         try {
-            val documents = files.map { parseDocument(readFile(it), it) }.toMutableList()
+            val documents = files.map { parseDocument(listOf(readFile(it) to it)) }.toMutableList()
 
-            if (schemaString.isNotBlank()) {
-                documents.add(parseDocument(schemaString.toString()))
+            if (schemaStrings.any { it.first.isNotBlank() }) {
+                documents.add(parseDocument(schemaStrings))
             }
 
             return documents
@@ -189,18 +196,35 @@ class SchemaParserBuilder {
         }
     }
 
-    private fun parseDocument(input: String, sourceName: String? = null): Document {
-        val sourceReader = MultiSourceReader
-            .newMultiSourceReader()
-            .string(input, sourceName)
-            .trackData(true).build()
+    private fun parseDocument(sources: List<Pair<String, String?>>): Document {
+        // MultiSourceReader numbers the last line of the last source from the start of the first one. Ending
+        // every source with a line break leaves only the end of input there, whose line is fixed below.
+        val inputs = sources.map { (input, _) -> if (sources.size > 1 && !input.endsWith("\n")) "$input\n" else input }
+        val sourceReaderBuilder = MultiSourceReader.newMultiSourceReader()
+        inputs.forEachIndexed { index, input -> sourceReaderBuilder.string(input, sources[index].second) }
+        val sourceReader = sourceReaderBuilder.trackData(true).build()
         val environment = ParserEnvironment
             .newParserEnvironment()
             .document(sourceReader)
             .parserOptions(parserOptions).build()
-        return parser.parseDocument(environment)
+        try {
+            return parser.parseDocument(environment)
+        } catch (e: InvalidSyntaxException) {
+            val location = e.location ?: throw e
+            val linesBeforeLast = inputs.dropLast(1).sumOf { it.lines().size - 1 }
+            val endOfInput = location.line == linesBeforeLast + inputs.last().lines().size
+            val line = if (endOfInput) location.line - linesBeforeLast else location.line
+            if (line == location.line && location.sourceName == null) throw e
+
+            val message = e.message.orEmpty().replaceFirst(" ${formatLine(location.line)} ", " ${formatLine(line)} ") +
+                location.sourceName?.let { " in $it" }.orEmpty()
+            throw SchemaSyntaxException(message, SourceLocation(line, location.column, location.sourceName), e)
+        }
     }
 
+    // Same formatting graphql-java uses for the line in its messages, e.g. "1,103" in English.
+    private fun formatLine(line: Int) = MessageFormat("{0}").format(arrayOf<Any>(line))
+
     private fun readFile(filename: String) =
         this::class.java.classLoader.getResource(filename)?.readText()
             ?: throw java.io.FileNotFoundException("classpath:$filename")
@@ -218,3 +242,6 @@ class InvalidSchemaError(
     override val message: String
         get() = "Invalid schema provided (${recognitionException.javaClass.name}) at: ${recognitionException.offendingToken}"
 }
+
+internal class SchemaSyntaxException(message: String, location: SourceLocation, e: InvalidSyntaxException) :
+    InvalidSyntaxException(message, location, e.offendingToken, e.sourcePreview, e)

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

@@ -3,6 +3,7 @@ package graphql.kickstart.tools
 import graphql.ExecutionResult
 import graphql.GraphQL
 import graphql.kickstart.tools.resolver.FieldResolverError
+import graphql.parser.InvalidSyntaxException
 import graphql.schema.*
 import graphql.schema.idl.SchemaDirectiveWiring
 import graphql.schema.idl.SchemaDirectiveWiringEnvironment
@@ -294,6 +295,137 @@ class SchemaParserTest {
         assertEquals(sourceLocation?.sourceName, "Test.graphqls")
     }
 
+    @Test
+    fun `parser should report syntax error line relative to the schema string containing it`() {
+        val error = assertThrows(InvalidSyntaxException::class.java) {
+            SchemaParser.newParser()
+                .schemaString(
+                    """
+                    |type Query {
+                    |    id: ID!
+                    |}
+                    """.trimMargin())
+                .schemaString(
+                    """
+                    |type Foo {
+                    |    bar: String!!
+                    |}
+                    """.trimMargin())
+                .build()
+        }
+
+        assertEquals(error.message, "Invalid syntax with offending token '!' at line 2 column 17")
+        assertEquals(error.location?.line, 2)
+    }
+
+    @Test
+    fun `parser should include file name in syntax error`() {
+        val error = assertThrows(InvalidSyntaxException::class.java) {
+            SchemaParser.newParser()
+                .file("Test.graphqls")
+                .file("InvalidSyntax.graphqls")
+                .build()
+        }
+
+        assertEquals(error.message, "Invalid syntax with offending token '!' at line 2 column 15 in InvalidSyntax.graphqls")
+        assertEquals(error.location?.sourceName, "InvalidSyntax.graphqls")
+    }
+
+    @Test
+    fun `parser should include source name in syntax error from named schema string`() {
+        val error = assertThrows(InvalidSyntaxException::class.java) {
+            SchemaParser.newParser()
+                .schemaString(
+                    """
+                    |type Query {
+                    |    id: ID!
+                    |}
+                    """.trimMargin(), "Query.graphqls")
+                .schemaString(
+                    """
+                    |type Foo {
+                    |    bar: String!!
+                    |}
+                    """.trimMargin(), "Foo.graphqls")
+                .build()
+        }
+
+        assertEquals(error.message, "Invalid syntax with offending token '!' at line 2 column 17 in Foo.graphqls")
+        assertEquals(error.location?.sourceName, "Foo.graphqls")
+    }
+
+    @Test
+    fun `parser should report syntax error on the last line of the last schema string relative to it`() {
+        val error = assertThrows(InvalidSyntaxException::class.java) {
+            SchemaParser.newParser()
+                .schemaString(
+                    """
+                    |type Query {
+                    |    id: ID!
+                    |}
+                    """.trimMargin())
+                .schemaString("type Foo { bar: String!! }")
+                .build()
+        }
+
+        assertEquals(error.message, "Invalid syntax with offending token '!' at line 1 column 24")
+        assertEquals(error.location?.line, 1)
+    }
+
+    @Test
+    fun `parser should report unexpected end of the last schema string relative to it`() {
+        val error = assertThrows(InvalidSyntaxException::class.java) {
+            SchemaParser.newParser()
+                .schemaString(
+                    """
+                    |type Query {
+                    |    id: ID!
+                    |}
+                    """.trimMargin(), "Query.graphqls")
+                .schemaString("type Foo {\n    bar: String\n", "Foo.graphqls")
+                .build()
+        }
+
+        assertEquals(error.message, "Invalid syntax with offending token '<EOF>' at line 3 column 1 in Foo.graphqls")
+        assertEquals(error.location?.line, 3)
+    }
+
+    @Test
+    fun `parser should report unexpected end of the last schema string relative to it after 1000 lines`() {
+        val error = assertThrows(InvalidSyntaxException::class.java) {
+            SchemaParser.newParser()
+                .schemaString((1..1100).joinToString("\n") { "type Type$it { id: ID! }" }, "Types.graphqls")
+                .schemaString("type Foo {\n    bar: String\n", "Foo.graphqls")
+                .build()
+        }
+
+        assertEquals(error.message, "Invalid syntax with offending token '<EOF>' at line 3 column 1 in Foo.graphqls")
+        assertEquals(error.location?.line, 3)
+    }
+
+    @Test
+    fun `parser should include source location for field definition in named schema string`() {
+        val schema = SchemaParser.newParser()
+            .schemaString(
+                """
+                |schema {
+                |    query: Query
+                |}
+                """.trimMargin(), "Schema.graphqls")
+            .schemaString("type Query { id: ID! }", "Query.graphqls")
+            .resolvers(QueryWithIdResolver())
+            .build()
+            .makeExecutableSchema()
+
+        val sourceLocation = schema.getObjectType("Query")!!
+            .getFieldDefinition("id")
+            .definition!!.sourceLocation
+        assertNotNull(sourceLocation)
+        assertEquals(sourceLocation?.line, 1)
+        assertEquals(sourceLocation?.column, 14)
+        assertEquals(sourceLocation?.sourceName, "Query.graphqls")
+    }
+
     @Test
     fun `support enum types if only used as input type`() {
         SchemaParser.newParser()

src/test/resources/InvalidSyntax.graphqls

@@ -0,0 +1,3 @@
+type Foo {
+  bar: String!!
+}