Fixed validation error on union lists (#4331)

PascalSenn · michaelstaib · web-flow · commit 6f024640c9c8 · 2021-11-11T13:28:00.000+01:00
Co-authored-by: Michael Staib &lt;michael@chillicream.com&gt;
diff --git a/src/HotChocolate/Core/src/Validation/Rules/FieldVisitor.cs b/src/HotChocolate/Core/src/Validation/Rules/FieldVisitor.cs
@@ -127,10 +127,10 @@ protected override ISyntaxVisitorAction Enter(
             IDocumentValidatorContext context)
         {
             if (context.Types.TryPeek(out IType type) &&
-                type.NamedType().Kind == TypeKind.Union &&
+                type.NamedType() is { Kind: TypeKind.Union } unionType &&
                 HasFields(node))
             {
-                context.Errors.Add(context.UnionFieldError(node, (UnionType)type));
+                context.Errors.Add(context.UnionFieldError(node, (UnionType)unionType));
                 return Skip;
             }
 
diff --git a/src/HotChocolate/Core/test/Validation.Tests/FieldsOnCorrectTypeRule.cs b/src/HotChocolate/Core/test/Validation.Tests/FieldsOnCorrectTypeRule.cs
@@ -19,11 +19,11 @@ fragment objectFieldSelection on Dog {
                     __typename
                     name
                 }
-                
+
                 query {
                     dog {
                          ...objectFieldSelection
-                    } 
+                    }
                 }
             ");
         }
@@ -36,11 +36,11 @@ fragment aliasedObjectFieldSelection on Dog {
                     tn : __typename
                     otherName : name
                 }
-                
+
                 query {
                     dog {
                          ...aliasedObjectFieldSelection
-                    } 
+                    }
                 }
             ");
         }
@@ -52,11 +52,11 @@ public void GoodInterfacesObjectFieldSelection()
                 fragment interfaceFieldSelection on Pet {
                     otherName : name
                 }
-                
+
                 query {
                     pet {
                          ...interfaceFieldSelection
-                    } 
+                    }
                 }
             ");
         }
@@ -75,7 +75,7 @@ ... on Cat {
                 query {
                     pet {
                          ...typeKnownAgain
-                    } 
+                    }
                 }
             ");
         }
@@ -87,11 +87,11 @@ public void BadFieldNotDefinedOnFragement()
                 fragment fieldNotDefined on Dog {
                     meowVolume
                 }
-                
+
                 query {
                     dog {
                          ...fieldNotDefined
-                    } 
+                    }
                 }
             ");
         }
@@ -105,11 +105,11 @@ fragment deepFieldNotDefined on Dog {
                         deeper_unknown_field
                     }
                 }
-                
+
                 query {
                     dog {
                          ...deepFieldNotDefined
-                    } 
+                    }
                 }
             ");
         }
@@ -123,11 +123,11 @@ fragment subFieldNotDefined on Human {
                         unknown_field
                     }
                 }
-                
+
                 query {
                     human {
                          ...subFieldNotDefined
-                    } 
+                    }
                 }
             ");
         }
@@ -141,11 +141,11 @@ ... on Dog {
                         meowVolume
                     }
                 }
-                
+
                 query {
                     pet {
                          ...fieldNotDefined
-                    } 
+                    }
                 }
             ");
         }
@@ -157,11 +157,11 @@ public void BadAliasedFieldTargetNotDefined()
                 fragment aliasedFieldTargetNotDefined on Dog {
                     volume : mooVolume
                 }
-                
+
                 query {
                     dog {
                          ...aliasedFieldTargetNotDefined
-                    } 
+                    }
                 }
             ");
         }
@@ -173,11 +173,11 @@ public void BadAliasedLyingFieldTargetNotDefined()
                 fragment aliasedLyingFieldTargetNotDefined on Dog {
                     barkVolume : kawVolume
                 }
-                
+
                 query {
                     dog {
                          ...aliasedLyingFieldTargetNotDefined
-                    } 
+                    }
                 }
             ");
         }
@@ -189,11 +189,11 @@ public void BadNotDefinedOnInterface()
                 fragment notDefinedOnInterface on Pet {
                     tailLength
                 }
-                
+
                 query {
                     pet {
                          ...notDefinedOnInterface
-                    } 
+                    }
                 }
             ");
         }
@@ -205,11 +205,11 @@ public void DefinedOnImplementorsButNotOnInterface()
                 fragment definedOnImplementorsButNotInterface on Pet {
                     nickname
                 }
-                
+
                 query {
                     pet {
                          ...definedOnImplementorsButNotInterface
-                    } 
+                    }
                 }
             ");
         }
@@ -221,11 +221,11 @@ public void MetaFieldSelectionOnUnion()
                 fragment directFieldSelectionOnUnion on CatOrDog {
                   __typename
                 }
-                
+
                 query {
                     catOrDog {
                          ...directFieldSelectionOnUnion
-                    } 
+                    }
                 }
             ");
         }
@@ -237,11 +237,11 @@ public void DireftFieldSelectionOnUnion()
                 fragment directFieldSelectionOnUnion on CatOrDog {
                     directField
                 }
-                
+
                 query {
                     catOrDog {
                          ...directFieldSelectionOnUnion
-                    } 
+                    }
                 }
             ");
         }
@@ -253,11 +253,11 @@ public void DefinedOnImplementorQueriedOnUnion()
                fragment definedOnImplementorsQueriedOnUnion on CatOrDog {
                     name
                 }
-                
+
                 query {
                     catOrDog {
                          ...definedOnImplementorsQueriedOnUnion
-                    } 
+                    }
                 }
             ");
         }
@@ -274,11 +274,41 @@ ... on Dog {
                         name
                     }
                 }
-                
+
                 query {
                     pet {
                          ...objectFieldSelection
-                    } 
+                    }
+                }
+            ");
+        }
+
+        [Fact]
+        public void WrongFieldsOnUnionTypeList()
+        {
+            // arrange
+            ISchema schema = SchemaBuilder
+                .New()
+                .AddDocumentFromString(@"
+                    type Bar { baz: String }
+                    type Baz { baz: String }
+                    union Foo = Bar | Baz
+                    type Query {
+                        list: [Foo!]
+                    }")
+                .AddResolver("Query", "list", ctx => null!)
+                .AddResolver("Bar", "baz", ctx => null!)
+                .AddResolver("Baz", "baz", ctx => null!)
+                .Create();
+
+
+            ExpectErrors(
+                schema,
+                @"
+                query {
+                    list {
+                        qux
+                    }
                 }
             ");
         }
diff --git a/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/FieldsOnCorrectTypeRuleTests.DefinedOnImplementorQueriedOnUnion.snap b/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/FieldsOnCorrectTypeRuleTests.DefinedOnImplementorQueriedOnUnion.snap
@@ -22,7 +22,7 @@
       "Kind": "SelectionSet",
       "Location": {
         "Start": 73,
-        "End": 156,
+        "End": 140,
         "Line": 2,
         "Column": 73
       },
diff --git a/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/FieldsOnCorrectTypeRuleTests.DireftFieldSelectionOnUnion.snap b/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/FieldsOnCorrectTypeRuleTests.DireftFieldSelectionOnUnion.snap
@@ -22,7 +22,7 @@
       "Kind": "SelectionSet",
       "Location": {
         "Start": 66,
-        "End": 156,
+        "End": 140,
         "Line": 2,
         "Column": 66
       },
diff --git a/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/FieldsOnCorrectTypeRuleTests.WrongFieldsOnUnionTypeList.snap b/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/FieldsOnCorrectTypeRuleTests.WrongFieldsOnUnionTypeList.snap
@@ -0,0 +1,56 @@
+﻿[
+  {
+    "Message": "A union type cannot declare a field directly. Use inline fragments or fragments instead.",
+    "Code": null,
+    "Path": {
+      "Parent": null,
+      "Depth": 0,
+      "Name": "list"
+    },
+    "Locations": [
+      {
+        "Line": 3,
+        "Column": 26
+      }
+    ],
+    "Extensions": {
+      "type": "Foo",
+      "specifiedBy": "http://spec.graphql.org/June2018/#sec-Field-Selections-on-Objects-Interfaces-and-Unions-Types"
+    },
+    "Exception": null,
+    "SyntaxNode": {
+      "Kind": "SelectionSet",
+      "Location": {
+        "Start": 50,
+        "End": 119,
+        "Line": 3,
+        "Column": 26
+      },
+      "Selections": [
+        {
+          "Kind": "Field",
+          "Alias": null,
+          "Arguments": [],
+          "SelectionSet": null,
+          "Location": {
+            "Start": 76,
+            "End": 101,
+            "Line": 4,
+            "Column": 25
+          },
+          "Name": {
+            "Kind": "Name",
+            "Location": {
+              "Start": 76,
+              "End": 101,
+              "Line": 4,
+              "Column": 25
+            },
+            "Value": "qux"
+          },
+          "Directives": []
+        }
+      ]
+    }
+  }
+]

Original file line number	Diff line number	Diff line change
`@@ -127,10 +127,10 @@ protected override ISyntaxVisitorAction Enter(`
`127`	`127`	`IDocumentValidatorContext context)`
`128`	`128`	`{`
`129`	`129`	`if (context.Types.TryPeek(out IType type) &&`
`130`		`- type.NamedType().Kind == TypeKind.Union &&`
	`130`	`+ type.NamedType() is { Kind: TypeKind.Union } unionType &&`
`131`	`131`	`HasFields(node))`
`132`	`132`	`{`
`133`		`- context.Errors.Add(context.UnionFieldError(node, (UnionType)type));`
	`133`	`+ context.Errors.Add(context.UnionFieldError(node, (UnionType)unionType));`
`134`	`134`	`return Skip;`
`135`	`135`	`}`
`136`	`136`
Original file line number	Diff line number	Diff line change
`@@ -19,11 +19,11 @@ fragment objectFieldSelection on Dog {`
`19`	`19`	`__typename`
`20`	`20`	`name`
`21`	`21`	`}`
`22`		`-`
	`22`	`+`
`23`	`23`	`query {`
`24`	`24`	`dog {`
`25`	`25`	`...objectFieldSelection`
`26`		`- }`
	`26`	`+ }`
`27`	`27`	`}`
`28`	`28`	`");`
`29`	`29`	`}`
`@@ -36,11 +36,11 @@ fragment aliasedObjectFieldSelection on Dog {`
`36`	`36`	`tn : __typename`
`37`	`37`	`otherName : name`
`38`	`38`	`}`
`39`		`-`
	`39`	`+`
`40`	`40`	`query {`
`41`	`41`	`dog {`
`42`	`42`	`...aliasedObjectFieldSelection`
`43`		`- }`
	`43`	`+ }`
`44`	`44`	`}`
`45`	`45`	`");`
`46`	`46`	`}`
`@@ -52,11 +52,11 @@ public void GoodInterfacesObjectFieldSelection()`
`52`	`52`	`fragment interfaceFieldSelection on Pet {`
`53`	`53`	`otherName : name`
`54`	`54`	`}`
`55`		`-`
	`55`	`+`
`56`	`56`	`query {`
`57`	`57`	`pet {`
`58`	`58`	`...interfaceFieldSelection`
`59`		`- }`
	`59`	`+ }`
`60`	`60`	`}`
`61`	`61`	`");`
`62`	`62`	`}`
`@@ -75,7 +75,7 @@ ... on Cat {`
`75`	`75`	`query {`
`76`	`76`	`pet {`
`77`	`77`	`...typeKnownAgain`
`78`		`- }`
	`78`	`+ }`
`79`	`79`	`}`
`80`	`80`	`");`
`81`	`81`	`}`
`@@ -87,11 +87,11 @@ public void BadFieldNotDefinedOnFragement()`
`87`	`87`	`fragment fieldNotDefined on Dog {`
`88`	`88`	`meowVolume`
`89`	`89`	`}`
`90`		`-`
	`90`	`+`
`91`	`91`	`query {`
`92`	`92`	`dog {`
`93`	`93`	`...fieldNotDefined`
`94`		`- }`
	`94`	`+ }`
`95`	`95`	`}`
`96`	`96`	`");`
`97`	`97`	`}`
`@@ -105,11 +105,11 @@ fragment deepFieldNotDefined on Dog {`
`105`	`105`	`deeper_unknown_field`
`106`	`106`	`}`
`107`	`107`	`}`
`108`		`-`
	`108`	`+`
`109`	`109`	`query {`
`110`	`110`	`dog {`
`111`	`111`	`...deepFieldNotDefined`
`112`		`- }`
	`112`	`+ }`
`113`	`113`	`}`
`114`	`114`	`");`
`115`	`115`	`}`
`@@ -123,11 +123,11 @@ fragment subFieldNotDefined on Human {`
`123`	`123`	`unknown_field`
`124`	`124`	`}`
`125`	`125`	`}`
`126`		`-`
	`126`	`+`
`127`	`127`	`query {`
`128`	`128`	`human {`
`129`	`129`	`...subFieldNotDefined`
`130`		`- }`
	`130`	`+ }`
`131`	`131`	`}`
`132`	`132`	`");`
`133`	`133`	`}`
`@@ -141,11 +141,11 @@ ... on Dog {`
`141`	`141`	`meowVolume`
`142`	`142`	`}`
`143`	`143`	`}`
`144`		`-`
	`144`	`+`
`145`	`145`	`query {`
`146`	`146`	`pet {`
`147`	`147`	`...fieldNotDefined`
`148`		`- }`
	`148`	`+ }`
`149`	`149`	`}`
`150`	`150`	`");`
`151`	`151`	`}`
`@@ -157,11 +157,11 @@ public void BadAliasedFieldTargetNotDefined()`
`157`	`157`	`fragment aliasedFieldTargetNotDefined on Dog {`
`158`	`158`	`volume : mooVolume`
`159`	`159`	`}`
`160`		`-`
	`160`	`+`
`161`	`161`	`query {`
`162`	`162`	`dog {`
`163`	`163`	`...aliasedFieldTargetNotDefined`
`164`		`- }`
	`164`	`+ }`
`165`	`165`	`}`
`166`	`166`	`");`
`167`	`167`	`}`
`@@ -173,11 +173,11 @@ public void BadAliasedLyingFieldTargetNotDefined()`
`173`	`173`	`fragment aliasedLyingFieldTargetNotDefined on Dog {`
`174`	`174`	`barkVolume : kawVolume`
`175`	`175`	`}`
`176`		`-`
	`176`	`+`
`177`	`177`	`query {`
`178`	`178`	`dog {`
`179`	`179`	`...aliasedLyingFieldTargetNotDefined`
`180`		`- }`
	`180`	`+ }`
`181`	`181`	`}`
`182`	`182`	`");`
`183`	`183`	`}`
`@@ -189,11 +189,11 @@ public void BadNotDefinedOnInterface()`
`189`	`189`	`fragment notDefinedOnInterface on Pet {`
`190`	`190`	`tailLength`
`191`	`191`	`}`
`192`		`-`
	`192`	`+`
`193`	`193`	`query {`
`194`	`194`	`pet {`
`195`	`195`	`...notDefinedOnInterface`
`196`		`- }`
	`196`	`+ }`
`197`	`197`	`}`
`198`	`198`	`");`
`199`	`199`	`}`
`@@ -205,11 +205,11 @@ public void DefinedOnImplementorsButNotOnInterface()`
`205`	`205`	`fragment definedOnImplementorsButNotInterface on Pet {`
`206`	`206`	`nickname`
`207`	`207`	`}`
`208`		`-`
	`208`	`+`
`209`	`209`	`query {`
`210`	`210`	`pet {`
`211`	`211`	`...definedOnImplementorsButNotInterface`
`212`		`- }`
	`212`	`+ }`
`213`	`213`	`}`
`214`	`214`	`");`
`215`	`215`	`}`
`@@ -221,11 +221,11 @@ public void MetaFieldSelectionOnUnion()`
`221`	`221`	`fragment directFieldSelectionOnUnion on CatOrDog {`
`222`	`222`	`__typename`
`223`	`223`	`}`
`224`		`-`
	`224`	`+`
`225`	`225`	`query {`
`226`	`226`	`catOrDog {`
`227`	`227`	`...directFieldSelectionOnUnion`
`228`		`- }`
	`228`	`+ }`
`229`	`229`	`}`
`230`	`230`	`");`
`231`	`231`	`}`
`@@ -237,11 +237,11 @@ public void DireftFieldSelectionOnUnion()`
`237`	`237`	`fragment directFieldSelectionOnUnion on CatOrDog {`
`238`	`238`	`directField`
`239`	`239`	`}`
`240`		`-`
	`240`	`+`
`241`	`241`	`query {`
`242`	`242`	`catOrDog {`
`243`	`243`	`...directFieldSelectionOnUnion`
`244`		`- }`
	`244`	`+ }`
`245`	`245`	`}`
`246`	`246`	`");`
`247`	`247`	`}`
`@@ -253,11 +253,11 @@ public void DefinedOnImplementorQueriedOnUnion()`
`253`	`253`	`fragment definedOnImplementorsQueriedOnUnion on CatOrDog {`
`254`	`254`	`name`
`255`	`255`	`}`
`256`		`-`
	`256`	`+`
`257`	`257`	`query {`
`258`	`258`	`catOrDog {`
`259`	`259`	`...definedOnImplementorsQueriedOnUnion`
`260`		`- }`
	`260`	`+ }`
`261`	`261`	`}`
`262`	`262`	`");`
`263`	`263`	`}`
`@@ -274,11 +274,41 @@ ... on Dog {`
`274`	`274`	`name`
`275`	`275`	`}`
`276`	`276`	`}`
`277`		`-`
	`277`	`+`
`278`	`278`	`query {`
`279`	`279`	`pet {`
`280`	`280`	`...objectFieldSelection`
`281`		`- }`
	`281`	`+ }`
	`282`	`+ }`
	`283`	`+ ");`
	`284`	`+ }`
	`285`	`+`
	`286`	`+ [Fact]`
	`287`	`+ public void WrongFieldsOnUnionTypeList()`
	`288`	`+ {`
	`289`	`+ // arrange`
	`290`	`+ ISchema schema = SchemaBuilder`
	`291`	`+ .New()`
	`292`	`+ .AddDocumentFromString(@"`
	`293`	`+ type Bar { baz: String }`
	`294`	`+ type Baz { baz: String }`
	`295`	`+ union Foo = Bar \| Baz`
	`296`	`+ type Query {`
	`297`	`+ list: [Foo!]`
	`298`	`+ }")`
	`299`	`+ .AddResolver("Query", "list", ctx => null!)`
	`300`	`+ .AddResolver("Bar", "baz", ctx => null!)`
	`301`	`+ .AddResolver("Baz", "baz", ctx => null!)`
	`302`	`+ .Create();`
	`303`	`+`
	`304`	`+`
	`305`	`+ ExpectErrors(`
	`306`	`+ schema,`
	`307`	`+ @"`
	`308`	`+ query {`
	`309`	`+ list {`
	`310`	`+ qux`
	`311`	`+ }`
`282`	`312`	`}`
`283`	`313`	`");`
`284`	`314`	`}`