From 69569de374a0699b4eb9448bd9c846f5620c3f01 Mon Sep 17 00:00:00 2001
From: dondonz <13839920+dondonz@users.noreply.github.com>
Date: Sun, 24 Nov 2024 14:51:40 +1100
Subject: [PATCH] Filter out negative line and column locations from
 toSpecification

---
 src/main/java/graphql/GraphqlErrorHelper.java | 18 ++++++++----
 .../groovy/graphql/GraphQLErrorTest.groovy    | 28 +++++++++++++++++--
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/main/java/graphql/GraphqlErrorHelper.java b/src/main/java/graphql/GraphqlErrorHelper.java
index 5b2aaa1342..c422a18436 100644
--- a/src/main/java/graphql/GraphqlErrorHelper.java
+++ b/src/main/java/graphql/GraphqlErrorHelper.java
@@ -7,7 +7,7 @@
 import java.util.Map;
 import java.util.Objects;
 
-import static graphql.collect.ImmutableKit.map;
+import static graphql.collect.ImmutableKit.mapAndDropNulls;
 
 /**
  * This little helper allows GraphQlErrors to implement
@@ -51,14 +51,20 @@ public static Map<String, Object> toSpecification(GraphQLError error) {
     }
 
     public static Object locations(List<SourceLocation> locations) {
-        return map(locations, GraphqlErrorHelper::location);
+        return mapAndDropNulls(locations, GraphqlErrorHelper::location);
     }
 
+    /**
+     *  Positive integers starting from 1 required for error locations,
+     *  from the spec <a href="https://spec.graphql.org/draft/#sec-Errors.Error-Result-Format">...</a>
+     */
     public static Object location(SourceLocation location) {
-        Map<String, Integer> map = new LinkedHashMap<>();
-        map.put("line", location.getLine());
-        map.put("column", location.getColumn());
-        return map;
+        int line = location.getLine();
+        int column = location.getColumn();
+        if (line < 1 || column < 1) {
+            return null;
+        }
+        return Map.of("line", line, "column", column);
     }
 
     public static int hashCode(GraphQLError dis) {
diff --git a/src/test/groovy/graphql/GraphQLErrorTest.groovy b/src/test/groovy/graphql/GraphQLErrorTest.groovy
index ca9fc1e8d7..f9c2a2358f 100644
--- a/src/test/groovy/graphql/GraphQLErrorTest.groovy
+++ b/src/test/groovy/graphql/GraphQLErrorTest.groovy
@@ -31,7 +31,7 @@ class GraphQLErrorTest extends Specification {
                 .description("Test ValidationError")
                 .build()                                                                               |
                 [
-                        locations: [[line: 666, column: 999], [line: 333, column: 0]],
+                        locations: [[line: 666, column: 999], [line: 333, column: 1]],
                         message  : "Test ValidationError",
                         extensions:[classification:"ValidationError"],
                 ]
@@ -44,7 +44,7 @@ class GraphQLErrorTest extends Specification {
 
         new InvalidSyntaxError(mkLocations(), "Not good syntax m'kay")                                 |
                 [
-                        locations: [[line: 666, column: 999], [line: 333, column: 0]],
+                        locations: [[line: 666, column: 999], [line: 333, column: 1]],
                         message  : "Not good syntax m'kay",
                         extensions:[classification:"InvalidSyntax"],
                 ]
@@ -72,6 +72,28 @@ class GraphQLErrorTest extends Specification {
 
     }
 
+    def "toSpecification filters out error locations with line and column not starting at 1, as required in spec"() {
+        // See specification wording: https://spec.graphql.org/draft/#sec-Errors.Error-Result-Format
+
+        given:
+        def error = ValidationError.newValidationError()
+                .validationErrorType(ValidationErrorType.UnknownType)
+                .sourceLocations([mkLocation(-1, -1), mkLocation(333, 1)])
+                .description("Test ValidationError")
+                .build()
+
+        def expectedMap = [
+                locations: [
+                        [line: 333, column: 1]
+                ],
+                message: "Test ValidationError",
+                extensions: [classification:"ValidationError"]
+        ]
+
+        expect:
+        error.toSpecification() == expectedMap
+    }
+
     class CustomException extends RuntimeException implements GraphQLError {
         private LinkedHashMap<String, String> map
 
@@ -110,7 +132,7 @@ class GraphQLErrorTest extends Specification {
     }
 
     List<SourceLocation> mkLocations() {
-        return [mkLocation(666, 999), mkLocation(333, 0)]
+        return [mkLocation(666, 999), mkLocation(333, 1)]
     }
 
     SourceLocation mkLocation(int line, int column) {