Skip to content
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

assertValidName: share character classes with lexer #3287

Merged
merged 1 commit into from Oct 5, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 52 additions & 0 deletions src/language/characterClasses.ts
@@ -0,0 +1,52 @@
/**
* ```
* Digit :: one of
* - `0` `1` `2` `3` `4` `5` `6` `7` `8` `9`
* ```
* @internal
*/
export function isDigit(code: number): boolean {
return code >= 0x0030 && code <= 0x0039;
}

/**
* ```
* Letter :: one of
* - `A` `B` `C` `D` `E` `F` `G` `H` `I` `J` `K` `L` `M`
* - `N` `O` `P` `Q` `R` `S` `T` `U` `V` `W` `X` `Y` `Z`
* - `a` `b` `c` `d` `e` `f` `g` `h` `i` `j` `k` `l` `m`
* - `n` `o` `p` `q` `r` `s` `t` `u` `v` `w` `x` `y` `z`
* ```
* @internal
*/
export function isLetter(code: number): boolean {
return (
(code >= 0x0061 && code <= 0x007a) || // A-Z
(code >= 0x0041 && code <= 0x005a) // a-z
);
}

/**
* ```
* NameStart ::
* - Letter
* - `_`
* ```
* @internal
*/
export function isNameStart(code: number): boolean {
return isLetter(code) || code === 0x005f;
}

/**
* ```
* NameContinue ::
* - Letter
* - Digit
* - `_`
* ```
* @internal
*/
export function isNameContinue(code: number): boolean {
return isLetter(code) || isDigit(code) || code === 0x005f;
}
43 changes: 2 additions & 41 deletions src/language/lexer.ts
Expand Up @@ -5,6 +5,7 @@ import type { TokenKindEnum } from './tokenKind';
import { Token } from './ast';
import { TokenKind } from './tokenKind';
import { dedentBlockStringValue } from './blockString';
import { isDigit, isNameStart, isNameContinue } from './characterClasses';

/**
* Given a Source object, creates a Lexer for that source.
Expand Down Expand Up @@ -836,15 +837,6 @@ function readBlockString(lexer: Lexer, start: number): Token {
* ```
* Name ::
* - NameStart NameContinue* [lookahead != NameContinue]
*
* NameStart ::
* - Letter
* - `_`
*
* NameContinue ::
* - Letter
* - Digit
* - `_`
* ```
*/
function readName(lexer: Lexer, start: number): Token {
Expand All @@ -854,8 +846,7 @@ function readName(lexer: Lexer, start: number): Token {

while (position < bodyLength) {
const code = body.charCodeAt(position);
// NameContinue
if (isLetter(code) || isDigit(code) || code === 0x005f) {
if (isNameContinue(code)) {
++position;
} else {
break;
Expand All @@ -870,33 +861,3 @@ function readName(lexer: Lexer, start: number): Token {
body.slice(start, position),
);
}

function isNameStart(code: number): boolean {
return isLetter(code) || code === 0x005f;
}

/**
* ```
* Digit :: one of
* - `0` `1` `2` `3` `4` `5` `6` `7` `8` `9`
* ```
*/
function isDigit(code: number): boolean {
return code >= 0x0030 && code <= 0x0039;
}

/**
* ```
* Letter :: one of
* - `A` `B` `C` `D` `E` `F` `G` `H` `I` `J` `K` `L` `M`
* - `N` `O` `P` `Q` `R` `S` `T` `U` `V` `W` `X` `Y` `Z`
* - `a` `b` `c` `d` `e` `f` `g` `h` `i` `j` `k` `l` `m`
* - `n` `o` `p` `q` `r` `s` `t` `u` `v` `w` `x` `y` `z`
* ```
*/
function isLetter(code: number): boolean {
return (
(code >= 0x0061 && code <= 0x007a) || // A-Z
(code >= 0x0041 && code <= 0x005a) // a-z
);
}
12 changes: 5 additions & 7 deletions src/type/__tests__/validation-test.ts
Expand Up @@ -493,7 +493,7 @@ describe('Type System: Objects must have fields', () => {
expectJSON(validateSchema(schema)).to.deep.equal([
{
message:
'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "bad-name-with-dashes" does not.',
'Names must only contain [_a-zA-Z0-9] but "bad-name-with-dashes" does not.',
},
]);
});
Expand Down Expand Up @@ -535,7 +535,7 @@ describe('Type System: Fields args must be properly named', () => {
expectJSON(validateSchema(schema)).to.deep.equal([
{
message:
'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "bad-name-with-dashes" does not.',
'Names must only contain [_a-zA-Z0-9] but "bad-name-with-dashes" does not.',
},
]);
});
Expand Down Expand Up @@ -968,24 +968,22 @@ describe('Type System: Enum types must be well defined', () => {
const schema1 = schemaWithEnum({ '#value': {} });
expectJSON(validateSchema(schema1)).to.deep.equal([
{
message:
'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "#value" does not.',
message: 'Names must start with [_a-zA-Z] but "#value" does not.',
},
]);

const schema2 = schemaWithEnum({ '1value': {} });
expectJSON(validateSchema(schema2)).to.deep.equal([
{
message:
'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "1value" does not.',
message: 'Names must start with [_a-zA-Z] but "1value" does not.',
},
]);

const schema3 = schemaWithEnum({ 'KEBAB-CASE': {} });
expectJSON(validateSchema(schema3)).to.deep.equal([
{
message:
'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "KEBAB-CASE" does not.',
'Names must only contain [_a-zA-Z0-9] but "KEBAB-CASE" does not.',
},
]);

Expand Down
16 changes: 15 additions & 1 deletion src/utilities/__tests__/assertValidName-test.ts
Expand Up @@ -19,7 +19,21 @@ describe('assertValidName()', () => {
expect(() => assertValidName({})).to.throw('Expected name to be a string.');
});

it('throws on empty strings', () => {
expect(() => assertValidName('')).to.throw(
'Expected name to be a non-empty string.',
);
});

it('throws for names with invalid characters', () => {
expect(() => assertValidName('>--()-->')).to.throw(/Names must match/);
expect(() => assertValidName('>--()-->')).to.throw(
'Names must only contain [_a-zA-Z0-9] but ">--()-->" does not.',
);
});

it('throws for names starting with invalid characters', () => {
expect(() => assertValidName('42MeaningsOfLife')).to.throw(
'Names must start with [_a-zA-Z] but "42MeaningsOfLife" does not.',
);
});
});
21 changes: 17 additions & 4 deletions src/utilities/assertValidName.ts
@@ -1,8 +1,7 @@
import { devAssert } from '../jsutils/devAssert';

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

const NAME_RX = /^[_a-zA-Z][_a-zA-Z0-9]*$/;
import { isNameStart, isNameContinue } from '../language/characterClasses';

/**
* Upholds the spec rules about naming.
Expand All @@ -20,14 +19,28 @@ export function assertValidName(name: string): string {
*/
export function isValidNameError(name: string): GraphQLError | undefined {
devAssert(typeof name === 'string', 'Expected name to be a string.');

if (name.startsWith('__')) {
return new GraphQLError(
`Name "${name}" must not begin with "__", which is reserved by GraphQL introspection.`,
);
}
if (!NAME_RX.test(name)) {

if (name.length === 0) {
return new GraphQLError('Expected name to be a non-empty string.');
}

for (let i = 1; i < name.length; ++i) {
if (!isNameContinue(name.charCodeAt(i))) {
return new GraphQLError(
`Names must only contain [_a-zA-Z0-9] but "${name}" does not.`,
);
}
}

if (!isNameStart(name.charCodeAt(0))) {
return new GraphQLError(
`Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "${name}" does not.`,
`Names must start with [_a-zA-Z] but "${name}" does not.`,
);
}
}