Merge pull request graphql-dotnet#233 from graphql-dotnet/argument-bug

Fix null reference on field without arguments

Author: joemcbride

ILMerge.exe

@@ -59,9 +59,9 @@ public ExecutionResult AssertQuery(
             var writtenResult = Writer.Write(runResult);
             var expectedResult = Writer.Write(expectedExecutionResult);
 
-#if DEBUG
-            Console.WriteLine(writtenResult);
-#endif
+//#if DEBUG
+//            Console.WriteLine(writtenResult);
+//#endif
 
             string additionalInfo = null;
 
@@ -110,12 +110,6 @@ public void build_dynamic_schema()
             schema.Query = root;
             schema.RegisterTypes(person);
 
-            var printed = new SchemaPrinter(schema).Print();
-
-#if DEBUG
-            Console.WriteLine(printed);
-#endif
-
             AssertQuerySuccess(
                 schema,
                 @"{ hero { name friends { name } } }",

src/GraphQL.Tests/Execution/RegisteredInstanceTests.cs

@@ -15,6 +15,16 @@ fragment argOnRequiredArg on Dog {
             ");
         }
 
+        [Fact]
+        public void no_args_are_known()
+        {
+            ShouldPassRule(@"
+              fragment multipleArgs on ComplicatedArgs {
+                noArgsField
+              }
+            ");
+        }
+
         [Fact]
         public void multiple_args_are_known()
         {
@@ -84,6 +94,20 @@ dog @skip(if: true)
             ");
         }
 
+        [Fact]
+        public void field_with_no_args_given_arg_is_invalid()
+        {
+            ShouldFailRule(_ =>
+            {
+                _.Query = @"
+                  fragment multipleArgs on ComplicatedArgs {
+                    noArgsField(first: 1)
+                  }
+                ";
+                _.Error(Rule.UnknownArgMessage("first", "noArgsField", "ComplicatedArgs", null), 3, 33);
+            });
+        }
+
         [Fact]
         public void undirective_args_are_invalid()
         {

src/GraphQL.Tests/Validation/KnownArgumentNamesTests.cs

@@ -197,6 +197,7 @@ public class ComplicatedArgs : ObjectGraphType
         public ComplicatedArgs()
         {
             Name = "ComplicatedArgs";
+            Field<StringGraphType>("noArgsField");
             Field<StringGraphType>(
                 "intArgField",
                 arguments: new QueryArguments(

src/GraphQL.Tests/Validation/ValidationSchema.cs

@@ -86,9 +86,14 @@ public static string QuotedOrList(IEnumerable<string> items, int maxLength = 5)
         /// </summary>
         public static string[] SuggestionList(string input, IEnumerable<string> options)
         {
+            if (options == null)
+            {
+                return new string[0];
+            }
+
             var optionsByDistance = new Dictionary<string, int>();
             var inputThreshold = input.Length / 2;
-            options.Apply(t =>
+            options?.Apply(t =>
             {
                 var distance = DamerauLevenshteinDistance(input, t, inputThreshold);
                 var threshold = Math.Max(inputThreshold, Math.Max(t.Length / 2, 1));
@@ -126,7 +131,7 @@ public static int DamerauLevenshteinDistance(string source, string target, int t
             // Return trivial case - difference in string lengths exceeds threshhold
             if (Math.Abs(length1 - length2) > threshold) { return int.MaxValue; }
 
-            // Ensure arrays [i] / length1 use shorter length 
+            // Ensure arrays [i] / length1 use shorter length
             if (length1 > length2)
             {
                 Swap(ref target, ref source);

src/GraphQL/Utilities/StringUtils.cs

@@ -1,5 +1,4 @@
-using System;
-using System.Linq;
+using System.Linq;
 using GraphQL.Language.AST;
 using GraphQL.Utilities;
 
@@ -13,27 +12,25 @@ namespace GraphQL.Validation.Rules
     /// </summary>
     public class KnownArgumentNames : IValidationRule
     {
-        public Func<string, string, string, string[], string> UnknownArgMessage =
-            (argName, fieldName, type, suggestedArgs) =>
+        public string UnknownArgMessage(string argName, string fieldName, string type, string[] suggestedArgs)
+        {
+            var message = $"Unknown argument \"{argName}\" on field \"{fieldName}\" of type \"{type}\".";
+            if (suggestedArgs != null && suggestedArgs.Length > 0)
             {
-                var message = $"Unknown argument \"{argName}\" on field \"{fieldName}\" of type \"{type}\".";
-                if (suggestedArgs != null && suggestedArgs.Length > 0)
-                {
-                    message += $"Did you mean {StringUtils.QuotedOrList(suggestedArgs)}";
-                }
-                return message;
-            };
+                message += $"Did you mean {StringUtils.QuotedOrList(suggestedArgs)}";
+            }
+            return message;
+        }
 
-        public Func<string, string, string[], string> UnknownDirectiveArgMessage =
-            (argName, directiveName, suggestedArgs) =>
+        public string UnknownDirectiveArgMessage(string argName, string directiveName, string[] suggestedArgs)
+        {
+            var message = $"Unknown argument \"{argName}\" on directive \"{directiveName}\".";
+            if (suggestedArgs != null && suggestedArgs.Length > 0)
             {
-                var message = $"Unknown argument \"{argName}\" on directive \"{directiveName}\".";
-                if (suggestedArgs != null && suggestedArgs.Length > 0)
-                {
-                    message += $"Did you mean {StringUtils.QuotedOrList(suggestedArgs)}";
-                }
-                return message;
-            };
+                message += $"Did you mean {StringUtils.QuotedOrList(suggestedArgs)}";
+            }
+            return message;
+        }
 
         public INodeVisitor Validate(ValidationContext context)
         {
@@ -48,7 +45,7 @@ public INodeVisitor Validate(ValidationContext context)
                         var fieldDef = context.TypeInfo.GetFieldDef();
                         if (fieldDef != null)
                         {
-                            var fieldArgDef = fieldDef.Arguments.Find(node.Name);
+                            var fieldArgDef = fieldDef.Arguments?.Find(node.Name);
                             if (fieldArgDef == null)
                             {
                                 var parentType = context.TypeInfo.GetParentType();
@@ -60,7 +57,7 @@ public INodeVisitor Validate(ValidationContext context)
                                         node.Name,
                                         fieldDef.Name,
                                         context.Print(parentType),
-                                        StringUtils.SuggestionList(node.Name, fieldDef.Arguments.Select(q => q.Name))),
+                                        StringUtils.SuggestionList(node.Name, fieldDef.Arguments?.Select(q => q.Name))),
                                     node));
                             }
                         }
@@ -69,7 +66,7 @@ public INodeVisitor Validate(ValidationContext context)
                         var directive = context.TypeInfo.GetDirective();
                         if (directive != null)
                         {
-                            var directiveArgDef = directive.Arguments.Find(node.Name);
+                            var directiveArgDef = directive.Arguments?.Find(node.Name);
                             if (directiveArgDef == null)
                             {
                                 context.ReportError(new ValidationError(
@@ -78,7 +75,7 @@ public INodeVisitor Validate(ValidationContext context)
                                     UnknownDirectiveArgMessage(
                                         node.Name,
                                         directive.Name,
-                                        StringUtils.SuggestionList(node.Name, directive.Arguments.Select(q => q.Name))),
+                                        StringUtils.SuggestionList(node.Name, directive.Arguments?.Select(q => q.Name))),
                                     node));
                             }
                         }