Repository: graphql/graphql-js
Pull request #3287 (2021-10-05 08:59:25)

assertValidName: share character classes with lexer


Patch:
diff --git a/src/language/characterClasses.ts b/src/language/characterClasses.ts
new file mode 100644
index 00000000..4489f9ed
--- /dev/null
+++ b/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;
+}
diff --git a/src/language/lexer.ts b/src/language/lexer.ts
index c6f24092..1f201d0d 100644
--- a/src/language/lexer.ts
+++ b/src/language/lexer.ts
@@ -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.
@@ -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 {
@@ -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;
@@ -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
-  );
-}
diff --git a/src/utilities/assertValidName.ts b/src/utilities/assertValidName.ts
index 7036092d..988a8efa 100644
--- a/src/utilities/assertValidName.ts
+++ b/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.
@@ -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.`,
     );
   }
 }


Test patch:
diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts
index 016ff8a7..8325f694 100644
--- a/src/type/__tests__/validation-test.ts
+++ b/src/type/__tests__/validation-test.ts
@@ -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.',
       },
     ]);
   });
@@ -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.',
       },
     ]);
   });
@@ -968,16 +968,14 @@ 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.',
       },
     ]);
 
@@ -985,7 +983,7 @@ describe('Type System: Enum types must be well defined', () => {
     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.',
       },
     ]);
 
diff --git a/src/utilities/__tests__/assertValidName-test.ts b/src/utilities/__tests__/assertValidName-test.ts
index a794d5fb..96909d58 100644
--- a/src/utilities/__tests__/assertValidName-test.ts
+++ b/src/utilities/__tests__/assertValidName-test.ts
@@ -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.',
+    );
   });
 });