Merge pull request #637 from dackroyd/correctly-apply-arg-names-valid…

…ation-rule Correctly apply arg names validation rule
graph-gophers · Apr 3, 2024 · cd26a0e · cd26a0e
2 parents 8649199 + 08873d6
commit cd26a0e
Show file tree

Hide file tree

Showing 5 changed files with 217 additions and 5 deletions.
diff --git a/internal/validation/suggestion.go b/internal/validation/suggestion.go
@@ -12,7 +12,7 @@ func makeSuggestion(prefix string, options []string, input string) string {
 	distances := make(map[string]int)
 	for _, opt := range options {
 		distance := levenshteinDistance(input, opt)
-		threshold := max(len(input)/2, max(len(opt)/2, 1))
+		threshold := max(len(input)/2, max(len(opt)/2, 2))
 		if distance < threshold {
 			selected = append(selected, opt)
 			distances[opt] = distance

diff --git a/internal/validation/testdata/export.ts b/internal/validation/testdata/export.ts
@@ -7,8 +7,7 @@ import { schemas, testCases } from 'graphql/src/validation/__tests__/harness.js'
 // require('graphql/src/validation/__tests__/ExecutableDefinitions-test');
 import 'graphql/src/validation/__tests__/FieldsOnCorrectTypeRule-test.js';
 import 'graphql/src/validation/__tests__/FragmentsOnCompositeTypesRule-test.js';
-// TODO: Fix test failures.
-// require('graphql/src/validation/__tests__/KnownArgumentNames-test');
+import 'graphql/src/validation/__tests__/KnownArgumentNamesRule-test.js';
 import 'graphql/src/validation/__tests__/KnownDirectivesRule-test.js';
 import 'graphql/src/validation/__tests__/KnownFragmentNamesRule-test.js';
 import 'graphql/src/validation/__tests__/KnownTypeNamesRule-test.js';

diff --git a/internal/validation/testdata/patches/graphql+17.0.0-alpha.3.patch b/internal/validation/testdata/patches/graphql+17.0.0-alpha.3.patch
@@ -31,6 +31,25 @@ index 849b662..1e37004 100644
 
  function expectErrors(queryStr: string) {
    return expectValidationErrors(FragmentsOnCompositeTypesRule, queryStr);
+diff --git a/node_modules/graphql/src/validation/__tests__/KnownArgumentNamesRule-test.ts b/node_modules/graphql/src/validation/__tests__/KnownArgumentNamesRule-test.ts
+index 0fcffec..8aa40d3 100644
+--- a/node_modules/graphql/src/validation/__tests__/KnownArgumentNamesRule-test.ts
++++ b/node_modules/graphql/src/validation/__tests__/KnownArgumentNamesRule-test.ts
+@@ -1,5 +1,3 @@
+-import { describe, it } from 'mocha';
+-
+ import type { GraphQLSchema } from '../../type/schema.js';
+
+ import { buildSchema } from '../../utilities/buildASTSchema.js';
+@@ -10,6 +8,8 @@ import {
+ } from '../rules/KnownArgumentNamesRule.js';
+
+ import {
++  describe,
++  it,
+   expectSDLValidationErrors,
+   expectValidationErrors,
+ } from './harness.js';
 diff --git a/node_modules/graphql/src/validation/__tests__/KnownDirectivesRule-test.ts b/node_modules/graphql/src/validation/__tests__/KnownDirectivesRule-test.ts
 index a3bbc19..7b04bba 100644
 --- a/node_modules/graphql/src/validation/__tests__/KnownDirectivesRule-test.ts

diff --git a/internal/validation/testdata/tests.json b/internal/validation/testdata/tests.json
@@ -400,6 +400,199 @@
         }
       ]
     },
+    {
+      "name": "Validate: Known argument names/single arg is known",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      fragment argOnRequiredArg on Dog {\n        doesKnowCommand(dogCommand: SIT)\n      }\n    ",
+      "errors": []
+    },
+    {
+      "name": "Validate: Known argument names/multiple args are known",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      fragment multipleArgs on ComplicatedArgs {\n        multipleReqs(req1: 1, req2: 2)\n      }\n    ",
+      "errors": []
+    },
+    {
+      "name": "Validate: Known argument names/ignores args of unknown fields",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      fragment argOnUnknownField on Dog {\n        unknownField(unknownArg: SIT)\n      }\n    ",
+      "errors": []
+    },
+    {
+      "name": "Validate: Known argument names/multiple args in reverse order are known",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      fragment multipleArgsReverseOrder on ComplicatedArgs {\n        multipleReqs(req2: 2, req1: 1)\n      }\n    ",
+      "errors": []
+    },
+    {
+      "name": "Validate: Known argument names/no args on optional arg",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      fragment noArgOnOptionalArg on Dog {\n        isHouseTrained\n      }\n    ",
+      "errors": []
+    },
+    {
+      "name": "Validate: Known argument names/args are known deeply",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      {\n        dog {\n          doesKnowCommand(dogCommand: SIT)\n        }\n        human {\n          pet {\n            ... on Dog {\n              doesKnowCommand(dogCommand: SIT)\n            }\n          }\n        }\n      }\n    ",
+      "errors": []
+    },
+    {
+      "name": "Validate: Known argument names/directive args are known",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      {\n        dog @skip(if: true)\n      }\n    ",
+      "errors": []
+    },
+    {
+      "name": "Validate: Known argument names/field args are invalid",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      {\n        dog @skip(unless: true)\n      }\n    ",
+      "errors": [
+        {
+          "message": "Unknown argument \"unless\" on directive \"@skip\".",
+          "locations": [
+            {
+              "line": 3,
+              "column": 19
+            }
+          ]
+        }
+      ]
+    },
+    {
+      "name": "Validate: Known argument names/directive without args is valid",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      {\n        dog @onField\n      }\n    ",
+      "errors": []
+    },
+    {
+      "name": "Validate: Known argument names/arg passed to directive without arg is reported",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      {\n        dog @onField(if: true)\n      }\n    ",
+      "errors": [
+        {
+          "message": "Unknown argument \"if\" on directive \"@onField\".",
+          "locations": [
+            {
+              "line": 3,
+              "column": 22
+            }
+          ]
+        }
+      ]
+    },
+    {
+      "name": "Validate: Known argument names/misspelled directive args are reported",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      {\n        dog @skip(iff: true)\n      }\n    ",
+      "errors": [
+        {
+          "message": "Unknown argument \"iff\" on directive \"@skip\". Did you mean \"if\"?",
+          "locations": [
+            {
+              "line": 3,
+              "column": 19
+            }
+          ]
+        }
+      ]
+    },
+    {
+      "name": "Validate: Known argument names/invalid arg name",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      fragment invalidArgName on Dog {\n        doesKnowCommand(unknown: true)\n      }\n    ",
+      "errors": [
+        {
+          "message": "Unknown argument \"unknown\" on field \"Dog.doesKnowCommand\".",
+          "locations": [
+            {
+              "line": 3,
+              "column": 25
+            }
+          ]
+        }
+      ]
+    },
+    {
+      "name": "Validate: Known argument names/misspelled arg name is reported",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      fragment invalidArgName on Dog {\n        doesKnowCommand(DogCommand: true)\n      }\n    ",
+      "errors": [
+        {
+          "message": "Unknown argument \"DogCommand\" on field \"Dog.doesKnowCommand\". Did you mean \"dogCommand\"?",
+          "locations": [
+            {
+              "line": 3,
+              "column": 25
+            }
+          ]
+        }
+      ]
+    },
+    {
+      "name": "Validate: Known argument names/unknown args amongst known args",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      fragment oneGoodArgOneInvalidArg on Dog {\n        doesKnowCommand(whoKnows: 1, dogCommand: SIT, unknown: true)\n      }\n    ",
+      "errors": [
+        {
+          "message": "Unknown argument \"whoKnows\" on field \"Dog.doesKnowCommand\".",
+          "locations": [
+            {
+              "line": 3,
+              "column": 25
+            }
+          ]
+        },
+        {
+          "message": "Unknown argument \"unknown\" on field \"Dog.doesKnowCommand\".",
+          "locations": [
+            {
+              "line": 3,
+              "column": 55
+            }
+          ]
+        }
+      ]
+    },
+    {
+      "name": "Validate: Known argument names/unknown args deeply",
+      "rule": "KnownArgumentNamesRule",
+      "schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
+      "query": "\n      {\n        dog {\n          doesKnowCommand(unknown: true)\n        }\n        human {\n          pet {\n            ... on Dog {\n              doesKnowCommand(unknown: true)\n            }\n          }\n        }\n      }\n    ",
+      "errors": [
+        {
+          "message": "Unknown argument \"unknown\" on field \"Dog.doesKnowCommand\".",
+          "locations": [
+            {
+              "line": 4,
+              "column": 27
+            }
+          ]
+        },
+        {
+          "message": "Unknown argument \"unknown\" on field \"Dog.doesKnowCommand\".",
+          "locations": [
+            {
+              "line": 9,
+              "column": 31
+            }
+          ]
+        }
+      ]
+    },
     {
       "name": "Validate: Known directives/with no directives",
       "rule": "KnownDirectivesRule",

diff --git a/internal/validation/validation.go b/internal/validation/validation.go
@@ -355,7 +355,7 @@ func validateSelection(c *opContext, sel ast.Selection, t ast.NamedType) {
 		validateArgumentLiterals(c, sel.Arguments)
 		if f != nil {
 			validateArgumentTypes(c, sel.Arguments, f.Arguments, sel.Alias.Loc,
-				func() string { return fmt.Sprintf("field %q of type %q", fieldName, t) },
+				func() string { return fmt.Sprintf(`field "%s.%s"`, t, fieldName) },
 				func() string { return fmt.Sprintf("Field %q", fieldName) },
 			)
 		}
@@ -752,7 +752,8 @@ func validateArgumentTypes(c *opContext, args ast.ArgumentList, argDecls ast.Arg
 	for _, selArg := range args {
 		arg := argDecls.Get(selArg.Name.Name)
 		if arg == nil {
-			c.addErr(selArg.Name.Loc, "KnownArgumentNames", "Unknown argument %q on %s.", selArg.Name.Name, owner1())
+			suggestion := makeSuggestion("Did you mean", argDecls.Names(), selArg.Name.Name)
+			c.addErr(selArg.Name.Loc, "KnownArgumentNamesRule", "Unknown argument %q on %s.%s", selArg.Name.Name, owner1(), suggestion)
 			continue
 		}
 		value := selArg.Value