Skip to content

Correctly apply arg names validation rule #637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/validation/suggestion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions internal/validation/testdata/export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
19 changes: 19 additions & 0 deletions internal/validation/testdata/patches/graphql+17.0.0-alpha.3.patch
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
193 changes: 193 additions & 0 deletions internal/validation/testdata/tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions internal/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) },
)
}
Expand Down Expand Up @@ -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
Expand Down