Skip to content

Commit 08873d6

Browse files
dackroyddackroyd
committed
Correctly Apply KnownArgumentNamesRule
Updating implementation of KnownArgumentNamesRule to follow the graphql-js test cases. Changes required here are to correct messaging and suggestions, along with pulling in the actual test cases. The minimum threshold is updated for the suggestion levenshteinDistance to allow suggestions for short names. From the test cases, this allows 'if' to be suggested when 'iff' is used with the @Skip directive.
1 parent 8649199 commit 08873d6

File tree

5 files changed

+217
-5
lines changed

5 files changed

+217
-5
lines changed

‎internal/validation/suggestion.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ func makeSuggestion(prefix string, options []string, input string) string {
1212
distances := make(map[string]int)
1313
for _, opt := range options {
1414
distance := levenshteinDistance(input, opt)
15-
threshold := max(len(input)/2, max(len(opt)/2, 1))
15+
threshold := max(len(input)/2, max(len(opt)/2, 2))
1616
if distance < threshold {
1717
selected = append(selected, opt)
1818
distances[opt] = distance

‎internal/validation/testdata/export.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import { schemas, testCases } from 'graphql/src/validation/__tests__/harness.js'
77
// require('graphql/src/validation/__tests__/ExecutableDefinitions-test');
88
import 'graphql/src/validation/__tests__/FieldsOnCorrectTypeRule-test.js';
99
import 'graphql/src/validation/__tests__/FragmentsOnCompositeTypesRule-test.js';
10-
// TODO: Fix test failures.
11-
// require('graphql/src/validation/__tests__/KnownArgumentNames-test');
10+
import 'graphql/src/validation/__tests__/KnownArgumentNamesRule-test.js';
1211
import 'graphql/src/validation/__tests__/KnownDirectivesRule-test.js';
1312
import 'graphql/src/validation/__tests__/KnownFragmentNamesRule-test.js';
1413
import 'graphql/src/validation/__tests__/KnownTypeNamesRule-test.js';

‎internal/validation/testdata/patches/graphql+17.0.0-alpha.3.patch

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,25 @@ index 849b662..1e37004 100644
3131

3232
function expectErrors(queryStr: string) {
3333
return expectValidationErrors(FragmentsOnCompositeTypesRule, queryStr);
34+
diff --git a/node_modules/graphql/src/validation/__tests__/KnownArgumentNamesRule-test.ts b/node_modules/graphql/src/validation/__tests__/KnownArgumentNamesRule-test.ts
35+
index 0fcffec..8aa40d3 100644
36+
--- a/node_modules/graphql/src/validation/__tests__/KnownArgumentNamesRule-test.ts
37+
+++ b/node_modules/graphql/src/validation/__tests__/KnownArgumentNamesRule-test.ts
38+
@@ -1,5 +1,3 @@
39+
-import { describe, it } from 'mocha';
40+
-
41+
import type { GraphQLSchema } from '../../type/schema.js';
42+
43+
import { buildSchema } from '../../utilities/buildASTSchema.js';
44+
@@ -10,6 +8,8 @@ import {
45+
} from '../rules/KnownArgumentNamesRule.js';
46+
47+
import {
48+
+ describe,
49+
+ it,
50+
expectSDLValidationErrors,
51+
expectValidationErrors,
52+
} from './harness.js';
3453
diff --git a/node_modules/graphql/src/validation/__tests__/KnownDirectivesRule-test.ts b/node_modules/graphql/src/validation/__tests__/KnownDirectivesRule-test.ts
3554
index a3bbc19..7b04bba 100644
3655
--- a/node_modules/graphql/src/validation/__tests__/KnownDirectivesRule-test.ts

‎internal/validation/testdata/tests.json

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,199 @@
400400
}
401401
]
402402
},
403+
{
404+
"name": "Validate: Known argument names/single arg is known",
405+
"rule": "KnownArgumentNamesRule",
406+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
407+
"query": "\n fragment argOnRequiredArg on Dog {\n doesKnowCommand(dogCommand: SIT)\n }\n ",
408+
"errors": []
409+
},
410+
{
411+
"name": "Validate: Known argument names/multiple args are known",
412+
"rule": "KnownArgumentNamesRule",
413+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
414+
"query": "\n fragment multipleArgs on ComplicatedArgs {\n multipleReqs(req1: 1, req2: 2)\n }\n ",
415+
"errors": []
416+
},
417+
{
418+
"name": "Validate: Known argument names/ignores args of unknown fields",
419+
"rule": "KnownArgumentNamesRule",
420+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
421+
"query": "\n fragment argOnUnknownField on Dog {\n unknownField(unknownArg: SIT)\n }\n ",
422+
"errors": []
423+
},
424+
{
425+
"name": "Validate: Known argument names/multiple args in reverse order are known",
426+
"rule": "KnownArgumentNamesRule",
427+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
428+
"query": "\n fragment multipleArgsReverseOrder on ComplicatedArgs {\n multipleReqs(req2: 2, req1: 1)\n }\n ",
429+
"errors": []
430+
},
431+
{
432+
"name": "Validate: Known argument names/no args on optional arg",
433+
"rule": "KnownArgumentNamesRule",
434+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
435+
"query": "\n fragment noArgOnOptionalArg on Dog {\n isHouseTrained\n }\n ",
436+
"errors": []
437+
},
438+
{
439+
"name": "Validate: Known argument names/args are known deeply",
440+
"rule": "KnownArgumentNamesRule",
441+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
442+
"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 ",
443+
"errors": []
444+
},
445+
{
446+
"name": "Validate: Known argument names/directive args are known",
447+
"rule": "KnownArgumentNamesRule",
448+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
449+
"query": "\n {\n dog @skip(if: true)\n }\n ",
450+
"errors": []
451+
},
452+
{
453+
"name": "Validate: Known argument names/field args are invalid",
454+
"rule": "KnownArgumentNamesRule",
455+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
456+
"query": "\n {\n dog @skip(unless: true)\n }\n ",
457+
"errors": [
458+
{
459+
"message": "Unknown argument \"unless\" on directive \"@skip\".",
460+
"locations": [
461+
{
462+
"line": 3,
463+
"column": 19
464+
}
465+
]
466+
}
467+
]
468+
},
469+
{
470+
"name": "Validate: Known argument names/directive without args is valid",
471+
"rule": "KnownArgumentNamesRule",
472+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
473+
"query": "\n {\n dog @onField\n }\n ",
474+
"errors": []
475+
},
476+
{
477+
"name": "Validate: Known argument names/arg passed to directive without arg is reported",
478+
"rule": "KnownArgumentNamesRule",
479+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
480+
"query": "\n {\n dog @onField(if: true)\n }\n ",
481+
"errors": [
482+
{
483+
"message": "Unknown argument \"if\" on directive \"@onField\".",
484+
"locations": [
485+
{
486+
"line": 3,
487+
"column": 22
488+
}
489+
]
490+
}
491+
]
492+
},
493+
{
494+
"name": "Validate: Known argument names/misspelled directive args are reported",
495+
"rule": "KnownArgumentNamesRule",
496+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
497+
"query": "\n {\n dog @skip(iff: true)\n }\n ",
498+
"errors": [
499+
{
500+
"message": "Unknown argument \"iff\" on directive \"@skip\". Did you mean \"if\"?",
501+
"locations": [
502+
{
503+
"line": 3,
504+
"column": 19
505+
}
506+
]
507+
}
508+
]
509+
},
510+
{
511+
"name": "Validate: Known argument names/invalid arg name",
512+
"rule": "KnownArgumentNamesRule",
513+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
514+
"query": "\n fragment invalidArgName on Dog {\n doesKnowCommand(unknown: true)\n }\n ",
515+
"errors": [
516+
{
517+
"message": "Unknown argument \"unknown\" on field \"Dog.doesKnowCommand\".",
518+
"locations": [
519+
{
520+
"line": 3,
521+
"column": 25
522+
}
523+
]
524+
}
525+
]
526+
},
527+
{
528+
"name": "Validate: Known argument names/misspelled arg name is reported",
529+
"rule": "KnownArgumentNamesRule",
530+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
531+
"query": "\n fragment invalidArgName on Dog {\n doesKnowCommand(DogCommand: true)\n }\n ",
532+
"errors": [
533+
{
534+
"message": "Unknown argument \"DogCommand\" on field \"Dog.doesKnowCommand\". Did you mean \"dogCommand\"?",
535+
"locations": [
536+
{
537+
"line": 3,
538+
"column": 25
539+
}
540+
]
541+
}
542+
]
543+
},
544+
{
545+
"name": "Validate: Known argument names/unknown args amongst known args",
546+
"rule": "KnownArgumentNamesRule",
547+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
548+
"query": "\n fragment oneGoodArgOneInvalidArg on Dog {\n doesKnowCommand(whoKnows: 1, dogCommand: SIT, unknown: true)\n }\n ",
549+
"errors": [
550+
{
551+
"message": "Unknown argument \"whoKnows\" on field \"Dog.doesKnowCommand\".",
552+
"locations": [
553+
{
554+
"line": 3,
555+
"column": 25
556+
}
557+
]
558+
},
559+
{
560+
"message": "Unknown argument \"unknown\" on field \"Dog.doesKnowCommand\".",
561+
"locations": [
562+
{
563+
"line": 3,
564+
"column": 55
565+
}
566+
]
567+
}
568+
]
569+
},
570+
{
571+
"name": "Validate: Known argument names/unknown args deeply",
572+
"rule": "KnownArgumentNamesRule",
573+
"schema": "3vm8aJ4UHhzj01V2PFAYRVZj2R0SPm+mxfn/edqH9aU=",
574+
"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 ",
575+
"errors": [
576+
{
577+
"message": "Unknown argument \"unknown\" on field \"Dog.doesKnowCommand\".",
578+
"locations": [
579+
{
580+
"line": 4,
581+
"column": 27
582+
}
583+
]
584+
},
585+
{
586+
"message": "Unknown argument \"unknown\" on field \"Dog.doesKnowCommand\".",
587+
"locations": [
588+
{
589+
"line": 9,
590+
"column": 31
591+
}
592+
]
593+
}
594+
]
595+
},
403596
{
404597
"name": "Validate: Known directives/with no directives",
405598
"rule": "KnownDirectivesRule",

‎internal/validation/validation.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ func validateSelection(c *opContext, sel ast.Selection, t ast.NamedType) {
355355
validateArgumentLiterals(c, sel.Arguments)
356356
if f != nil {
357357
validateArgumentTypes(c, sel.Arguments, f.Arguments, sel.Alias.Loc,
358-
func() string { return fmt.Sprintf("field %q of type %q", fieldName, t) },
358+
func() string { return fmt.Sprintf(`field "%s.%s"`, t, fieldName) },
359359
func() string { return fmt.Sprintf("Field %q", fieldName) },
360360
)
361361
}
@@ -752,7 +752,8 @@ func validateArgumentTypes(c *opContext, args ast.ArgumentList, argDecls ast.Arg
752752
for _, selArg := range args {
753753
arg := argDecls.Get(selArg.Name.Name)
754754
if arg == nil {
755-
c.addErr(selArg.Name.Loc, "KnownArgumentNames", "Unknown argument %q on %s.", selArg.Name.Name, owner1())
755+
suggestion := makeSuggestion("Did you mean", argDecls.Names(), selArg.Name.Name)
756+
c.addErr(selArg.Name.Loc, "KnownArgumentNamesRule", "Unknown argument %q on %s.%s", selArg.Name.Name, owner1(), suggestion)
756757
continue
757758
}
758759
value := selArg.Value

0 commit comments

Comments
 (0)