feat(validation): reject directive definition cycles

src/index.ts

@@ -444,6 +444,7 @@ export {
   UniqueFieldDefinitionNamesRule,
   UniqueArgumentDefinitionNamesRule,
   UniqueDirectiveNamesRule,
+  NoDirectiveDefinitionCyclesRule,
   PossibleTypeExtensionsRule,
   // Custom validation rules
   NoDeprecatedCustomRule,

src/utilities/__tests__/extendSchema-test.ts

@@ -315,7 +315,7 @@ describe('extendSchema', () => {
         someScalar(arg: SomeScalar): SomeScalar
       }
 
-      directive @foo(arg: SomeScalar) on SCALAR
+      directive @foo on SCALAR
 
       input FooInput {
         foo: SomeScalar

src/validation/__tests__/NoDirectiveDefinitionCyclesRule-test.ts

@@ -0,0 +1,266 @@
+import { describe, it } from 'mocha';
+
+import { expectJSON } from '../../__testUtils__/expectJSON.js';
+
+import { parse } from '../../language/parser.js';
+
+import type { GraphQLSchema } from '../../type/schema.js';
+
+import { buildSchema } from '../../utilities/buildASTSchema.js';
+
+import { NoDirectiveDefinitionCyclesRule } from '../rules/NoDirectiveDefinitionCyclesRule.js';
+import { validateSDL } from '../validate.js';
+
+function expectErrors(
+  sdlStr: string,
+  schema?: GraphQLSchema,
+  parseOptions?: { experimentalDirectivesOnDirectiveDefinitions?: boolean },
+) {
+  const doc = parse(sdlStr, parseOptions);
+  const errors = validateSDL(doc, schema, [NoDirectiveDefinitionCyclesRule]);
+  return expectJSON(errors);
+}
+
+function expectValid(
+  sdlStr: string,
+  schema?: GraphQLSchema,
+  parseOptions?: { experimentalDirectivesOnDirectiveDefinitions?: boolean },
+) {
+  expectErrors(sdlStr, schema, parseOptions).toDeepEqual([]);
+}
+
+describe('Validate: No directive definition cycles', () => {
+  it('single reference is valid', () => {
+    expectValid(`
+      directive @a(arg: String @b) on FIELD_DEFINITION
+      directive @b on ARGUMENT_DEFINITION
+    `);
+  });
+
+  it('does not false positive on unknown directive', () => {
+    expectValid(`
+      directive @a(arg: String @unknown) on FIELD_DEFINITION
+    `);
+  });
+
+  it('rejects a self-referential directive definition', () => {
+    expectErrors(`
+      directive @self(arg: String @self) on FIELD_DEFINITION
+    `).toDeepEqual([
+      {
+        message: 'Cannot reference directive "@self" within itself.',
+        locations: [{ line: 2, column: 35 }],
+      },
+    ]);
+  });
+
+  it('rejects directive definitions with circular references', () => {
+    expectErrors(`
+      directive @a(arg: String @b) on FIELD_DEFINITION
+      directive @b(arg: String @a) on FIELD_DEFINITION
+    `).toDeepEqual([
+      {
+        message:
+          'Cannot reference directive "@a" within itself through a series of directive applications: "@b", "@a".',
+        locations: [
+          { line: 2, column: 32 },
+          { line: 3, column: 32 },
+        ],
+      },
+    ]);
+  });
+
+  it('rejects directive definitions with overlapping circular references', () => {
+    expectErrors(`
+      directive @a(arg: String @b) on FIELD_DEFINITION
+      directive @b(arg: String @c) on FIELD_DEFINITION
+      directive @c(first: String @a, second: String @d) on FIELD_DEFINITION
+      directive @d(arg: String @b) on FIELD_DEFINITION
+    `).toDeepEqual([
+      {
+        message:
+          'Cannot reference directive "@a" within itself through a series of directive applications: "@b", "@c", "@a".',
+        locations: [
+          { line: 2, column: 32 },
+          { line: 3, column: 32 },
+          { line: 4, column: 34 },
+        ],
+      },
+      {
+        message:
+          'Cannot reference directive "@b" within itself through a series of directive applications: "@c", "@d", "@b".',
+        locations: [
+          { line: 3, column: 32 },
+          { line: 4, column: 53 },
+          { line: 5, column: 32 },
+        ],
+      },
+    ]);
+  });
+
+  it('rejects directive definitions that recurse through a referenced type', () => {
+    expectErrors(`
+      directive @a(arg: InputObject) on FIELD_DEFINITION
+
+      input InputObject {
+        value: String @a
+      }
+    `).toDeepEqual([
+      {
+        message:
+          'Cannot reference directive "@a" within itself through a series of references: "InputObject", "@a".',
+        locations: [
+          { line: 2, column: 25 },
+          { line: 5, column: 23 },
+        ],
+      },
+    ]);
+  });
+
+  it('does not duplicate cycles through recursive referenced types', () => {
+    expectErrors(`
+      directive @a(arg: InputObject) on INPUT_FIELD_DEFINITION
+      input InputObject {
+        self: InputObject @a
+      }
+    `).toDeepEqual([
+      {
+        message:
+          'Cannot reference directive "@a" within itself through a series of references: "InputObject", "@a".',
+        locations: [
+          { line: 2, column: 25 },
+          { line: 4, column: 27 },
+        ],
+      },
+    ]);
+  });
+
+  it('rejects type extensions that create cycles with existing directives', () => {
+    const schema = buildSchema(
+      `
+        directive @a(arg: InputObject) on INPUT_FIELD_DEFINITION
+        input InputObject {
+          value: String
+        }
+      `,
+      { noLocation: true },
+    );
+
+    expectErrors(
+      `
+        extend input InputObject {
+          recursive: String @a
+        }
+      `,
+      schema,
+    ).toDeepEqual([
+      {
+        message:
+          'Cannot reference directive "@a" within itself through a series of references: "InputObject", "@a".',
+        locations: [{ line: 3, column: 29 }],
+      },
+    ]);
+  });
+
+  it('rejects directives on directive definitions when the syntax exists', () => {
+    expectErrors(
+      `
+        directive @a @b on DIRECTIVE_DEFINITION
+        directive @b @a on DIRECTIVE_DEFINITION
+      `,
+      undefined,
+      {
+        experimentalDirectivesOnDirectiveDefinitions: true,
+      },
+    ).toDeepEqual([
+      {
+        message:
+          'Cannot reference directive "@a" within itself through a series of directive applications: "@b", "@a".',
+        locations: [
+          { line: 2, column: 22 },
+          { line: 3, column: 22 },
+        ],
+      },
+    ]);
+  });
+
+  it('rejects directive extensions with circular references', () => {
+    const schema = buildSchema(
+      `
+        directive @a on DIRECTIVE_DEFINITION
+        directive @b on DIRECTIVE_DEFINITION
+      `,
+      {
+        noLocation: true,
+        experimentalDirectivesOnDirectiveDefinitions: true,
+      },
+    );
+
+    expectErrors(
+      `
+        extend directive @a @b
+        extend directive @b @a
+      `,
+      schema,
+      {
+        experimentalDirectivesOnDirectiveDefinitions: true,
+      },
+    ).toDeepEqual([
+      {
+        message:
+          'Cannot reference directive "@a" within itself through a series of directive applications: "@b", "@a".',
+        locations: [
+          { line: 2, column: 29 },
+          { line: 3, column: 29 },
+        ],
+      },
+    ]);
+  });
+
+  it('ignores directive applications from the schema being extended', () => {
+    const schema = buildSchema(
+      `
+        directive @a @b on DIRECTIVE_DEFINITION
+        directive @b on DIRECTIVE_DEFINITION
+      `,
+      {
+        noLocation: true,
+        experimentalDirectivesOnDirectiveDefinitions: true,
+      },
+    );
+
+    expectValid(
+      `
+        extend directive @b @a
+      `,
+      schema,
+      {
+        experimentalDirectivesOnDirectiveDefinitions: true,
+      },
+    );
+  });
+
+  it('only considers applied directives from the SDL document being validated', () => {
+    const schema = buildSchema(`
+      directive @a(arg: InputObject) on INPUT_FIELD_DEFINITION
+      input InputObject {
+        field: String @b
+      }
+      directive @b on INPUT_FIELD_DEFINITION
+    `);
+
+    // `schemaToExtend` still contributes the directive argument reference
+    // from `@a` to `InputObject`, but the already-applied `@b` on
+    // `InputObject.field` is outside the current SDL document's validation
+    // surface. Counting it would incorrectly report `@b -> @a -> InputObject -> @b`.
+    expectValid(
+      `
+        extend directive @b @a
+      `,
+      schema,
+      {
+        experimentalDirectivesOnDirectiveDefinitions: true,
+      },
+    );
+  });
+});

src/validation/index.ts

@@ -109,6 +109,7 @@ export { UniqueEnumValueNamesRule } from './rules/UniqueEnumValueNamesRule.js';
 export { UniqueFieldDefinitionNamesRule } from './rules/UniqueFieldDefinitionNamesRule.js';
 export { UniqueArgumentDefinitionNamesRule } from './rules/UniqueArgumentDefinitionNamesRule.js';
 export { UniqueDirectiveNamesRule } from './rules/UniqueDirectiveNamesRule.js';
+export { NoDirectiveDefinitionCyclesRule } from './rules/NoDirectiveDefinitionCyclesRule.js';
 export { PossibleTypeExtensionsRule } from './rules/PossibleTypeExtensionsRule.js';
 
 // Optional rules not defined by the GraphQL Specification