Skip to content

Commit 3f8f27a

Browse files
phryneasyaacovCR
andauthored
fix(validation): incorrect validation errors when variable descriptions are used (#4517)
I initially reported this as an LSP error (graphql/graphiql#4138), but turns out it's an error in `graphql-js`. <img width="1742" height="564" alt="image" src="https://github.com/user-attachments/assets/eca78ebc-e022-4fa0-b423-8169c9c902c4"/> --------- Co-authored-by: Yaacov Rydzinski <yaacovCR@gmail.com>
1 parent b34a4cd commit 3f8f27a

File tree

3 files changed

+41
-1
lines changed

3 files changed

+41
-1
lines changed

src/validation/__tests__/validation-test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,25 @@ describe('Validate: Limit maximum number of validation errors', () => {
179179
).to.throw(/^Error from custom rule!$/);
180180
});
181181
});
182+
183+
describe('operation and variable definition descriptions', () => {
184+
it('validates operation with description and variable descriptions', () => {
185+
const schema = buildSchema(
186+
'type Query { field(a: Int, b: String): String }',
187+
);
188+
const query = `
189+
"Operation description"
190+
query myQuery(
191+
"Variable a description"
192+
$a: Int,
193+
"""Variable b\nmultiline description"""
194+
$b: String
195+
) {
196+
field(a: $a, b: $b)
197+
}
198+
`;
199+
const ast = parse(query);
200+
const errors = validate(schema, ast);
201+
expect(errors.length).to.equal(0);
202+
});
203+
});

src/validation/rules/ValuesOfCorrectTypeRule.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ export function ValuesOfCorrectTypeRule(
116116
EnumValue: (node) => isValidValueNode(context, node),
117117
IntValue: (node) => isValidValueNode(context, node),
118118
FloatValue: (node) => isValidValueNode(context, node),
119+
// Descriptions are string values that would not validate according
120+
// to the below logic, but since (per the specification) descriptions must
121+
// not affect validation, they are ignored entirely when visiting the AST
122+
// and do not require special handling.
123+
// See https://spec.graphql.org/draft/#sec-Descriptions
119124
StringValue: (node) => isValidValueNode(context, node),
120125
BooleanValue: (node) => isValidValueNode(context, node),
121126
};

src/validation/validate.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { devAssert } from '../jsutils/devAssert';
2+
import { mapValue } from '../jsutils/mapValue';
23
import type { Maybe } from '../jsutils/Maybe';
34

45
import { GraphQLError } from '../error/GraphQLError';
56

67
import type { DocumentNode } from '../language/ast';
8+
import { QueryDocumentKeys } from '../language/ast';
79
import { visit, visitInParallel } from '../language/visitor';
810

911
import type { GraphQLSchema } from '../type/schema';
@@ -15,6 +17,13 @@ import { specifiedRules, specifiedSDLRules } from './specifiedRules';
1517
import type { SDLValidationRule, ValidationRule } from './ValidationContext';
1618
import { SDLValidationContext, ValidationContext } from './ValidationContext';
1719

20+
// Per the specification, descriptions must not affect validation.
21+
// See https://spec.graphql.org/draft/#sec-Descriptions
22+
const QueryDocumentKeysToValidate = mapValue(
23+
QueryDocumentKeys,
24+
(keys: ReadonlyArray<string>) => keys.filter((key) => key !== 'description'),
25+
);
26+
1827
/**
1928
* Implements the "Validation" section of the spec.
2029
*
@@ -76,7 +85,11 @@ export function validate(
7685

7786
// Visit the whole document with each instance of all provided rules.
7887
try {
79-
visit(documentAST, visitWithTypeInfo(typeInfo, visitor));
88+
visit(
89+
documentAST,
90+
visitWithTypeInfo(typeInfo, visitor),
91+
QueryDocumentKeysToValidate,
92+
);
8093
} catch (e) {
8194
if (e !== abortObj) {
8295
throw e;

0 commit comments

Comments
 (0)