From d0a09da13ed29aee6d1600b94979e7391e9d9276 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 3 Oct 2026 18:19:16 -0400 Subject: [PATCH] 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!! +}