Add fragment arguments to AST and related utility functions

mjmahone · mjmahone · commit 5b1236702d9e · 2021-06-01T11:11:59.000-04:00
diff --git a/src/language/__tests__/parser-test.ts b/src/language/__tests__/parser-test.ts
@@ -363,11 +363,20 @@ describe('Parser', () => {
     expect('loc' in result).to.equal(false);
   });
 
-  it('Legacy: allows parsing fragment defined variables', () => {
+  it('allows parsing fragment defined variables', () => {
     const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';
 
     expect(() =>
-      parse(document, { allowLegacyFragmentVariables: true }),
+      parse(document, { allowFragmentArguments: true }),
+    ).to.not.throw();
+    expect(() => parse(document)).to.throw('Syntax Error');
+  });
+
+  it('allows parsing fragment spread arguments', () => {
+    const document = 'fragment a on t { ...b(v: $v) }';
+
+    expect(() =>
+      parse(document, { allowFragmentArguments: true }),
     ).to.not.throw();
     expect(() => parse(document)).to.throw('Syntax Error');
   });
diff --git a/src/language/__tests__/printer-test.ts b/src/language/__tests__/printer-test.ts
@@ -113,10 +113,10 @@ describe('Printer: Query document', () => {
     );
   });
 
-  it('Legacy: prints fragment with variable directives', () => {
+  it('prints fragment with variable directives', () => {
     const queryASTWithVariableDirective = parse(
       'fragment Foo($foo: TestType @test) on TestType @testDirective { id }',
-      { allowLegacyFragmentVariables: true },
+      { allowFragmentArguments: true },
     );
     expect(print(queryASTWithVariableDirective)).to.equal(dedent`
       fragment Foo($foo: TestType @test) on TestType @testDirective {
@@ -125,14 +125,14 @@ describe('Printer: Query document', () => {
     `);
   });
 
-  it('Legacy: correctly prints fragment defined variables', () => {
+  it('correctly prints fragment defined variables', () => {
     const fragmentWithVariable = parse(
       `
         fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
           id
         }
       `,
-      { allowLegacyFragmentVariables: true },
+      { allowFragmentArguments: true },
     );
     expect(print(fragmentWithVariable)).to.equal(dedent`
       fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
@@ -141,6 +141,34 @@ describe('Printer: Query document', () => {
     `);
   });
 
+  it('prints fragment spread with arguments', () => {
+    const queryASTWithVariableDirective = parse(
+      'fragment Foo on TestType { ...Bar(a: {x: $x}, b: true) }',
+      { allowFragmentArguments: true },
+    );
+    expect(print(queryASTWithVariableDirective)).to.equal(dedent`
+      fragment Foo on TestType {
+        ...Bar(a: {x: $x}, b: true)
+      }
+    `);
+  });
+
+  it('prints fragment spread with multi-line arguments', () => {
+    const queryASTWithVariableDirective = parse(
+      'fragment Foo on TestType { ...Bar(a: {x: $x, y: $y, z: $z, xy: $xy}, b: true, c: "a long string extending arguments over max length") }',
+      { allowFragmentArguments: true },
+    );
+    expect(print(queryASTWithVariableDirective)).to.equal(dedent`
+      fragment Foo on TestType {
+        ...Bar(
+          a: {x: $x, y: $y, z: $z, xy: $xy}
+          b: true
+          c: "a long string extending arguments over max length"
+        )
+      }
+    `);
+  });
+
   it('prints kitchen sink without altering ast', () => {
     const ast = parse(kitchenSinkQuery, { noLocation: true });
 
diff --git a/src/language/__tests__/visitor-test.ts b/src/language/__tests__/visitor-test.ts
@@ -462,10 +462,10 @@ describe('Visitor', () => {
     ]);
   });
 
-  it('Legacy: visits variables defined in fragments', () => {
+  it('visits variables defined in fragments', () => {
     const ast = parse('fragment a($v: Boolean = false) on t { f }', {
       noLocation: true,
-      allowLegacyFragmentVariables: true,
+      allowFragmentArguments: true,
     });
     const visited: Array<any> = [];
 
@@ -512,6 +512,50 @@ describe('Visitor', () => {
     ]);
   });
 
+  it('visits arguments on fragment spreads', () => {
+    const ast = parse('fragment a on t { ...s(v: false) }', {
+      noLocation: true,
+      allowFragmentArguments: true,
+    });
+    const visited: Array<any> = [];
+
+    visit(ast, {
+      enter(node) {
+        checkVisitorFnArgs(ast, arguments);
+        visited.push(['enter', node.kind, getValue(node)]);
+      },
+      leave(node) {
+        checkVisitorFnArgs(ast, arguments);
+        visited.push(['leave', node.kind, getValue(node)]);
+      },
+    });
+
+    expect(visited).to.deep.equal([
+      ['enter', 'Document', undefined],
+      ['enter', 'FragmentDefinition', undefined],
+      ['enter', 'Name', 'a'],
+      ['leave', 'Name', 'a'],
+      ['enter', 'NamedType', undefined],
+      ['enter', 'Name', 't'],
+      ['leave', 'Name', 't'],
+      ['leave', 'NamedType', undefined],
+      ['enter', 'SelectionSet', undefined],
+      ['enter', 'FragmentSpread', undefined],
+      ['enter', 'Name', 's'],
+      ['leave', 'Name', 's'],
+      ['enter', 'Argument', { kind: 'BooleanValue', value: false }],
+      ['enter', 'Name', 'v'],
+      ['leave', 'Name', 'v'],
+      ['enter', 'BooleanValue', false],
+      ['leave', 'BooleanValue', false],
+      ['leave', 'Argument', { kind: 'BooleanValue', value: false }],
+      ['leave', 'FragmentSpread', undefined],
+      ['leave', 'SelectionSet', undefined],
+      ['leave', 'FragmentDefinition', undefined],
+      ['leave', 'Document', undefined],
+    ]);
+  });
+
   it('visits kitchen sink', () => {
     const ast = parse(kitchenSinkQuery);
     const visited: Array<any> = [];
diff --git a/src/language/ast.ts b/src/language/ast.ts
@@ -318,6 +318,7 @@ export interface FragmentSpreadNode {
   readonly kind: 'FragmentSpread';
   readonly loc?: Location;
   readonly name: NameNode;
+  readonly arguments?: ReadonlyArray<ArgumentNode>;
   readonly directives?: ReadonlyArray<DirectiveNode>;
 }
 
@@ -333,7 +334,6 @@ export interface FragmentDefinitionNode {
   readonly kind: 'FragmentDefinition';
   readonly loc?: Location;
   readonly name: NameNode;
-  /** @deprecated variableDefinitions will be removed in v17.0.0 */
   readonly variableDefinitions?: ReadonlyArray<VariableDefinitionNode>;
   readonly typeCondition: NamedTypeNode;
   readonly directives?: ReadonlyArray<DirectiveNode>;
diff --git a/src/language/parser.ts b/src/language/parser.ts
@@ -82,20 +82,22 @@ export interface ParseOptions {
   noLocation?: boolean;
 
   /**
-   * @deprecated will be removed in the v17.0.0
-   *
    * If enabled, the parser will understand and parse variable definitions
    * contained in a fragment definition. They'll be represented in the
    * `variableDefinitions` field of the FragmentDefinitionNode.
    *
+   * Additionally, the parser will understand arguments passed to fragment
+   * spreads. They'll be represented in the `arguments` field of the
+   * FragmentSpreadNode.
+   *
    * The syntax is identical to normal, query-defined variables. For example:
    *
    *   fragment A($var: Boolean = false) on T  {
    *     ...
    *   }
    *
    */
-  allowLegacyFragmentVariables?: boolean;
+  allowFragmentArguments?: boolean;
 }
 
 /**
@@ -437,7 +439,7 @@ export class Parser {
   /**
    * Corresponds to both FragmentSpread and InlineFragment in the spec.
    *
-   * FragmentSpread : ... FragmentName Directives?
+   * FragmentSpread : ... FragmentName Arguments? Directives?
    *
    * InlineFragment : ... TypeCondition? Directives? SelectionSet
    */
@@ -447,6 +449,14 @@ export class Parser {
 
     const hasTypeCondition = this.expectOptionalKeyword('on');
     if (!hasTypeCondition && this.peek(TokenKind.NAME)) {
+      if (this._options?.allowFragmentArguments === true) {
+        return this.node<FragmentSpreadNode>(start, {
+          kind: Kind.FRAGMENT_SPREAD,
+          name: this.parseFragmentName(),
+          arguments: this.parseArguments(false),
+          directives: this.parseDirectives(false),
+        });
+      }
       return this.node<FragmentSpreadNode>(start, {
         kind: Kind.FRAGMENT_SPREAD,
         name: this.parseFragmentName(),
@@ -473,7 +483,7 @@ export class Parser {
     // Legacy support for defining variables within fragments changes
     // the grammar of FragmentDefinition:
     //   - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet
-    if (this._options?.allowLegacyFragmentVariables === true) {
+    if (this._options?.allowFragmentArguments === true) {
       return this.node<FragmentDefinitionNode>(start, {
         kind: Kind.FRAGMENT_DEFINITION,
         name: this.parseFragmentName(),
diff --git a/src/language/printer.ts b/src/language/printer.ts
@@ -71,8 +71,16 @@ const printDocASTReducer: ASTReducer<string> = {
   // Fragments
 
   FragmentSpread: {
-    leave: ({ name, directives }) =>
-      '...' + name + wrap(' ', join(directives, ' ')),
+    leave: ({ name, arguments: args, directives }) => {
+      const prefix = '...' + name;
+      let argsLine = prefix + wrap('(', join(args, ', '), ')');
+
+      if (argsLine.length > MAX_LINE_LENGTH) {
+        argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)');
+      }
+
+      return argsLine + wrap(' ', join(directives, ' '));
+    },
   },
 
   InlineFragment: {
diff --git a/src/language/visitor.ts b/src/language/visitor.ts
@@ -92,11 +92,10 @@ const QueryDocumentKeys = {
   Field: ['alias', 'name', 'arguments', 'directives', 'selectionSet'],
   Argument: ['name', 'value'],
 
-  FragmentSpread: ['name', 'directives'],
+  FragmentSpread: ['name', 'arguments', 'directives'],
   InlineFragment: ['typeCondition', 'directives', 'selectionSet'],
   FragmentDefinition: [
     'name',
-    // Note: fragment variable definitions are deprecated and will removed in v17.0.0
     'variableDefinitions',
     'typeCondition',
     'directives',
diff --git a/src/utilities/buildASTSchema.ts b/src/utilities/buildASTSchema.ts
@@ -99,7 +99,7 @@ export function buildSchema(
 ): GraphQLSchema {
   const document = parse(source, {
     noLocation: options?.noLocation,
-    allowLegacyFragmentVariables: options?.allowLegacyFragmentVariables,
+    allowFragmentArguments: options?.allowFragmentArguments,
   });
 
   return buildASTSchema(document, {
