Merge pull request #838 from graphql-java-kickstart/bugfix/392

Fix line numbers in schema syntax errors and name the source

Author: oryan-block

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.NumberFormat
 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<Source>()
     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(Source(string, 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(Source(string, 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(Source(readFile(it), it))) }.toMutableList()
 
-            if (schemaString.isNotBlank()) {
-                documents.add(parseDocument(schemaString.toString()))
+            if (schemaStrings.any { it.text.isNotBlank() }) {
+                documents.add(parseDocument(schemaStrings))
             }
 
             return documents
@@ -189,16 +196,37 @@ class SchemaParserBuilder {
         }
     }
 
-    private fun parseDocument(input: String, sourceName: String? = null): Document {
-        val sourceReader = MultiSourceReader
-            .newMultiSourceReader()
-            .string(input, sourceName)
+    private fun parseDocument(sources: List<Source>): Document {
+        // graphql-java's 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 on that line, see syntaxErrorInSource.
+        val separated = if (sources.size > 1) sources.map { it.endingWithLineBreak() } else sources
+        val sourceReader = MultiSourceReader.newMultiSourceReader()
+            .apply { separated.forEach { string(it.text, it.name) } }
             .trackData(true).build()
         val environment = ParserEnvironment
             .newParserEnvironment()
             .document(sourceReader)
             .parserOptions(parserOptions).build()
-        return parser.parseDocument(environment)
+        try {
+            return parser.parseDocument(environment)
+        } catch (e: InvalidSyntaxException) {
+            throw syntaxErrorInSource(e, separated.last())
+        }
+    }
+
+    /**
+     * Makes the line of an unexpected end of input relative to the last source and adds the source name to the message.
+     */
+    private fun syntaxErrorInSource(e: InvalidSyntaxException, lastSource: Source): InvalidSyntaxException {
+        val location = e.location ?: return e
+        val line = if (e.offendingToken == "<EOF>") lastSource.text.lines().size else location.line
+        if (line == location.line && location.sourceName == null) return e
+
+        // graphql-java formats the line for the default locale, e.g. "1,103"
+        val number = NumberFormat.getInstance()
+        val message = e.message.orEmpty().replaceFirst(" ${number.format(location.line)} ", " ${number.format(line)} ")
+        val inSource = location.sourceName?.let { " in $it" }.orEmpty()
+        return SchemaSyntaxException(message + inSource, SourceLocation(line, location.column, location.sourceName), e)
     }
 
     private fun readFile(filename: String) =
@@ -209,6 +237,10 @@ class SchemaParserBuilder {
      * Build the parser with the supplied schema and dictionary.
      */
     fun build() = SchemaParser(scan(), options, runtimeWiringBuilder.build())
+
+    private data class Source(val text: String, val name: String?) {
+        fun endingWithLineBreak() = if (text.endsWith("\n")) this else copy(text = "$text\n")
+    }
 }
 
 class InvalidSchemaError(
@@ -218,3 +250,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,73 @@ 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!! }")
+                .build()
+        }
+
+        assertEquals(error.message, "Invalid syntax with offending token '!' at line 1 column 24")
+        assertEquals(error.location?.line, 1)
+    }
+
+    @Test
+    fun `parser should include file name in syntax error`() {
+        val error = assertThrows(InvalidSyntaxException::class.java) {
+            SchemaParser.newParser()
+                .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 report unexpected end of the last schema string relative to it`() {
+        val error = assertThrows(InvalidSyntaxException::class.java) {
+            SchemaParser.newParser()
+                // over 1000 lines, so graphql-java's message groups the thousands in the overall line
+                .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!!
+}