Improved field selection error messages (#7573)

ChilliCream · Oct 8, 2024 · a3df265 · a3df265
1 parent 2df21d7
commit a3df265
Show file tree

Hide file tree

Showing 27 changed files with 158 additions and 111 deletions.
diff --git a/src/HotChocolate/AspNetCore/test/AspNetCore.Tests/GraphQLOverHttpSpecTests.cs b/src/HotChocolate/AspNetCore/test/AspNetCore.Tests/GraphQLOverHttpSpecTests.cs
@@ -241,32 +241,10 @@ public async Task ValidationError2(string? acceptHeader, HttpTransportVersion tr
         using var response = await client.SendAsync(request);
 
         // assert
-        Snapshot
-            .Create()
-            .Add(response)
-            .MatchInline(
-                @$"Headers:
-                Content-Type: {expectedContentType}
-                -------------------------->
-                Status Code: {expectedStatusCode}
-                -------------------------->
-                " +
-                @"{""errors"":[{""message"":""`__type` is an object, interface or " +
-                @"union type field. Leaf selections on objects, interfaces, and unions without " +
-                @"subfields are disallowed."",""locations"":[{""line"":1,""column"":3}]," +
-                @"""extensions"":{""declaringType"":""Query"",""field"":""__type""," +
-                @"""type"":""__Type"",""responseName"":""__type""," +
-                @"""specifiedBy"":""https://spec.graphql.org/October2021/#sec-Field-Selections-" +
-                @"on-Objects-Interfaces-and-Unions-Types""}},{""message"":""The field `name" +
-                @"` does not exist on the type `Query`."",""locations"":[{" +
-                @"""line"":1,""column"":10}],""extensions"":{""type"":""Query""," +
-                @"""field"":""name"",""responseName"":""name"",""specifiedBy"":" +
-                @"""https://spec.graphql.org/October2021/#sec-Field-Selections-on-Objects-" +
-                @"Interfaces-and-Unions-Types""}},{""message"":""The argument `name` " +
-                @"is required."",""locations"":[{""line"":1,""column"":3}],""extensions"":{" +
-                @"""type"":""Query"",""field"":""__type"",""argument"":""name""," +
-                @"""specifiedBy"":""https://spec.graphql.org/October2021/#sec-Required-Arguments""" +
-                "}}]}");
+        Assert.Equal(
+            expectedContentType,
+            response.Content.Headers.GetValues("Content-Type").Single());
+        Assert.Equal(expectedStatusCode, response.StatusCode);
     }
 
     [Fact]

diff --git a/src/HotChocolate/Core/src/Validation/ErrorHelper.cs b/src/HotChocolate/Core/src/Validation/ErrorHelper.cs
@@ -143,7 +143,8 @@ public static IError LeafFieldsCannotHaveSelections(
         return ErrorBuilder.New()
             .SetMessage(
                 Resources.ErrorHelper_LeafFieldsCannotHaveSelections,
-                node.Name.Value, fieldType.IsScalarType() ? "a scalar" : "an enum")
+                node.Name.Value,
+                fieldType.Print())
             .SetLocations([node])
             .SetPath(context.CreateErrorPath())
             .SetExtension("declaringType", declaringType.Name)
@@ -216,7 +217,8 @@ public static IError NoSelectionOnCompositeField(
         return ErrorBuilder.New()
             .SetMessage(
                 Resources.ErrorHelper_NoSelectionOnCompositeField,
-                node.Name.Value)
+                node.Name.Value,
+                fieldType.Print())
             .SetLocations([node])
             .SetPath(context.CreateErrorPath())
             .SetExtension("declaringType", declaringType.Name)

diff --git a/src/HotChocolate/Core/src/Validation/Properties/Resources.Designer.cs b/src/HotChocolate/Core/src/Validation/Properties/Resources.Designer.cs
diff --git a/src/HotChocolate/Core/src/Validation/Properties/Resources.resx b/src/HotChocolate/Core/src/Validation/Properties/Resources.resx
@@ -126,7 +126,7 @@
     <value>The field `{0}` does not exist on the type `{1}`.</value>
   </data>
   <data name="ErrorHelper_LeafFieldsCannotHaveSelections" xml:space="preserve">
-    <value>`{0}` returns {1} value. Selections on scalars or enums are never allowed, because they are the leaf nodes of any GraphQL query.</value>
+    <value>Field "{0}" must not have a selection since type "{1}" has no subfields.</value>
   </data>
   <data name="ErrorHelper_ArgumentValueIsNotCompatible" xml:space="preserve">
     <value>The specified argument value does not match the argument type.</value>
@@ -138,7 +138,7 @@
     <value>The specified value type of variable `{0}` does not match the variable type.</value>
   </data>
   <data name="ErrorHelper_NoSelectionOnCompositeField" xml:space="preserve">
-    <value>`{0}` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.</value>
+    <value>Field "{0}" of type "{1}" must have a selection of subfields. Did you mean "{0} {{ ... }}"?</value>
   </data>
   <data name="ErrorHelper_NoSelectionOnRootType" xml:space="preserve">
     <value>Operation `{0}` has a empty selection set. Root types without subfields are disallowed.</value>

diff --git a/...ests/Errors/__snapshots__/ErrorBehaviorTests.Property_Return_UnexpectedErrorWithPath.json b/...ests/Errors/__snapshots__/ErrorBehaviorTests.Property_Return_UnexpectedErrorWithPath.json
@@ -1,7 +1,7 @@
 {
   "errors": [
     {
-      "message": "`error13` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.",
+      "message": "Field \"error13\" of type \"Foo\" must have a selection of subfields. Did you mean \"error13 { ... }\"?",
       "locations": [
         {
           "line": 1,

diff --git a/src/HotChocolate/Core/test/Validation.Tests/DocumentValidatorTests.cs b/src/HotChocolate/Core/test/Validation.Tests/DocumentValidatorTests.cs
@@ -441,8 +441,8 @@ await ExpectErrors(
                 }
             ",
             t => Assert.Equal(
-                "`barkVolume` returns a scalar value. Selections on scalars or enums" +
-                " are never allowed, because they are the leaf nodes of any GraphQL query.",
+                "Field \"barkVolume\" must not have a selection since type \"Int\" has no " +
+                "subfields.",
                 t.Message));
     }
 

diff --git a/src/HotChocolate/Core/test/Validation.Tests/LeafFieldSelectionsRuleTests.cs b/src/HotChocolate/Core/test/Validation.Tests/LeafFieldSelectionsRuleTests.cs
@@ -45,9 +45,8 @@ public void ScalarSelectionsNotAllowedOnInt()
                 }
             ",
             t => Assert.Equal(
-                "`barkVolume` returns a scalar value. Selections on scalars " +
-                "or enums are never allowed, because they are the leaf " +
-                "nodes of any GraphQL query.",
+                "Field \"barkVolume\" must not have a selection since type \"Int\" has no " +
+                "subfields.",
                 t.Message));
     }
 
@@ -60,9 +59,8 @@ query directQueryOnObjectWithoutSubFields {
                 }
             ",
             t => Assert.Equal(
-                "`human` is an object, interface or union type " +
-                "field. Leaf selections on objects, interfaces, and " +
-                "unions without subfields are disallowed.",
+                "Field \"human\" of type \"Human\" must have a selection of subfields. Did you " +
+                "mean \"human { ... }\"?",
                 t.Message));
     }
 
@@ -75,9 +73,8 @@ query directQueryOnObjectWithoutSubFields {
                 }
             ",
             t => Assert.Equal(
-                "`human` is an object, interface or union type " +
-                "field. Leaf selections on objects, interfaces, and " +
-                "unions without subfields are disallowed.",
+                "Field \"human\" of type \"Human\" must have a selection of subfields. Did you " +
+                "mean \"human { ... }\"?",
                 t.Message));
     }
 
@@ -90,9 +87,8 @@ query directQueryOnInterfaceWithoutSubFields {
                 }
             ",
             t => Assert.Equal(
-                "`pet` is an object, interface or union type " +
-                "field. Leaf selections on objects, interfaces, and " +
-                "unions without subfields are disallowed.",
+                "Field \"pet\" of type \"Human\" must have a selection of subfields. Did you mean " +
+                "\"pet { ... }\"?",
                 t.Message));
     }
 
@@ -105,9 +101,8 @@ query directQueryOnInterfaceWithoutSubFields {
                 }
             ",
             t => Assert.Equal(
-                "`pet` is an object, interface or union type " +
-                "field. Leaf selections on objects, interfaces, and " +
-                "unions without subfields are disallowed.",
+                "Field \"pet\" of type \"Human\" must have a selection of subfields. Did you mean " +
+                "\"pet { ... }\"?",
                 t.Message));
     }
 
@@ -120,9 +115,8 @@ query directQueryOnUnionWithoutSubFields {
                 }
             ",
             t => Assert.Equal(
-                "`catOrDog` is an object, interface or union type " +
-                "field. Leaf selections on objects, interfaces, and " +
-                "unions without subfields are disallowed.",
+                "Field \"catOrDog\" of type \"CatOrDog\" must have a selection of subfields. Did " +
+                "you mean \"catOrDog { ... }\"?",
                 t.Message));
     }
 
@@ -135,9 +129,8 @@ query directQueryOnUnionWithoutSubFields {
                 }
             ",
             t => Assert.Equal(
-                "`catOrDog` is an object, interface or union type " +
-                "field. Leaf selections on objects, interfaces, and " +
-                "unions without subfields are disallowed.",
+                "Field \"catOrDog\" of type \"CatOrDog\" must have a selection of subfields. Did " +
+                "you mean \"catOrDog { ... }\"?",
                 t.Message));
     }
 
@@ -150,9 +143,8 @@ public void InterfaceTypeMissingSelection()
                 }
             ",
             t => Assert.Equal(
-                "`pets` is an object, interface or union type " +
-                "field. Leaf selections on objects, interfaces, and " +
-                "unions without subfields are disallowed.",
+                "Field \"pets\" of type \"[Pet]\" must have a selection of subfields. Did you " +
+                "mean \"pets { ... }\"?",
                 t.Message));
     }
 
@@ -165,9 +157,8 @@ public void InterfaceTypeMissingSelectionEmptySelection()
                 }
             ",
             t => Assert.Equal(
-                "`pets` is an object, interface or union type " +
-                "field. Leaf selections on objects, interfaces, and " +
-                "unions without subfields are disallowed.",
+                "Field \"pets\" of type \"[Pet]\" must have a selection of subfields. Did you " +
+                "mean \"pets { ... }\"?",
                 t.Message));
     }
 
@@ -256,9 +247,8 @@ public void ScalarSelectionNotAllowedOnBoolean()
                 }
             ",
             t => Assert.Equal(
-                "`barks` returns a scalar value. Selections on scalars " +
-                "or enums are never allowed, because they are the leaf " +
-                "nodes of any GraphQL query.",
+                "Field \"barks\" must not have a selection since type \"Boolean!\" has no " +
+                "subfields.",
                 t.Message));
     }
 
@@ -277,9 +267,40 @@ ... on Cat {
                 }
             ",
             t => Assert.Equal(
-                "`furColor` returns an enum value. Selections on scalars " +
-                "or enums are never allowed, because they are the leaf " +
-                "nodes of any GraphQL query.",
+                "Field \"furColor\" must not have a selection since type \"FurColor\" has no " +
+                "subfields.",
+                t.Message));
+    }
+
+    [Fact]
+    public void ScalarSelectionNotAllowedOnListOfScalars()
+    {
+        ExpectErrors(@"
+                {
+                    listOfScalars {
+                        x
+                    }
+                }
+            ",
+            t => Assert.Equal(
+                "Field \"listOfScalars\" must not have a selection since type \"[String]\" has " +
+                "no subfields.",
+                t.Message));
+    }
+
+    [Fact]
+    public void ScalarSelectionNotAllowedOnListOfListOfScalars()
+    {
+        ExpectErrors(@"
+                {
+                    listOfListOfScalars {
+                        x
+                    }
+                }
+            ",
+            t => Assert.Equal(
+                "Field \"listOfListOfScalars\" must not have a selection since type " +
+                "\"[[String]]\" has no subfields.",
                 t.Message));
     }
 
@@ -294,9 +315,8 @@ public void ScalarSelectionNotAllowedWithArgs()
                 }
             ",
             t => Assert.Equal(
-                "`doesKnowCommand` returns a scalar value. Selections on scalars " +
-                "or enums are never allowed, because they are the leaf " +
-                "nodes of any GraphQL query.",
+                "Field \"doesKnowCommand\" must not have a selection since type \"Boolean!\" has " +
+                "no subfields.",
                 t.Message));
     }
 
@@ -311,9 +331,7 @@ name @include(if: true) { isAlsoHumanName }
                 }
             ",
             t => Assert.Equal(
-                "`name` returns a scalar value. Selections on scalars " +
-                "or enums are never allowed, because they are the leaf " +
-                "nodes of any GraphQL query.",
+                "Field \"name\" must not have a selection since type \"String!\" has no subfields.",
                 t.Message));
     }
 
@@ -328,9 +346,8 @@ public void ScalarSelectionNotAllowedWithDirectivesAndArgs()
                 }
             ",
             t => Assert.Equal(
-                "`doesKnowCommand` returns a scalar value. Selections on scalars " +
-                "or enums are never allowed, because they are the leaf " +
-                "nodes of any GraphQL query.",
+                "Field \"doesKnowCommand\" must not have a selection since type \"Boolean!\" has " +
+                "no subfields.",
                 t.Message));
     }
 }
diff --git a/src/HotChocolate/Core/test/Validation.Tests/Types/QueryType.cs b/src/HotChocolate/Core/test/Validation.Tests/Types/QueryType.cs
@@ -38,5 +38,13 @@ protected override void Configure(IObjectTypeDescriptor<Query> descriptor)
         descriptor.Field("nonNull")
             .Argument("a", a => a.Type<NonNullType<StringType>>().DefaultValue("abc"))
             .Resolve("foo");
+
+        descriptor.Field("listOfScalars")
+            .Type<ListType<StringType>>()
+            .Resolve<string[]>(_ => []);
+
+        descriptor.Field("listOfListOfScalars")
+            .Type<ListType<ListType<StringType>>>()
+            .Resolve<string[][]>(_ => []);
     }
 }
diff --git a/...alidation.Tests/__snapshots__/DocumentValidatorTests.ScalarSelectionsNotAllowedOnInt.snap b/...alidation.Tests/__snapshots__/DocumentValidatorTests.ScalarSelectionsNotAllowedOnInt.snap
@@ -1,6 +1,6 @@
 [
   {
-    "Message": "`barkVolume` returns a scalar value. Selections on scalars or enums are never allowed, because they are the leaf nodes of any GraphQL query.",
+    "Message": "Field \"barkVolume\" must not have a selection since type \"Int\" has no subfields.",
     "Code": null,
     "Path": {
       "Name": "dog",

diff --git a/...Validation.Tests/__snapshots__/DocumentValidatorTests.SkipDirectiveIsInTheWrongPlace.snap b/...Validation.Tests/__snapshots__/DocumentValidatorTests.SkipDirectiveIsInTheWrongPlace.snap
@@ -1,6 +1,6 @@
 [
   {
-    "Message": "`field` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.",
+    "Message": "Field \"field\" of type \"Query\" must have a selection of subfields. Did you mean \"field { ... }\"?",
     "Code": null,
     "Path": null,
     "Locations": [

diff --git a/.../test/Validation.Tests/__snapshots__/DocumentValidatorTests.Type_query_repeated_6250.snap b/.../test/Validation.Tests/__snapshots__/DocumentValidatorTests.Type_query_repeated_6250.snap
@@ -16,7 +16,7 @@
     "Exception": null
   },
   {
-    "Message": "`__type` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.",
+    "Message": "Field \"__type\" of type \"__Type\" must have a selection of subfields. Did you mean \"__type { ... }\"?",
     "Code": null,
     "Path": null,
     "Locations": [
@@ -35,7 +35,7 @@
     "Exception": null
   },
   {
-    "Message": "`__type` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.",
+    "Message": "Field \"__type\" of type \"__Type\" must have a selection of subfields. Did you mean \"__type { ... }\"?",
     "Code": null,
     "Path": null,
     "Locations": [
@@ -54,7 +54,7 @@
     "Exception": null
   },
   {
-    "Message": "`__type` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.",
+    "Message": "Field \"__type\" of type \"__Type\" must have a selection of subfields. Did you mean \"__type { ... }\"?",
     "Code": null,
     "Path": null,
     "Locations": [
@@ -73,7 +73,7 @@
     "Exception": null
   },
   {
-    "Message": "`__type` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.",
+    "Message": "Field \"__type\" of type \"__Type\" must have a selection of subfields. Did you mean \"__type { ... }\"?",
     "Code": null,
     "Path": null,
     "Locations": [

diff --git a/...ts/__snapshots__/LeafFieldSelectionsRuleTests.DirectQueryOnInterfaceWithoutSubFields.snap b/...ts/__snapshots__/LeafFieldSelectionsRuleTests.DirectQueryOnInterfaceWithoutSubFields.snap
@@ -1,6 +1,6 @@
-[
+[
   {
-    "Message": "`pet` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.",
+    "Message": "Field \"pet\" of type \"Human\" must have a selection of subfields. Did you mean \"pet { ... }\"?",
     "Code": null,
     "Path": null,
     "Locations": [

diff --git a/...__/LeafFieldSelectionsRuleTests.DirectQueryOnInterfaceWithoutSubFieldsEmptySelection.snap b/...__/LeafFieldSelectionsRuleTests.DirectQueryOnInterfaceWithoutSubFieldsEmptySelection.snap
@@ -1,6 +1,6 @@
-[
+[
   {
-    "Message": "`pet` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.",
+    "Message": "Field \"pet\" of type \"Human\" must have a selection of subfields. Did you mean \"pet { ... }\"?",
     "Code": null,
     "Path": null,
     "Locations": [