From d0a09da13ed29aee6d1600b94979e7391e9d9276 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 3 Oct 2026 18:19:16 -0400 Subject: [PATCH 1/2] Report schema syntax errors per source with its name SchemaParserBuilder appended every schemaString to one StringBuilder and parsed the result as a single unnamed source. A syntax error in the second or later string was reported with a line number counted across all strings, and nothing said which string it came from. This is what Spring Boot starter users hit, since the starter passes each .graphqls file as its own schemaString. Files added with file() were already parsed with their name, but graphql-java's InvalidSyntaxException message only contains the line and column, so the name was only visible on the exception's SourceLocation. Each schemaString is now its own source in one MultiSourceReader, so they are still parsed as a single document, but line numbers in parse errors and AST source locations are relative to the string they came from. graphql-java's MultiSourceReader numbers the last line of the last source from the start of the first source, so every source is ended with a line break, which leaves only the end of input on that line, and the line of an unexpected end of input error is made relative to the last source before it is reported. A new schemaString(string, sourceName) overload names the source, and when a syntax error comes from a named source or file the exception is rethrown as an InvalidSyntaxException subclass whose message ends with "in ". Spring Boot starter users get line numbers relative to each file from this change alone, but file names only once graphql-spring-boot passes them to the new overload. Fixes #392 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../kickstart/tools/SchemaParserBuilder.kt | 55 ++++++-- .../kickstart/tools/SchemaParserTest.kt | 132 ++++++++++++++++++ src/test/resources/InvalidSyntax.graphqls | 3 + 3 files changed, 176 insertions(+), 14 deletions(-) create mode 100644 src/test/resources/InvalidSyntax.graphqls diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserBuilder.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserBuilder.kt index 2e8365f9..1b88fc43 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserBuilder.kt +++ b/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>() private val files = mutableListOf() private val resolvers = mutableListOf>() private val scalars = mutableListOf() @@ -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 { 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>): 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(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) diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index e5e2ba30..055a9682 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/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 '' 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 '' 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() diff --git a/src/test/resources/InvalidSyntax.graphqls b/src/test/resources/InvalidSyntax.graphqls new file mode 100644 index 00000000..913550ee --- /dev/null +++ b/src/test/resources/InvalidSyntax.graphqls @@ -0,0 +1,3 @@ +type Foo { + bar: String!! +} From 6f63923a4421f711fa06ae033044738998558ccc Mon Sep 17 00:00:00 2001 From: Oryan Date: Fri, 16 Oct 2026 13:58:40 -0400 Subject: [PATCH 2/2] Simplify the per-source syntax error handling - Keep schema strings as a named Source instead of a Pair - Move the syntax error rewrite out of parseDocument into syntaxErrorInSource - Find an unexpected end of input by its '' token instead of comparing line counts. It is always on the last line of the last source, so its line is that source's line count - Format the line with NumberFormat, which is what graphql-java's MessageFormat uses for numbers - Fold the seven new tests into four that each cover a separate case Co-Authored-By: Claude Opus 5.5 --- .../kickstart/tools/SchemaParserBuilder.kt | 56 ++++++++------- .../kickstart/tools/SchemaParserTest.kt | 72 ++----------------- 2 files changed, 36 insertions(+), 92 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserBuilder.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserBuilder.kt index 1b88fc43..18a0ed38 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserBuilder.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserBuilder.kt @@ -13,7 +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 java.text.NumberFormat import kotlin.Int.Companion.MAX_VALUE import kotlin.reflect.KClass @@ -23,7 +23,7 @@ import kotlin.reflect.KClass class SchemaParserBuilder { private val dictionary = SchemaParserDictionary() - private val schemaStrings = mutableListOf>() + private val schemaStrings = mutableListOf() private val files = mutableListOf() private val resolvers = mutableListOf>() private val scalars = mutableListOf() @@ -52,14 +52,14 @@ class SchemaParserBuilder { * Add a GraphQL schema string directly. */ fun schemaString(string: String) = this.apply { - schemaStrings.add(string to null) + 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(string to sourceName) + schemaStrings.add(Source(string, sourceName)) } /** @@ -179,9 +179,9 @@ class SchemaParserBuilder { private fun parseDocuments(): List { try { - val documents = files.map { parseDocument(listOf(readFile(it) to it)) }.toMutableList() + val documents = files.map { parseDocument(listOf(Source(readFile(it), it))) }.toMutableList() - if (schemaStrings.any { it.first.isNotBlank() }) { + if (schemaStrings.any { it.text.isNotBlank() }) { documents.add(parseDocument(schemaStrings)) } @@ -196,13 +196,13 @@ class SchemaParserBuilder { } } - private fun parseDocument(sources: List>): 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() + private fun parseDocument(sources: List): 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) @@ -210,20 +210,24 @@ class SchemaParserBuilder { 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) + throw syntaxErrorInSource(e, separated.last()) } } - // 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(line)) + /** + * 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 == "") 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) = this::class.java.classLoader.getResource(filename)?.readText() @@ -233,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( diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index 055a9682..3987d9c6 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt @@ -305,24 +305,18 @@ class SchemaParserTest { | id: ID! |} """.trimMargin()) - .schemaString( - """ - |type Foo { - | bar: String!! - |} - """.trimMargin()) + .schemaString("type Foo { bar: String!! }") .build() } - assertEquals(error.message, "Invalid syntax with offending token '!' at line 2 column 17") - assertEquals(error.location?.line, 2) + 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("Test.graphqls") .file("InvalidSyntax.graphqls") .build() } @@ -331,69 +325,11 @@ class SchemaParserTest { 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 '' 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() + // 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()