Skip to content

fix(validate): no reusing root types #3453

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
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
25 changes: 11 additions & 14 deletions src/jsutils/didYouMean.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { orList } from './formatList';

const MAX_SUGGESTIONS = 5;

/**
Expand All @@ -12,26 +14,21 @@ export function didYouMean(
firstArg: string | ReadonlyArray<string>,
secondArg?: ReadonlyArray<string>,
) {
const [subMessage, suggestionsArg] = secondArg
const [subMessage, suggestions] = secondArg
? [firstArg as string, secondArg]
: [undefined, firstArg as ReadonlyArray<string>];

if (suggestions.length === 0) {
return '';
}

let message = ' Did you mean ';
if (subMessage) {
message += subMessage + ' ';
}

const suggestions = suggestionsArg.map((x) => `"${x}"`);
switch (suggestions.length) {
case 0:
return '';
case 1:
return message + suggestions[0] + '?';
case 2:
return message + suggestions[0] + ' or ' + suggestions[1] + '?';
}

const selected = suggestions.slice(0, MAX_SUGGESTIONS);
const lastItem = selected.pop();
return message + selected.join(', ') + ', or ' + lastItem + '?';
const suggestionList = orList(
suggestions.slice(0, MAX_SUGGESTIONS).map((x) => `"${x}"`),
);
return message + suggestionList + '?';
}
30 changes: 30 additions & 0 deletions src/jsutils/formatList.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { invariant } from './invariant';

/**
* Given [ A, B, C ] return 'A, B, or C'.
*/
export function orList(items: ReadonlyArray<string>): string {
return formatList('or', items);
}

/**
* Given [ A, B, C ] return 'A, B, and C'.
*/
export function andList(items: ReadonlyArray<string>): string {
return formatList('and', items);
}

function formatList(conjunction: string, items: ReadonlyArray<string>): string {
invariant(items.length !== 0);

switch (items.length) {
case 1:
return items[0];
case 2:
return items[0] + ' ' + conjunction + ' ' + items[1];
}

const allButLast = items.slice(0, -1);
const lastItem = items[items.length - 1];
return allButLast.join(', ') + ', ' + conjunction + ' ' + lastItem;
}
80 changes: 80 additions & 0 deletions src/type/__tests__/validation-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,86 @@ describe('Type System: A Schema must have Object root types', () => {
});
});

describe('Type System: Root types must all be different if provided', () => {
it('accepts a Schema with different root types', () => {
const schema = buildSchema(`
type SomeObject1 {
field: String
}

type SomeObject2 {
field: String
}

type SomeObject3 {
field: String
}

schema {
query: SomeObject1
mutation: SomeObject2
subscription: SomeObject3
}
`);
expectJSON(validateSchema(schema)).toDeepEqual([]);
});

it('rejects a Schema where the same type is used for multiple root types', () => {
const schema = buildSchema(`
type SomeObject {
field: String
}

type UniqueObject {
field: String
}

schema {
query: SomeObject
mutation: UniqueObject
subscription: SomeObject
}
`);

expectJSON(validateSchema(schema)).toDeepEqual([
{
message:
'All root types must be different, "SomeObject" type is used as query and subscription root types.',
locations: [
{ line: 11, column: 16 },
{ line: 13, column: 23 },
],
},
]);
});

it('rejects a Schema where the same type is used for all root types', () => {
const schema = buildSchema(`
type SomeObject {
field: String
}

schema {
query: SomeObject
mutation: SomeObject
subscription: SomeObject
}
`);

expectJSON(validateSchema(schema)).toDeepEqual([
{
message:
'All root types must be different, "SomeObject" type is used as query, mutation, and subscription root types.',
locations: [
{ line: 7, column: 16 },
{ line: 8, column: 19 },
{ line: 9, column: 23 },
],
},
]);
});
});

describe('Type System: Objects must have fields', () => {
it('accepts an Object type with fields object', () => {
const schema = buildSchema(`
Expand Down
38 changes: 30 additions & 8 deletions src/type/validate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { AccumulatorMap } from '../jsutils/AccumulatorMap';
import { capitalize } from '../jsutils/capitalize';
import { andList } from '../jsutils/formatList';
import { inspect } from '../jsutils/inspect';
import type { Maybe } from '../jsutils/Maybe';

Expand Down Expand Up @@ -118,18 +120,38 @@ function validateRootTypes(context: SchemaValidationContext): void {
context.reportError('Query root type must be provided.', schema.astNode);
}

const rootTypesMap = new AccumulatorMap<
GraphQLObjectType,
OperationTypeNode
>();
for (const operationType of Object.values(OperationTypeNode)) {
const rootType = schema.getRootType(operationType);

if (rootType != null && !isObjectType(rootType)) {
const operationTypeStr = capitalize(operationType);
const rootTypeStr = inspect(rootType);
if (rootType != null) {
if (!isObjectType(rootType)) {
const operationTypeStr = capitalize(operationType);
const rootTypeStr = inspect(rootType);
context.reportError(
operationType === OperationTypeNode.QUERY
? `${operationTypeStr} root type must be Object type, it cannot be ${rootTypeStr}.`
: `${operationTypeStr} root type must be Object type if provided, it cannot be ${rootTypeStr}.`,
getOperationTypeNode(schema, operationType) ??
(rootType as any).astNode,
);
} else {
rootTypesMap.add(rootType, operationType);
}
}
}

for (const [rootType, operationTypes] of rootTypesMap.entries()) {
if (operationTypes.length > 1) {
const operationList = andList(operationTypes);
context.reportError(
operationType === OperationTypeNode.QUERY
? `${operationTypeStr} root type must be Object type, it cannot be ${rootTypeStr}.`
: `${operationTypeStr} root type must be Object type if provided, it cannot be ${rootTypeStr}.`,
getOperationTypeNode(schema, operationType) ??
(rootType as any).astNode,
`All root types must be different, "${rootType.name}" type is used as ${operationList} root types.`,
operationTypes.map((operationType) =>
getOperationTypeNode(schema, operationType),
),
);
}
}
Expand Down