Skip to content

Commit

Permalink
Cover empty field-selections (#4228)
Browse files Browse the repository at this point in the history
Closes graphql/graphql-js#3790

This follows the spec closer to the letter as we consider empty
selection-sets invalid as well. This however does not cover empty
operation-definitions, which should also be considered invalid.

The parser itself considers empty-selections invalid as it requires you
to specify a name before a closing curly, hence why testing happens a
bit more manual here. Despite our parser not considering this valid, I
think this is good to validate when folks use a separate parser or
construct documents manually as this is valid in our type-system.
  • Loading branch information
JoviDeCroock authored and erikwrede committed Oct 17, 2024
1 parent 924e9d3 commit adafea8
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
42 changes: 41 additions & 1 deletion src/validation/__tests__/ScalarLeafsRule-test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { describe, it } from 'mocha';

import { expectJSON } from '../../__testUtils__/expectJSON.js';

import type { DocumentNode } from '../../language/ast.js';
import { OperationTypeNode } from '../../language/ast.js';
import { Kind } from '../../language/kinds.js';

import { ScalarLeafsRule } from '../rules/ScalarLeafsRule.js';
import { validate } from '../validate.js';

import { expectValidationErrors } from './harness.js';
import { expectValidationErrors, testSchema } from './harness.js';

function expectErrors(queryStr: string) {
return expectValidationErrors(ScalarLeafsRule, queryStr);
Expand Down Expand Up @@ -35,6 +42,39 @@ describe('Validate: Scalar leafs', () => {
]);
});

it('object type having only one selection', () => {
const doc: DocumentNode = {
kind: Kind.DOCUMENT,
definitions: [
{
kind: Kind.OPERATION_DEFINITION,
operation: OperationTypeNode.QUERY,
selectionSet: {
kind: Kind.SELECTION_SET,
selections: [
{
kind: Kind.FIELD,
name: { kind: Kind.NAME, value: 'human' },
selectionSet: { kind: Kind.SELECTION_SET, selections: [] },
},
],
},
},
],
};

// We can't leverage expectErrors since it doesn't support passing in the
// documentNode directly. We have to do this because this is technically
// an invalid document.
const errors = validate(testSchema, doc, [ScalarLeafsRule]);
expectJSON(errors).toDeepEqual([
{
message:
'Field "human" of type "Human" must have at least one field selected.',
},
]);
});

it('interface type missing selection', () => {
expectErrors(`
{
Expand Down
9 changes: 9 additions & 0 deletions src/validation/rules/ScalarLeafsRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ export function ScalarLeafsRule(context: ValidationContext): ASTVisitor {
{ nodes: node },
),
);
} else if (selectionSet.selections.length === 0) {
const fieldName = node.name.value;
const typeStr = inspect(type);
context.reportError(
new GraphQLError(
`Field "${fieldName}" of type "${typeStr}" must have at least one field selected.`,
{ nodes: node },
),
);
}
}
},
Expand Down

0 comments on commit adafea8

Please sign in to comment.