Skip to content

Commit

Permalink
Merge pull request graphql#3 from xuewei8910/wxue_question_mark
Browse files Browse the repository at this point in the history
Added question mark token and corresponding behavior. Also updated the validation rule to include question mark.
  • Loading branch information
xuewei8910 committed May 21, 2021
2 parents fce978a + 956f729 commit 33aa1dd
Show file tree
Hide file tree
Showing 14 changed files with 87 additions and 38 deletions.
2 changes: 2 additions & 0 deletions src/__testUtils__/kitchenSinkQuery.ts
Expand Up @@ -12,6 +12,8 @@ query queryName($foo: ComplexType, $site: Site = MOBILE) @onQuery {
}
field3!
requiredField4: field4!
field5?
optionalField6: field6?
}
... @skip(unless: $foo) {
id
Expand Down
15 changes: 11 additions & 4 deletions src/execution/execute.ts
Expand Up @@ -29,7 +29,7 @@ import type {
import { Kind } from '../language/kinds';

import type { GraphQLSchema } from '../type/schema';
import type {
import {
GraphQLObjectType,
GraphQLOutputType,
GraphQLLeafType,
Expand All @@ -39,6 +39,7 @@ import type {
GraphQLResolveInfo,
GraphQLTypeResolver,
GraphQLList,
getNullableType,
} from '../type/definition';
import { assertValidSchema } from '../type/validate';
import {
Expand Down Expand Up @@ -608,9 +609,15 @@ function resolveField(
return;
}

const returnType = fieldNodes[0].required // TODO: Need to update FieldNode
? new GraphQLNonNull(fieldDef.type)
: fieldDef.type;
let returnType
if (fieldNodes[0].required === 'required' && !isNonNullType(fieldDef.type)) {
returnType = new GraphQLNonNull(fieldDef.type)
} else if(fieldNodes[0].required === 'optional' ){
returnType = getNullableType(fieldDef.type)
} else {
returnType = fieldDef.type
}

const resolveFn = fieldDef.resolve ?? exeContext.fieldResolver;

const info = buildResolveInfo(
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Expand Up @@ -209,6 +209,7 @@ export {
isTypeDefinitionNode,
isTypeSystemExtensionNode,
isTypeExtensionNode,
RequiredStatus,
} from './language/index';

export type {
Expand Down
22 changes: 11 additions & 11 deletions src/language/__tests__/lexer-test.ts
Expand Up @@ -161,16 +161,16 @@ describe('Lexer', () => {
it('errors respect whitespace', () => {
let caughtError;
try {
lexOne(['', '', ' ?', ''].join('\n'));
lexOne(['', '', ' ~', ''].join('\n'));
} catch (error) {
caughtError = error;
}
expect(String(caughtError)).to.equal(dedent`
Syntax Error: Cannot parse the unexpected character "?".
Syntax Error: Cannot parse the unexpected character "~".
GraphQL request:3:5
2 |
3 | ?
3 | ~
| ^
4 |
`);
Expand All @@ -179,18 +179,18 @@ describe('Lexer', () => {
it('updates line numbers in error for file context', () => {
let caughtError;
try {
const str = ['', '', ' ?', ''].join('\n');
const str = ['', '', ' ~', ''].join('\n');
const source = new Source(str, 'foo.js', { line: 11, column: 12 });
new Lexer(source).advance();
} catch (error) {
caughtError = error;
}
expect(String(caughtError)).to.equal(dedent`
Syntax Error: Cannot parse the unexpected character "?".
Syntax Error: Cannot parse the unexpected character "~".
foo.js:13:6
12 |
13 | ?
13 | ~
| ^
14 |
`);
Expand All @@ -199,16 +199,16 @@ describe('Lexer', () => {
it('updates column numbers in error for file context', () => {
let caughtError;
try {
const source = new Source('?', 'foo.js', { line: 1, column: 5 });
const source = new Source('~', 'foo.js', { line: 1, column: 5 });
new Lexer(source).advance();
} catch (error) {
caughtError = error;
}
expect(String(caughtError)).to.equal(dedent`
Syntax Error: Cannot parse the unexpected character "?".
Syntax Error: Cannot parse the unexpected character "~".
foo.js:1:5
1 | ?
1 | ~
| ^
`);
});
Expand Down Expand Up @@ -819,8 +819,8 @@ describe('Lexer', () => {
locations: [{ line: 1, column: 1 }],
});

expectSyntaxError('?').to.deep.equal({
message: 'Syntax Error: Cannot parse the unexpected character "?".',
expectSyntaxError('~').to.deep.equal({
message: 'Syntax Error: Cannot parse the unexpected character "~".',
locations: [{ line: 1, column: 1 }],
});

Expand Down
10 changes: 10 additions & 0 deletions src/language/__tests__/parser-test.ts
Expand Up @@ -215,6 +215,16 @@ describe('Parser', () => {
).to.not.throw();
});

it('parses optional field', () => {
expect(() =>
parse(`
query {
optionalField?
}
`),
).to.not.throw();
});

it('parses required with alias', () => {
expect(() =>
parse(`
Expand Down
2 changes: 2 additions & 0 deletions src/language/__tests__/printer-test.ts
Expand Up @@ -167,6 +167,8 @@ describe('Printer: Query document', () => {
}
field3!
requiredField4: field4!
field5?
optionalField6: field6?
}
... @skip(unless: $foo) {
id
Expand Down
10 changes: 10 additions & 0 deletions src/language/__tests__/visitor-test.ts
Expand Up @@ -672,6 +672,16 @@ describe('Visitor', () => {
['enter', 'Name', 'name', 'Field'],
['leave', 'Name', 'name', 'Field'],
['leave', 'Field', 2, undefined],
['enter', 'Field', 3, undefined],
['enter', 'Name', 'name', 'Field'],
['leave', 'Name', 'name', 'Field'],
['leave', 'Field', 3, undefined],
['enter', 'Field', 4, undefined],
['enter', 'Name', 'alias', 'Field'],
['leave', 'Name', 'alias', 'Field'],
['enter', 'Name', 'name', 'Field'],
['leave', 'Name', 'name', 'Field'],
['leave', 'Field', 4, undefined],
['leave', 'SelectionSet', 'selectionSet', 'InlineFragment'],
['leave', 'InlineFragment', 1, undefined],
['enter', 'InlineFragment', 2, undefined],
Expand Down
2 changes: 1 addition & 1 deletion src/language/ast.ts
Expand Up @@ -285,7 +285,7 @@ export type SelectionSetNode = {

export type SelectionNode = FieldNode | FragmentSpreadNode | InlineFragmentNode;

type RequiredStatus = 'required' | 'optional' | 'unset';
export type RequiredStatus = 'required' | 'optional' | 'unset';

export type FieldNode = {
readonly kind: 'Field';
Expand Down
1 change: 1 addition & 0 deletions src/language/index.ts
Expand Up @@ -85,6 +85,7 @@ export type {
UnionTypeExtensionNode,
EnumTypeExtensionNode,
InputObjectTypeExtensionNode,
RequiredStatus,
} from './ast';

export {
Expand Down
2 changes: 2 additions & 0 deletions src/language/lexer.ts
Expand Up @@ -204,6 +204,8 @@ function readToken(lexer: Lexer, prev: Token): Token {
case 56: // 8
case 57: // 9
return readNumber(source, pos, code, line, col, prev);
case 63: // ?
return new Token(TokenKind.QUESTION_MARK, pos, pos + 1, line, col, prev);
case 65: // A
case 66: // B
case 67: // C
Expand Down
12 changes: 9 additions & 3 deletions src/language/parser.ts
Expand Up @@ -393,9 +393,15 @@ export class Parser {
} else {
name = nameOrAlias;
}
const required = this.expectOptionalToken(TokenKind.BANG)
? 'required'
: 'unset';

let required;
if (this.expectOptionalToken(TokenKind.BANG)) {
required = 'required'
} else if (this.expectOptionalToken(TokenKind.QUESTION_MARK)) {
required = 'optional'
} else {
required = 'unset'
}

// @ts-expect-error FIXME
return this.node(start, {
Expand Down
8 changes: 7 additions & 1 deletion src/language/printer.ts
Expand Up @@ -56,7 +56,13 @@ const printDocASTReducer: any = {

Field: {
leave({ alias, name, arguments: args, directives, selectionSet, required }) {
const prefix = wrap('', alias, ': ') + name + (required ? '!' : '');
let prefix = wrap('', alias, ': ') + name;
if (required === 'required') {
prefix += '!'
} else if (required === 'optional') {
prefix += '?'
}

let argsLine = prefix + wrap('(', join(args, ', '), ')');

if (argsLine.length > MAX_LINE_LENGTH) {
Expand Down
1 change: 1 addition & 0 deletions src/language/tokenKind.ts
Expand Up @@ -25,6 +25,7 @@ export const TokenKind = Object.freeze({
STRING: 'String',
BLOCK_STRING: 'BlockString',
COMMENT: 'Comment',
QUESTION_MARK: '?',
} as const);

/**
Expand Down
37 changes: 19 additions & 18 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
Expand Up @@ -10,15 +10,18 @@ import type {
FieldNode,
ArgumentNode,
FragmentDefinitionNode,
RequiredStatus,
} from '../../language/ast';
import { Kind } from '../../language/kinds';
import { print } from '../../language/printer';

import type {
import {
GraphQLNamedType,
GraphQLOutputType,
GraphQLCompositeType,
GraphQLField,
GraphQLNonNull,
getNullableType,
} from '../../type/definition';
import {
getNamedType,
Expand All @@ -34,6 +37,7 @@ import { typeFromAST } from '../../utilities/typeFromAST';
import type { Maybe } from '../../jsutils/Maybe';

import type { ValidationContext } from '../ValidationContext';
import { getIntrospectionQuery } from '../../../old_dts';

function reasonMessage(reason: ConflictReasonMessage): string {
if (Array.isArray(reason)) {
Expand Down Expand Up @@ -584,23 +588,8 @@ function findConflict(
}
}

// For now, fail the validation if the nullability status is different
if (node1.required === 'required') {
// if node1 is marked non-nullable,
if (node2.required !== 'required' && def2 && !isNonNullType(def2.type)) {
// if node2 is not marked non-nullable, and type is known and marked nullable
return [
[responseName, 'they have differing nullability status'],
[node1],
[node2],
];
}
}

if (node2.required === 'required') {
// if node1 is marked non-nullable,
if (node1.required !== 'required' && def1 && !isNonNullType(def1.type)) {
// if node2 is not marked non-nullable, and type is known and marked nullable
if (node1.required !== node2.required) {
if (def1 && def2 && isNonNullWithRequiredStatus(def1.type, node1.required) !== isNonNullWithRequiredStatus(def2.type, node2.required)) {
return [
[responseName, 'they have differing nullability status'],
[node1],
Expand Down Expand Up @@ -697,6 +686,18 @@ function doTypesConflict(
return false;
}

function isNonNullWithRequiredStatus(
type: GraphQLOutputType,
required: RequiredStatus
): boolean {
if (required === 'required') {
return true
} else if (required === 'optional') {
return false
}
return isNonNullType(type)
}

// Given a selection set, return the collection of fields (a mapping of response
// name to field nodes and definitions) as well as a list of fragment names
// referenced via fragment spreads.
Expand Down

0 comments on commit 33aa1dd

Please sign in to comment.