Extract check for unique enum value names into separate rule

src/type/__tests__/validation-test.js

@@ -790,25 +790,6 @@ describe('Type System: Enum types must be well defined', () => {
     ]);
   });
 
-  it('rejects an Enum type with duplicate values', () => {
-    const schema = buildSchema(`
-      type Query {
-        field: SomeEnum
-      }
-
-      enum SomeEnum {
-        SOME_VALUE
-        SOME_VALUE
-      }
-    `);
-    expect(validateSchema(schema)).to.deep.equal([
-      {
-        message: 'Enum type SomeEnum can include value SOME_VALUE only once.',
-        locations: [{ line: 7, column: 9 }, { line: 8, column: 9 }],
-      },
-    ]);
-  });
-
   it('rejects an Enum type with incorrectly named values', () => {
     function schemaWithEnum(name) {
       return schemaWithFieldType(

src/type/validate.js

@@ -37,7 +37,6 @@ import { GraphQLError } from '../error/GraphQLError';
 import type {
   ASTNode,
   FieldDefinitionNode,
-  EnumValueDefinitionNode,
   InputValueDefinitionNode,
   NamedTypeNode,
   TypeNode,
@@ -499,15 +498,6 @@ function validateEnumValues(
   for (const enumValue of enumValues) {
     const valueName = enumValue.name;
 
-    // Ensure no duplicates.
-    const allNodes = getEnumValueNodes(enumType, valueName);
-    if (allNodes && allNodes.length > 1) {
-      context.reportError(
-        `Enum type ${enumType.name} can include value ${valueName} only once.`,
-        allNodes,
-      );
-    }
-
     // Ensure valid name.
     validateName(context, enumValue);
     if (valueName === 'true' || valueName === 'false' || valueName === 'null') {
@@ -682,12 +672,3 @@ function getUnionMemberTypeNodes(
     typeNode => typeNode.name.value === typeName,
   );
 }
-
-function getEnumValueNodes(
-  enumType: GraphQLEnumType,
-  valueName: string,
-): ?$ReadOnlyArray<EnumValueDefinitionNode> {
-  return getAllSubNodes(enumType, enumNode => enumNode.values).filter(
-    valueNode => valueNode.name.value === valueName,
-  );
-}

src/utilities/extendSchema.js

@@ -8,7 +8,6 @@
  */
 
 import invariant from '../jsutils/invariant';
-import keyMap from '../jsutils/keyMap';
 import keyValMap from '../jsutils/keyValMap';
 import objectValues from '../jsutils/objectValues';
 import { ASTDefinitionBuilder } from './buildASTSchema';
@@ -353,11 +352,8 @@ export function extendSchema(
 
   function extendValueMap(type: GraphQLEnumType) {
     const newValueMap = Object.create(null);
-    const oldValueMap = keyMap(type.getValues(), value => value.name);
-    for (const valueName of Object.keys(oldValueMap)) {
-      const value = oldValueMap[valueName];
-      newValueMap[valueName] = {
-        name: value.name,
+    for (const value of type.getValues()) {
+      newValueMap[value.name] = {
         description: value.description,
         value: value.value,
         deprecationReason: value.deprecationReason,
@@ -370,15 +366,7 @@ export function extendSchema(
     if (extensions) {
       for (const extension of extensions) {
         for (const value of extension.values) {
-          const valueName = value.name.value;
-          if (oldValueMap[valueName]) {
-            throw new GraphQLError(
-              `Enum value "${type.name}.${valueName}" already exists in the ` +
-                'schema. It cannot also be defined in this type extension.',
-              [value],
-            );
-          }
-          newValueMap[valueName] = astBuilder.buildEnumValue(value);
+          newValueMap[value.name.value] = astBuilder.buildEnumValue(value);
         }
       }
     }

src/validation/__tests__/UniqueEnumValueNames-test.js

@@ -0,0 +1,170 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict
+ */
+
+import { describe, it } from 'mocha';
+import { buildSchema } from '../../utilities';
+import { expectSDLValidationErrors } from './harness';
+import {
+  UniqueEnumValueNames,
+  duplicateEnumValueNameMessage,
+  existedEnumValueNameMessage,
+} from '../rules/UniqueEnumValueNames';
+
+function expectSDLErrors(sdlStr, schema) {
+  return expectSDLValidationErrors(schema, UniqueEnumValueNames, sdlStr);
+}
+
+function expectValidSDL(sdlStr, schema) {
+  expectSDLErrors(sdlStr, schema).to.deep.equal([]);
+}
+
+function duplicateEnumValuesName(typeName, valueName, l1, c1, l2, c2) {
+  return {
+    message: duplicateEnumValueNameMessage(typeName, valueName),
+    locations: [{ line: l1, column: c1 }, { line: l2, column: c2 }],
+  };
+}
+
+function existedEnumValueName(typeName, valueName, l1, c1) {
+  return {
+    message: existedEnumValueNameMessage(typeName, valueName),
+    locations: [{ line: l1, column: c1 }],
+  };
+}
+
+describe('Validate: Unique enum value names', () => {
+  it('no values', () => {
+    expectValidSDL(`
+      enum SomeEnum
+    `);
+  });
+
+  it('one value', () => {
+    expectValidSDL(`
+      enum SomeEnum {
+        FOO
+      }
+    `);
+  });
+
+  it('multiple values', () => {
+    expectValidSDL(`
+      enum SomeEnum {
+        FOO
+        BAR
+      }
+    `);
+  });
+
+  it('duplicate values inside the same enum definition', () => {
+    expectSDLErrors(`
+      enum SomeEnum {
+        FOO
+        BAR
+        FOO
+      }
+    `).to.deep.equal([duplicateEnumValuesName('SomeEnum', 'FOO', 3, 9, 5, 9)]);
+  });
+
+  it('extend enum with new value', () => {
+    expectValidSDL(`
+      enum SomeEnum {
+        FOO
+      }
+      extend enum SomeEnum {
+        BAR
+      }
+      extend enum SomeEnum {
+        BAZ
+      }
+    `);
+  });
+
+  it('extend enum with duplicate value', () => {
+    expectSDLErrors(`
+      extend enum SomeEnum {
+        FOO
+      }
+      enum SomeEnum {
+        FOO
+      }
+    `).to.deep.equal([duplicateEnumValuesName('SomeEnum', 'FOO', 3, 9, 6, 9)]);
+  });
+
+  it('duplicate value inside extension', () => {
+    expectSDLErrors(`
+      enum SomeEnum
+      extend enum SomeEnum {
+        FOO
+        BAR
+        FOO
+      }
+    `).to.deep.equal([duplicateEnumValuesName('SomeEnum', 'FOO', 4, 9, 6, 9)]);
+  });
+
+  it('duplicate value inside different extensions', () => {
+    expectSDLErrors(`
+      enum SomeEnum
+      extend enum SomeEnum {
+        FOO
+      }
+      extend enum SomeEnum {
+        FOO
+      }
+    `).to.deep.equal([duplicateEnumValuesName('SomeEnum', 'FOO', 4, 9, 7, 9)]);
+  });
+
+  it('adding new value to the type inside existing schema', () => {
+    const schema = buildSchema('enum SomeEnum');
+    const sdl = `
+      extend enum SomeEnum {
+        FOO
+      }
+    `;
+
+    expectValidSDL(sdl, schema);
+  });
+
+  it('adding conflicting value to existing schema twice', () => {
+    const schema = buildSchema(`
+      enum SomeEnum {
+        FOO
+      }
+    `);
+    const sdl = `
+      extend enum SomeEnum {
+        FOO
+      }
+      extend enum SomeEnum {
+        FOO
+      }
+    `;
+
+    expectSDLErrors(sdl, schema).to.deep.equal([
+      existedEnumValueName('SomeEnum', 'FOO', 3, 9),
+      existedEnumValueName('SomeEnum', 'FOO', 6, 9),
+    ]);
+  });
+
+  it('adding fields to existing schema twice', () => {
+    const schema = buildSchema('enum SomeEnum');
+    const sdl = `
+      extend enum SomeEnum {
+        FOO
+      }
+      extend enum SomeEnum {
+        FOO
+      }
+    `;
+
+    expectSDLErrors(sdl, schema).to.deep.equal([
+      duplicateEnumValuesName('SomeEnum', 'FOO', 3, 9, 6, 9),
+    ]);
+  });
+});

src/validation/rules/UniqueEnumValueNames.js

@@ -0,0 +1,87 @@
+/**
+ * Copyright (c) 2018-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict
+ */
+
+import type { SDLValidationContext } from '../ValidationContext';
+import { GraphQLError } from '../../error/GraphQLError';
+import type { ASTVisitor } from '../../language/visitor';
+import { isEnumType } from '../../type/definition';
+
+export function duplicateEnumValueNameMessage(
+  typeName: string,
+  valueName: string,
+): string {
+  return `Enum value "${typeName}.${valueName}" can only be defined once.`;
+}
+
+export function existedEnumValueNameMessage(
+  typeName: string,
+  valueName: string,
+): string {
+  return (
+    `Enum value "${typeName}.${valueName}" already exists in the schema. ` +
+    'It cannot also be defined in this type extension.'
+  );
+}
+
+/**
+ * Unique enum value names
+ *
+ * A GraphQL enum type is only valid if all its values are uniquely named.
+ */
+export function UniqueEnumValueNames(
+  context: SDLValidationContext,
+): ASTVisitor {
+  const schema = context.getSchema();
+  const existingTypeMap = schema ? schema.getTypeMap() : Object.create(null);
+  const knownValueNames = Object.create(null);
+
+  return {
+    EnumTypeDefinition: checkValueUniqueness,
+    EnumTypeExtension: checkValueUniqueness,
+  };
+
+  function checkValueUniqueness(node) {
+    const typeName = node.name.value;
+
+    if (!knownValueNames[typeName]) {
+      knownValueNames[typeName] = Object.create(null);
+    }
+
+    const valueNames = knownValueNames[typeName];
+    if (node.values) {
+      for (const valueDef of node.values) {
+        const valueName = valueDef.name.value;
+
+        const existingType = existingTypeMap[typeName];
+        if (isEnumType(existingType) && existingType.getValue(valueName)) {
+          context.reportError(
+            new GraphQLError(
+              existedEnumValueNameMessage(typeName, valueName),
+              valueDef.name,
+            ),
+          );
+          continue;
+        }
+
+        if (valueNames[valueName]) {
+          context.reportError(
+            new GraphQLError(
+              duplicateEnumValueNameMessage(typeName, valueName),
+              [valueNames[valueName], valueDef.name],
+            ),
+          );
+        } else {
+          valueNames[valueName] = valueDef.name;
+        }
+      }
+    }
+
+    return false;
+  }
+}

src/validation/specifiedRules.js

@@ -125,6 +125,7 @@ export const specifiedRules: $ReadOnlyArray<ValidationRule> = [
 import { LoneSchemaDefinition } from './rules/LoneSchemaDefinition';
 import { UniqueOperationTypes } from './rules/UniqueOperationTypes';
 import { UniqueTypeNames } from './rules/UniqueTypeNames';
+import { UniqueEnumValueNames } from './rules/UniqueEnumValueNames';
 import { UniqueFieldDefinitionNames } from './rules/UniqueFieldDefinitionNames';
 import { UniqueDirectiveNames } from './rules/UniqueDirectiveNames';
 import { KnownArgumentNamesOnDirectives } from './rules/KnownArgumentNames';
@@ -135,6 +136,7 @@ export const specifiedSDLRules: $ReadOnlyArray<SDLValidationRule> = [
   LoneSchemaDefinition,
   UniqueOperationTypes,
   UniqueTypeNames,
+  UniqueEnumValueNames,
   UniqueFieldDefinitionNames,
   UniqueDirectiveNames,
   KnownTypeNames,