Ensures that FieldResult<T> has errors declared. (#7612)

Author: michaelstaib

src/HotChocolate/Core/src/Types/Configuration/Validation/EnsureFieldResultsDeclareErrorsRule.cs

@@ -0,0 +1,72 @@
+using HotChocolate.Types;
+using HotChocolate.Types.Descriptors;
+using HotChocolate.Utilities;
+
+namespace HotChocolate.Configuration.Validation;
+
+internal sealed class EnsureFieldResultsDeclareErrorsRule : ISchemaValidationRule
+{
+    private const string _errorKey = "HotChocolate.Types.Errors.ErrorDefinitions";
+
+    public void Validate(
+        IDescriptorContext context,
+        ISchema schema,
+        ICollection<ISchemaError> errors)
+    {
+        var mutationType = schema.MutationType;
+
+        foreach (var objectType in schema.Types.OfType<ObjectType>())
+        {
+            if (ReferenceEquals(objectType, mutationType))
+            {
+                continue;
+            }
+
+            foreach (var field in objectType.Fields.AsSpan())
+            {
+                var member = field.ResolverMember ?? field.Member;
+                if (member is not null)
+                {
+                    var returnType = member.GetReturnType();
+                    if (returnType is not null
+                        && returnType.IsGenericType
+                        && returnType.GenericTypeArguments.Length == 1)
+                    {
+                        var typeDefinition = returnType.GetGenericTypeDefinition();
+
+                        if (typeDefinition == typeof(FieldResult<>))
+                        {
+                            EnsureErrorsAreDefined(field, errors);
+                        }
+                        else if (typeDefinition == typeof(ValueTask<>) || typeDefinition == typeof(Task<>))
+                        {
+                            var type = returnType.GenericTypeArguments[0];
+                            if (type.IsGenericType
+                                && type.GenericTypeArguments.Length == 1
+                                && type.GetGenericTypeDefinition() == typeof(FieldResult<>))
+                            {
+                                EnsureErrorsAreDefined(field, errors);
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    private static void EnsureErrorsAreDefined(
+        ObjectField field,
+        ICollection<ISchemaError> errors)
+    {
+        if (!field.ContextData.ContainsKey(_errorKey))
+        {
+            errors.Add(
+                SchemaErrorBuilder.New()
+                    .SetMessage(
+                        "The field `{0}` must declare errors to use a FieldResult<T>.",
+                        field.Coordinate)
+                    .SetTypeSystemObject(field.DeclaringType)
+                    .Build());
+        }
+    }
+}

src/HotChocolate/Core/src/Types/Configuration/Validation/SchemaValidator.cs

@@ -12,6 +12,7 @@ internal static class SchemaValidator
         new DirectiveValidationRule(),
         new InterfaceHasAtLeastOneImplementationRule(),
         new IsSelectedPatternValidation(),
+        new EnsureFieldResultsDeclareErrorsRule()
     ];
 
     public static IReadOnlyList<ISchemaError> Validate(

src/HotChocolate/Core/test/Types.Queries.Tests/CodeFirstSchemaTests.cs

@@ -354,6 +354,78 @@ async Task Error() =>
         exception.Errors[0].Message.MatchSnapshot();
     }
 
+    [Fact]
+    public async Task Throw_SchemaError_When_FieldResult_Has_No_Errors()
+    {
+        async Task Error()
+            => await new ServiceCollection()
+                .AddGraphQL()
+                .AddQueryConventions()
+                .AddQueryType<InvalidQuery>()
+                .BuildSchemaAsync();
+
+        var exception = await Assert.ThrowsAsync<SchemaException>(Error);
+        Assert.Single(exception.Errors);
+        exception.Errors[0].Message.MatchSnapshot();
+    }
+
+    [Fact]
+    public async Task Throw_SchemaError_When_FieldResult_Has_No_Errors_1()
+    {
+        async Task Error()
+            => await new ServiceCollection()
+                .AddGraphQL()
+                .AddQueryType<InvalidQuery>()
+                .BuildSchemaAsync();
+
+        var exception = await Assert.ThrowsAsync<SchemaException>(Error);
+        Assert.Single(exception.Errors);
+        exception.Errors[0].Message.MatchSnapshot();
+    }
+
+    [Fact]
+    public async Task Throw_SchemaError_When_FieldResult_Has_No_Errors_2()
+    {
+        async Task Error()
+            => await new ServiceCollection()
+                .AddGraphQL()
+                .AddQueryConventions()
+                .AddQueryType<InvalidQueryTask>()
+                .BuildSchemaAsync();
+
+        var exception = await Assert.ThrowsAsync<SchemaException>(Error);
+        Assert.Single(exception.Errors);
+        exception.Errors[0].Message.MatchSnapshot();
+    }
+
+    [Fact]
+    public async Task Throw_SchemaError_When_FieldResult_Has_No_Errors_3()
+    {
+        async Task Error()
+            => await new ServiceCollection()
+                .AddGraphQL()
+                .AddQueryConventions()
+                .AddQueryType<InvalidQueryValueTask>()
+                .BuildSchemaAsync();
+
+        var exception = await Assert.ThrowsAsync<SchemaException>(Error);
+        Assert.Single(exception.Errors);
+        exception.Errors[0].Message.MatchSnapshot();
+    }
+
+    [Fact]
+    public async Task FieldResult_With_Errors_Are_Valid()
+    {
+        var schema =
+            await new ServiceCollection()
+                .AddGraphQL()
+                .AddQueryConventions()
+                .AddQueryType<ValidQueryValueTask>()
+                .BuildSchemaAsync();
+
+        schema.MatchSnapshot();
+    }
+
     public class QueryWithFieldResultType : ObjectType
     {
         protected override void Configure(IObjectTypeDescriptor descriptor)
@@ -507,4 +579,30 @@ public sealed record AddressNotFound(string Id, string Message);
     public sealed class UserNotFoundException : Exception;
 
     public sealed class InvalidUserIdException : Exception;
+
+    public class InvalidQuery
+    {
+        public FieldResult<Foo> Foo() => default!;
+    }
+
+    public class InvalidQueryTask
+    {
+        public Task<FieldResult<Foo>> Foo() => default!;
+    }
+
+    public class InvalidQueryValueTask
+    {
+        public Task<FieldResult<Foo>> Foo() => default!;
+    }
+
+    public class ValidQueryValueTask
+    {
+        [Error<ArgumentException>]
+        public Task<FieldResult<Foo>> Foo() => default!;
+    }
+
+    public class Foo
+    {
+        public string Bar => default!;
+    }
 }

src/HotChocolate/Core/test/Types.Queries.Tests/__snapshots__/CodeFirstSchemaTests.FieldResult_With_Errors_Are_Valid.graphql

@@ -0,0 +1,22 @@
+schema {
+  query: ValidQueryValueTask
+}
+
+interface Error {
+  message: String!
+}
+
+type ArgumentError implements Error {
+  message: String!
+  paramName: String
+}
+
+type Foo {
+  bar: String!
+}
+
+type ValidQueryValueTask {
+  foo: FooResult!
+}
+
+union FooResult = Foo | ArgumentError

src/HotChocolate/Core/test/Types.Queries.Tests/__snapshots__/CodeFirstSchemaTests.Throw_SchemaError_When_FieldResult_Has_No_Errors.snap

@@ -0,0 +1 @@
+The field `InvalidQuery.foo` must declare errors to use a FieldResult<T>.

src/HotChocolate/Core/test/Types.Queries.Tests/__snapshots__/CodeFirstSchemaTests.Throw_SchemaError_When_FieldResult_Has_No_Errors_1.snap

@@ -0,0 +1 @@
+The field `InvalidQuery.foo` must declare errors to use a FieldResult<T>.

src/HotChocolate/Core/test/Types.Queries.Tests/__snapshots__/CodeFirstSchemaTests.Throw_SchemaError_When_FieldResult_Has_No_Errors_2.snap

@@ -0,0 +1 @@
+The field `InvalidQueryTask.foo` must declare errors to use a FieldResult<T>.

src/HotChocolate/Core/test/Types.Queries.Tests/__snapshots__/CodeFirstSchemaTests.Throw_SchemaError_When_FieldResult_Has_No_Errors_3.snap

@@ -0,0 +1 @@
+The field `InvalidQueryValueTask.foo` must declare errors to use a FieldResult<T>.