From 90d048d604bbe3ee57fc7d3afa8c328972e0bc9c Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 14:20:02 -0500
Subject: [PATCH 01/13] Tests for fragment args

---
 src/execution/__tests__/variables-test.ts     | 227 ++++++++++++++++++
 src/language/__tests__/parser-test.ts         |  13 +-
 src/language/__tests__/printer-test.ts        |  40 ++-
 src/language/__tests__/visitor-test.ts        |  46 +++-
 .../NoUndefinedVariablesRule-test.ts          |  63 +++++
 .../__tests__/NoUnusedVariablesRule-test.ts   |  22 ++
 .../OverlappingFieldsCanBeMergedRule-test.ts  |  59 +++++
 .../ProvidedRequiredArgumentsRule-test.ts     | 108 +++++++++
 .../__tests__/ValuesOfCorrectTypeRule-test.ts |  31 +++
 .../VariablesAreInputTypesRule-test.ts        |  27 +++
 .../VariablesInAllowedPositionRule-test.ts    | 104 ++++++++
 11 files changed, 725 insertions(+), 15 deletions(-)

diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts
index 7c74d8ee92..8f3db65a21 100644
--- a/src/execution/__tests__/variables-test.ts
+++ b/src/execution/__tests__/variables-test.ts
@@ -82,6 +82,13 @@ function fieldWithInputArg(
   };
 }
 
+const NestedType: GraphQLObjectType = new GraphQLObjectType({
+  name: 'NestedType',
+  fields: {
+    echo: fieldWithInputArg({ type: GraphQLString }),
+  },
+});
+
 const TestType = new GraphQLObjectType({
   name: 'TestType',
   fields: {
@@ -107,6 +114,10 @@ const TestType = new GraphQLObjectType({
       defaultValue: 'Hello World',
     }),
     list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }),
+    nested: {
+      type: NestedType,
+      resolve: () => ({}),
+    },
     nnList: fieldWithInputArg({
       type: new GraphQLNonNull(new GraphQLList(GraphQLString)),
     }),
@@ -1006,6 +1017,222 @@ describe('Execute: Handles inputs', () => {
     });
   });
 
+  describe('using fragment arguments', () => {
+    it('when there are no fragment arguments', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+        fragment a on TestType {
+          fieldWithNonNullableStringInput(input: "A")
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when a value is required and provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "A")
+        }
+        fragment a($value: String!) on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when a value is required and not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+        fragment a($value: String!) on TestType {
+          fieldWithNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNullableStringInput: null,
+        },
+      });
+    });
+
+    it('when the definition has a default and is provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "A")
+        }
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when the definition has a default and is not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"B"',
+        },
+      });
+    });
+
+    it('when the definition has a non-nullable default and is provided null', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: null)
+        }
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNullableStringInput: 'null',
+        },
+      });
+    });
+
+    it('when the definition has no default and is not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+        fragment a($value: String) on TestType {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue:
+            '"Hello World"',
+        },
+      });
+    });
+
+    it('when an argument is shadowed by an operation variable', () => {
+      const result = executeQuery(`
+        query($x: String! = "A") {
+          ...a(x: "B")
+        }
+        fragment a($x: String) on TestType {
+          fieldWithNullableStringInput(input: $x)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNullableStringInput: '"B"',
+        },
+      });
+    });
+
+    it('when a nullable argument with a field default is not provided and shadowed by an operation variable', () => {
+      const result = executeQuery(`
+        query($x: String = "A") {
+          ...a
+        }
+        fragment a($x: String) on TestType {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $x)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue:
+            '"Hello World"',
+        },
+      });
+    });
+
+    it('when a fragment is used with different args', () => {
+      const result = executeQuery(`
+        query($x: String = "Hello") {
+          a: nested {
+            ...a(x: "a")
+          }
+          b: nested {
+            ...a(x: "b", b: true)
+          }
+          hello: nested {
+            ...a(x: $x)
+          }
+        }
+        fragment a($x: String, $b: Boolean = false) on NestedType {
+          a: echo(input: $x) @skip(if: $b)
+          b: echo(input: $x) @include(if: $b)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          a: {
+            a: '"a"',
+          },
+          b: {
+            b: '"b"',
+          },
+          hello: {
+            a: '"Hello"',
+          },
+        },
+      });
+    });
+
+    it('when the argument variable is nested in a complex type', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "C")
+        }
+        fragment a($value: String) on TestType {
+          list(input: ["A", "B", $value, "D"])
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          list: '["A", "B", "C", "D"]',
+        },
+      });
+    });
+
+    it('when argument variables are used recursively', () => {
+      const result = executeQuery(`
+        query {
+          ...a(aValue: "C")
+        }
+        fragment a($aValue: String) on TestType {
+          ...b(bValue: $aValue)
+        }
+        fragment b($bValue: String) on TestType {
+          list(input: ["A", "B", $bValue, "D"])
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          list: '["A", "B", "C", "D"]',
+        },
+      });
+    });
+  });
+
   describe('getVariableValues: limit maximum number of coercion errors', () => {
     const doc = parse(`
       query ($input: [String!]) {
diff --git a/src/language/__tests__/parser-test.ts b/src/language/__tests__/parser-test.ts
index 34e9dff4b9..687180846e 100644
--- a/src/language/__tests__/parser-test.ts
+++ b/src/language/__tests__/parser-test.ts
@@ -607,13 +607,16 @@ describe('Parser', () => {
     expect('loc' in result).to.equal(false);
   });
 
-  it('Legacy: allows parsing fragment defined variables', () => {
+  it('allows parsing fragment defined arguments', () => {
     const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';
 
-    expect(() =>
-      parse(document, { allowLegacyFragmentVariables: true }),
-    ).to.not.throw();
-    expect(() => parse(document)).to.throw('Syntax Error');
+    expect(() => parse(document)).to.not.throw();
+  });
+
+  it('allows parsing fragment spread arguments', () => {
+    const document = 'fragment a on t { ...b(v: $v) }';
+
+    expect(() => parse(document)).to.not.throw();
   });
 
   it('contains location that can be Object.toStringified, JSON.stringified, or jsutils.inspected', () => {
diff --git a/src/language/__tests__/printer-test.ts b/src/language/__tests__/printer-test.ts
index 0585cae6d9..94152fe079 100644
--- a/src/language/__tests__/printer-test.ts
+++ b/src/language/__tests__/printer-test.ts
@@ -110,34 +110,58 @@ describe('Printer: Query document', () => {
     `);
   });
 
-  it('Legacy: prints fragment with variable directives', () => {
-    const queryASTWithVariableDirective = parse(
+  it('prints fragment with argument definition directives', () => {
+    const fragmentWithArgumentDefinitionDirective = parse(
       'fragment Foo($foo: TestType @test) on TestType @testDirective { id }',
-      { allowLegacyFragmentVariables: true },
     );
-    expect(print(queryASTWithVariableDirective)).to.equal(dedent`
+    expect(print(fragmentWithArgumentDefinitionDirective)).to.equal(dedent`
       fragment Foo($foo: TestType @test) on TestType @testDirective {
         id
       }
     `);
   });
 
-  it('Legacy: correctly prints fragment defined variables', () => {
-    const fragmentWithVariable = parse(
+  it('correctly prints fragment defined arguments', () => {
+    const fragmentWithArgumentDefinition = parse(
       `
         fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
           id
         }
       `,
-      { allowLegacyFragmentVariables: true },
     );
-    expect(print(fragmentWithVariable)).to.equal(dedent`
+    expect(print(fragmentWithArgumentDefinition)).to.equal(dedent`
       fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
         id
       }
     `);
   });
 
+  it('prints fragment spread with arguments', () => {
+    const fragmentSpreadWithArguments = parse(
+      'fragment Foo on TestType { ...Bar(a: {x: $x}, b: true) }',
+    );
+    expect(print(fragmentSpreadWithArguments)).to.equal(dedent`
+      fragment Foo on TestType {
+        ...Bar(a: { x: $x }, b: true)
+      }
+    `);
+  });
+
+  it('prints fragment spread with multi-line arguments', () => {
+    const fragmentSpreadWithArguments = 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") }',
+    );
+    expect(print(fragmentSpreadWithArguments)).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
index 270c948225..7ee17a13a7 100644
--- a/src/language/__tests__/visitor-test.ts
+++ b/src/language/__tests__/visitor-test.ts
@@ -455,10 +455,9 @@ describe('Visitor', () => {
     ]);
   });
 
-  it('Legacy: visits variables defined in fragments', () => {
+  it('visits arguments defined on fragments', () => {
     const ast = parse('fragment a($v: Boolean = false) on t { f }', {
       noLocation: true,
-      allowLegacyFragmentVariables: true,
     });
     const visited: Array<any> = [];
 
@@ -505,6 +504,49 @@ describe('Visitor', () => {
     ]);
   });
 
+  it('visits arguments on fragment spreads', () => {
+    const ast = parse('fragment a on t { ...s(v: false) }', {
+      noLocation: 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('n', () => {
     const ast = parse(kitchenSinkQuery, {
       experimentalClientControlledNullability: true,
diff --git a/src/validation/__tests__/NoUndefinedVariablesRule-test.ts b/src/validation/__tests__/NoUndefinedVariablesRule-test.ts
index c6ed758cad..9d53ea8c2e 100644
--- a/src/validation/__tests__/NoUndefinedVariablesRule-test.ts
+++ b/src/validation/__tests__/NoUndefinedVariablesRule-test.ts
@@ -404,4 +404,67 @@ describe('Validate: No undefined variables', () => {
       },
     ]);
   });
+
+  it('fragment defined arguments are not undefined variables', () => {
+    expectValid(`
+      query Foo {
+        ...FragA
+      }
+      fragment FragA($a: String) on Type {
+        field1(a: $a)
+      }
+    `);
+  });
+
+  it('defined variables used as fragment arguments are not undefined variables', () => {
+    expectValid(`
+      query Foo($b: String) {
+        ...FragA(a: $b)
+      }
+      fragment FragA($a: String) on Type {
+        field1
+      }
+    `);
+  });
+
+  it('variables used as fragment arguments may be undefined variables', () => {
+    expectErrors(`
+      query Foo {
+        ...FragA(a: $a)
+      }
+      fragment FragA($a: String) on Type {
+        field1
+      }
+    `).toDeepEqual([
+      {
+        message: 'Variable "$a" is not defined by operation "Foo".',
+        locations: [
+          { line: 3, column: 21 },
+          { line: 2, column: 7 },
+        ],
+      },
+    ]);
+  });
+
+  it('variables shadowed by parent fragment arguments are still undefined variables', () => {
+    expectErrors(`
+      query Foo {
+        ...FragA
+      }
+      fragment FragA($a: String) on Type {
+        ...FragB
+      }
+      fragment FragB on Type {
+        field1(a: $a)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Variable "$a" is not defined by operation "Foo".',
+        locations: [
+          { line: 9, column: 19 },
+          { line: 2, column: 7 },
+        ],
+      },
+    ]);
+  });
 });
diff --git a/src/validation/__tests__/NoUnusedVariablesRule-test.ts b/src/validation/__tests__/NoUnusedVariablesRule-test.ts
index 47dac39c99..d08d547931 100644
--- a/src/validation/__tests__/NoUnusedVariablesRule-test.ts
+++ b/src/validation/__tests__/NoUnusedVariablesRule-test.ts
@@ -230,4 +230,26 @@ describe('Validate: No unused variables', () => {
       },
     ]);
   });
+
+  it('fragment defined arguments are not unused variables', () => {
+    expectValid(`
+      query Foo {
+        ...FragA
+      }
+      fragment FragA($a: String) on Type {
+        field1(a: $a)
+      }
+    `);
+  });
+
+  it('defined variables used as fragment arguments are not unused variables', () => {
+    expectValid(`
+      query Foo($b: String) {
+        ...FragA(a: $b)
+      }
+      fragment FragA($a: String) on Type {
+        field1
+      }
+    `);
+  });
 });
diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
index 52c2deb1a0..e395df52c8 100644
--- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
+++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
@@ -1246,4 +1246,63 @@ describe('Validate: Overlapping fields can be merged', () => {
       },
     ]);
   });
+
+  describe('fragment arguments must produce fields that can be merged', () => {
+    it('encounters conflict in fragments', () => {
+      expectErrors(`
+        {
+          ...WithArgs(x: 3)
+          ...WithArgs(x: 4)
+        }
+        fragment WithArgs($x: Int) on Type {
+          a(x: $x)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Spreads "WithArgs" conflict because WithArgs(x: 3) and WithArgs(x: 4) have different fragment arguments.',
+          locations: [
+            { line: 3, column: 11 },
+            { line: 4, column: 11 },
+          ],
+        },
+      ]);
+    });
+    it('encounters nested conflict in fragments', () => {
+      expectErrors(`
+        {
+          connection {
+            edges {
+              ...WithArgs(x: 3)
+            }
+          }
+          ...Connection
+        }
+
+        fragment Connection on Type {
+          connection {
+            edges {
+              ...WithArgs(x: 4)
+            }
+          }
+        }
+        fragment WithArgs($x: Int) on Type {
+          a(x: $x)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Fields "connection" conflict because subfields "edges" conflict because child spreads "WithArgs" conflict because WithArgs(x: 3) and WithArgs(x: 4) have different fragment arguments. Use different aliases on the fields to fetch both if this was intentional.',
+          locations: [
+            { line: 3, column: 11 },
+            { line: 4, column: 13 },
+            { line: 5, column: 15 },
+            { line: 12, column: 11 },
+            { line: 13, column: 13 },
+            { line: 14, column: 15 },
+          ],
+        },
+      ]);
+    });
+  });
 });
diff --git a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts
index 6f0d223c15..66545ddb59 100644
--- a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts
+++ b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts
@@ -356,4 +356,112 @@ describe('Validate: Provided required arguments', () => {
       ]);
     });
   });
+
+  describe('Fragment required arguments', () => {
+    it('ignores unknown arguments', () => {
+      expectValid(`
+        {
+          ...Foo(unknownArgument: true)
+        }
+        fragment Foo on Query {
+          dog
+        }
+      `);
+    });
+
+    // Query: should this be allowed?
+    // We could differentiate between required/optional (i.e. no default value)
+    // vs. nullable/non-nullable (i.e. no !), whereas now they are conflated.
+    // So today:
+    // $x: Int!     `x:` is required and must not be null (NOT a nullable variable)
+    // $x: Int! = 3 `x:` is not required and must not be null (MAY BE a nullable variable)
+    // $x: Int      `x:` is not required and may be null
+    // $x: Int = 3  `x:` is not required and may be null
+    //
+    // It feels weird to collapse the nullable cases but not the non-nullable ones.
+    // Whereas all four feel like they ought to mean something explicitly different.
+    //
+    // Potential proposal:
+    // $x: Int!     `x:` is required and must not be null (NOT a nullable variable)
+    // $x: Int! = 3 `x:` is not required and must not be null (NOT a nullable variable)
+    // $x: Int      `x:` is required and may be null
+    // $x: Int = 3  `x:` is not required and may be null
+    //
+    // Required then is whether there's a default value,
+    // and nullable is whether there's a !
+    it('Missing nullable argument with default is allowed', () => {
+      expectValid(`
+          {
+            ...F
+            
+          }
+          fragment F($x: Int = 3) on Query {
+            foo
+          }
+        `);
+    });
+    // Above proposal: this should be an error
+    it('Missing nullable argument is allowed', () => {
+      expectValid(`
+          {
+            ...F
+            
+          }
+          fragment F($x: Int) on Query {
+            foo
+          }
+        `);
+    });
+    it('Missing non-nullable argument with default is allowed', () => {
+      expectValid(`
+          {
+            ...F
+            
+          }
+          fragment F($x: Int! = 3) on Query {
+            foo
+          }
+        `);
+    });
+    it('Missing non-nullable argument is not allowed', () => {
+      expectErrors(`
+          {
+            ...F
+            
+          }
+          fragment F($x: Int!) on Query {
+            foo
+          }
+        `).toDeepEqual([
+        {
+          message:
+            'Fragment "F" argument "x" of type "{ kind: "NonNullType", type: { kind: "NamedType", name: [Object], loc: [Object] }, loc: [Object] }" is required, but it was not provided.',
+          locations: [
+            { line: 3, column: 13 },
+            { line: 6, column: 22 },
+          ],
+        },
+      ]);
+    });
+
+    it('Supplies required variables', () => {
+      expectValid(`
+          {
+            ...F(x: 3)
+            
+          }
+          fragment F($x: Int!) on Query {
+            foo
+          }
+        `);
+    });
+
+    it('Skips missing fragments', () => {
+      expectValid(`
+          {
+            ...Missing(x: 3)
+          }
+        `);
+    });
+  });
 });
diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts
index f0b7dfa57e..d843139d89 100644
--- a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts
+++ b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts
@@ -1198,4 +1198,35 @@ describe('Validate: Values of correct type', () => {
       ]);
     });
   });
+
+  describe('Fragment argument values', () => {
+    it('list variables with invalid item', () => {
+      expectErrors(`
+        fragment InvalidItem($a: [String] = ["one", 2]) on Query {
+          dog { name }
+        }
+      `).toDeepEqual([
+        {
+          message: 'String cannot represent a non string value: 2',
+          locations: [{ line: 2, column: 53 }],
+        },
+      ]);
+    });
+
+    it('fragment spread with invalid argument value', () => {
+      expectErrors(`
+        fragment GivesString on Query {
+          ...ExpectsInt(a: "three")
+        }
+        fragment ExpectsInt($a: Int) on Query {
+          dog { name }
+        }
+      `).toDeepEqual([
+        {
+          message: 'Int cannot represent non-integer value: "three"',
+          locations: [{ line: 3, column: 28 }],
+        },
+      ]);
+    });
+  });
 });
diff --git a/src/validation/__tests__/VariablesAreInputTypesRule-test.ts b/src/validation/__tests__/VariablesAreInputTypesRule-test.ts
index 8027a35826..eddd202df8 100644
--- a/src/validation/__tests__/VariablesAreInputTypesRule-test.ts
+++ b/src/validation/__tests__/VariablesAreInputTypesRule-test.ts
@@ -18,6 +18,9 @@ describe('Validate: Variables are input types', () => {
       query Foo($a: Unknown, $b: [[Unknown!]]!) {
         field(a: $a, b: $b)
       }
+      fragment Bar($a: Unknown, $b: [[Unknown!]]!) on Query {
+        field(a: $a, b: $b)
+      }
     `);
   });
 
@@ -26,6 +29,9 @@ describe('Validate: Variables are input types', () => {
       query Foo($a: String, $b: [Boolean!]!, $c: ComplexInput) {
         field(a: $a, b: $b, c: $c)
       }
+      fragment Bar($a: String, $b: [Boolean!]!, $c: ComplexInput) on Query {
+        field(a: $a, b: $b, c: $c)
+      }
     `);
   });
 
@@ -49,4 +55,25 @@ describe('Validate: Variables are input types', () => {
       },
     ]);
   });
+
+  it('output types on fragment arguments are invalid', () => {
+    expectErrors(`
+      fragment Bar($a: Dog, $b: [[CatOrDog!]]!, $c: Pet) on Query {
+        field(a: $a, b: $b, c: $c)
+      }
+    `).toDeepEqual([
+      {
+        locations: [{ line: 2, column: 24 }],
+        message: 'Variable "$a" cannot be non-input type "Dog".',
+      },
+      {
+        locations: [{ line: 2, column: 33 }],
+        message: 'Variable "$b" cannot be non-input type "[[CatOrDog!]]!".',
+      },
+      {
+        locations: [{ line: 2, column: 53 }],
+        message: 'Variable "$c" cannot be non-input type "Pet".',
+      },
+    ]);
+  });
 });
diff --git a/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts b/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts
index 16467741bb..d0da8f5305 100644
--- a/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts
+++ b/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts
@@ -356,5 +356,109 @@ describe('Validate: Variables are in allowed positions', () => {
           dog @include(if: $boolVar)
         }`);
     });
+
+    it('undefined in directive with default value with option', () => {
+      expectValid(`
+        {
+          dog @include(if: $x)
+        }`);
+    });
+  });
+
+  describe('Fragment arguments are validated', () => {
+    it('Boolean => Boolean', () => {
+      expectValid(`
+        query Query($booleanArg: Boolean)
+        {
+          complicatedArgs {
+            ...A(b: $booleanArg)
+          }
+        }
+        fragment A($b: Boolean) on ComplicatedArgs {
+          booleanArgField(booleanArg: $b)
+        }
+      `);
+    });
+
+    it('Boolean => Boolean with default value', () => {
+      expectValid(`
+        query Query($booleanArg: Boolean)
+        {
+          complicatedArgs {
+            ...A(b: $booleanArg)
+          }
+        }
+        fragment A($b: Boolean = true) on ComplicatedArgs {
+          booleanArgField(booleanArg: $b)
+        }
+      `);
+    });
+
+    it('Boolean => Boolean!', () => {
+      expectErrors(`
+        query Query($ab: Boolean)
+        {
+          complicatedArgs {
+            ...A(b: $ab)
+          }
+        }
+        fragment A($b: Boolean!) on ComplicatedArgs {
+          booleanArgField(booleanArg: $b)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Variable "$ab" of type "Boolean" used in position expecting type "Boolean!".',
+          locations: [
+            { line: 2, column: 21 },
+            { line: 5, column: 21 },
+          ],
+        },
+      ]);
+    });
+
+    it('Int => Int! fails when variable provides null default value', () => {
+      expectErrors(`
+        query Query($intVar: Int = null) {
+          complicatedArgs {
+            ...A(i: $intVar)
+          }
+        }
+        fragment A($i: Int!) on ComplicatedArgs {
+          nonNullIntArgField(nonNullIntArg: $i)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Variable "$intVar" of type "Int" used in position expecting type "Int!".',
+          locations: [
+            { line: 2, column: 21 },
+            { line: 4, column: 21 },
+          ],
+        },
+      ]);
+    });
+
+    it('Int fragment arg => Int! field arg fails even when shadowed by Int! variable', () => {
+      expectErrors(`
+        query Query($intVar: Int!) {
+          complicatedArgs {
+            ...A(i: $intVar)
+          }
+        }
+        fragment A($intVar: Int) on ComplicatedArgs {
+          nonNullIntArgField(nonNullIntArg: $intVar)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Variable "$intVar" of type "Int" used in position expecting type "Int!".',
+          locations: [
+            { line: 7, column: 20 },
+            { line: 8, column: 45 },
+          ],
+        },
+      ]);
+    });
   });
 });

From 94fef5d8aefa5e9ce3ff3d0a2cddf13754039d9e Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 15:09:48 -0500
Subject: [PATCH 02/13] Make FragmentArgumentDefinition a unique AST node to
 see downstream effects

---
 src/language/__tests__/visitor-test.ts |  4 +-
 src/language/ast.ts                    | 29 +++++++--
 src/language/kinds.ts                  |  1 +
 src/language/parser.ts                 | 89 ++++++++++++++++----------
 src/language/printer.ts                | 28 ++++++--
 src/utilities/buildASTSchema.ts        |  1 -
 6 files changed, 106 insertions(+), 46 deletions(-)

diff --git a/src/language/__tests__/visitor-test.ts b/src/language/__tests__/visitor-test.ts
index 7ee17a13a7..b960fe5109 100644
--- a/src/language/__tests__/visitor-test.ts
+++ b/src/language/__tests__/visitor-test.ts
@@ -477,7 +477,7 @@ describe('Visitor', () => {
       ['enter', 'FragmentDefinition', undefined],
       ['enter', 'Name', 'a'],
       ['leave', 'Name', 'a'],
-      ['enter', 'VariableDefinition', undefined],
+      ['enter', 'FragmentArgumentDefinition', undefined],
       ['enter', 'Variable', undefined],
       ['enter', 'Name', 'v'],
       ['leave', 'Name', 'v'],
@@ -488,7 +488,7 @@ describe('Visitor', () => {
       ['leave', 'NamedType', undefined],
       ['enter', 'BooleanValue', false],
       ['leave', 'BooleanValue', false],
-      ['leave', 'VariableDefinition', undefined],
+      ['leave', 'FragmentArgumentDefinition', undefined],
       ['enter', 'NamedType', undefined],
       ['enter', 'Name', 't'],
       ['leave', 'Name', 't'],
diff --git a/src/language/ast.ts b/src/language/ast.ts
index 22a7cc253c..d06d15c395 100644
--- a/src/language/ast.ts
+++ b/src/language/ast.ts
@@ -149,6 +149,7 @@ export type ASTNode =
   | FragmentSpreadNode
   | InlineFragmentNode
   | FragmentDefinitionNode
+  | FragmentArgumentDefinitionNode
   | IntValueNode
   | FloatValueNode
   | StringValueNode
@@ -227,16 +228,22 @@ export const QueryDocumentKeys: {
   NonNullAssertion: ['nullabilityAssertion'],
   ErrorBoundary: ['nullabilityAssertion'],
 
-  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',
+    'arguments',
     'typeCondition',
     'directives',
     'selectionSet',
   ],
+  FragmentArgumentDefinition: [
+    'description',
+    'variable',
+    'type',
+    'defaultValue',
+    'directives',
+  ],
 
   IntValue: [],
   FloatValue: [],
@@ -428,6 +435,7 @@ export interface FragmentSpreadNode {
   readonly kind: Kind.FRAGMENT_SPREAD;
   readonly loc?: Location | undefined;
   readonly name: NameNode;
+  readonly arguments?: ReadonlyArray<ArgumentNode> | undefined;
   readonly directives?: ReadonlyArray<DirectiveNode> | undefined;
 }
 
@@ -443,15 +451,24 @@ export interface FragmentDefinitionNode {
   readonly kind: Kind.FRAGMENT_DEFINITION;
   readonly loc?: Location | undefined;
   readonly name: NameNode;
-  /** @deprecated variableDefinitions will be removed in v17.0.0 */
-  readonly variableDefinitions?:
-    | ReadonlyArray<VariableDefinitionNode>
+  readonly arguments?:
+    | ReadonlyArray<FragmentArgumentDefinitionNode>
     | undefined;
   readonly typeCondition: NamedTypeNode;
   readonly directives?: ReadonlyArray<DirectiveNode> | undefined;
   readonly selectionSet: SelectionSetNode;
 }
 
+export interface FragmentArgumentDefinitionNode {
+  readonly kind: Kind.FRAGMENT_ARGUMENT_DEFINITION;
+  readonly loc?: Location | undefined;
+  readonly description?: StringValueNode | undefined;
+  readonly variable: VariableNode;
+  readonly type: TypeNode;
+  readonly defaultValue?: ConstValueNode | undefined;
+  readonly directives?: ReadonlyArray<ConstDirectiveNode> | undefined;
+}
+
 /** Values */
 
 export type ValueNode =
diff --git a/src/language/kinds.ts b/src/language/kinds.ts
index d606c12cbe..8d9098ed4e 100644
--- a/src/language/kinds.ts
+++ b/src/language/kinds.ts
@@ -22,6 +22,7 @@ export enum Kind {
   FRAGMENT_SPREAD = 'FragmentSpread',
   INLINE_FRAGMENT = 'InlineFragment',
   FRAGMENT_DEFINITION = 'FragmentDefinition',
+  FRAGMENT_ARGUMENT_DEFINITION = 'FragmentArgumentDefinition',
 
   /** Values */
   VARIABLE = 'Variable',
diff --git a/src/language/parser.ts b/src/language/parser.ts
index bc58875e9d..6cad241033 100644
--- a/src/language/parser.ts
+++ b/src/language/parser.ts
@@ -24,6 +24,7 @@ import type {
   FieldDefinitionNode,
   FieldNode,
   FloatValueNode,
+  FragmentArgumentDefinitionNode,
   FragmentDefinitionNode,
   FragmentSpreadNode,
   InlineFragmentNode,
@@ -91,23 +92,6 @@ export interface ParseOptions {
    */
   maxTokens?: number | undefined;
 
-  /**
-   * @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.
-   *
-   * The syntax is identical to normal, query-defined variables. For example:
-   *
-   * ```graphql
-   * fragment A($var: Boolean = false) on T {
-   *   ...
-   * }
-   * ```
-   */
-  allowLegacyFragmentVariables?: boolean | undefined;
-
   /**
    * EXPERIMENTAL:
    *
@@ -550,7 +534,7 @@ export class Parser {
   /**
    * Corresponds to both FragmentSpread and InlineFragment in the spec.
    *
-   * FragmentSpread : ... FragmentName Directives?
+   * FragmentSpread : ... FragmentName Arguments? Directives?
    *
    * InlineFragment : ... TypeCondition? Directives? SelectionSet
    */
@@ -560,9 +544,18 @@ export class Parser {
 
     const hasTypeCondition = this.expectOptionalKeyword('on');
     if (!hasTypeCondition && this.peek(TokenKind.NAME)) {
+      const name = this.parseFragmentName();
+      if (this.peek(TokenKind.PAREN_L)) {
+        return this.node<FragmentSpreadNode>(start, {
+          kind: Kind.FRAGMENT_SPREAD,
+          name,
+          arguments: this.parseArguments(false),
+          directives: this.parseDirectives(false),
+        });
+      }
       return this.node<FragmentSpreadNode>(start, {
         kind: Kind.FRAGMENT_SPREAD,
-        name: this.parseFragmentName(),
+        name,
         directives: this.parseDirectives(false),
       });
     }
@@ -576,29 +569,17 @@ export class Parser {
 
   /**
    * FragmentDefinition :
-   *   - fragment FragmentName on TypeCondition Directives? SelectionSet
+   *   - fragment FragmentName FragmentArgumentsDefinition? on TypeCondition Directives? SelectionSet
    *
    * TypeCondition : NamedType
    */
   parseFragmentDefinition(): FragmentDefinitionNode {
     const start = this._lexer.token;
     this.expectKeyword('fragment');
-    // Legacy support for defining variables within fragments changes
-    // the grammar of FragmentDefinition:
-    //   - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet
-    if (this._options.allowLegacyFragmentVariables === true) {
-      return this.node<FragmentDefinitionNode>(start, {
-        kind: Kind.FRAGMENT_DEFINITION,
-        name: this.parseFragmentName(),
-        variableDefinitions: this.parseVariableDefinitions(),
-        typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
-        directives: this.parseDirectives(false),
-        selectionSet: this.parseSelectionSet(),
-      });
-    }
     return this.node<FragmentDefinitionNode>(start, {
       kind: Kind.FRAGMENT_DEFINITION,
       name: this.parseFragmentName(),
+      arguments: this.parseFragmentArgumentDefs(),
       typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
       directives: this.parseDirectives(false),
       selectionSet: this.parseSelectionSet(),
@@ -615,6 +596,48 @@ export class Parser {
     return this.parseName();
   }
 
+  /**
+   * FragmentArgumentsDefinition : ( FragmentArgumentDefinition+ )
+   */
+  parseFragmentArgumentDefs(): Array<FragmentArgumentDefinitionNode> {
+    return this.optionalMany(
+      TokenKind.PAREN_L,
+      this.parseFragmentArgumentDef,
+      TokenKind.PAREN_R,
+    );
+  }
+
+  /**
+   * FragmentArgumentDefinition :
+   *   - Description? Variable : Type DefaultValue? Directives[Const]?
+   *
+   * Note: identical to InputValueDefinition, EXCEPT Name always begins
+   * with $, so we need to parse a Variable out instead of a plain Name.
+   *
+   * Note: identical to VariableDefinition, EXCEPT we allow Description.
+   * Fragments are re-used, and their arguments may need documentation.
+   */
+  parseFragmentArgumentDef(): FragmentArgumentDefinitionNode {
+    const start = this._lexer.token;
+    const description = this.parseDescription();
+    const variable = this.parseVariable();
+    this.expectToken(TokenKind.COLON);
+    const type = this.parseTypeReference();
+    let defaultValue;
+    if (this.expectOptionalToken(TokenKind.EQUALS)) {
+      defaultValue = this.parseConstValueLiteral();
+    }
+    const directives = this.parseConstDirectives();
+    return this.node<FragmentArgumentDefinitionNode>(start, {
+      kind: Kind.FRAGMENT_ARGUMENT_DEFINITION,
+      description,
+      variable,
+      type,
+      defaultValue,
+      directives,
+    });
+  }
+
   // Implements the parsing rules in the Values section.
 
   /**
diff --git a/src/language/printer.ts b/src/language/printer.ts
index a07decc11d..7de5916322 100644
--- a/src/language/printer.ts
+++ b/src/language/printer.ts
@@ -105,8 +105,15 @@ 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: {
@@ -126,17 +133,30 @@ const printDocASTReducer: ASTReducer<string> = {
     leave: ({
       name,
       typeCondition,
-      variableDefinitions,
+      arguments: args,
       directives,
       selectionSet,
     }) =>
       // Note: fragment variable definitions are experimental and may be changed
       // or removed in the future.
-      `fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` +
+      `fragment ${name}${wrap('(', join(args, ', '), ')')} ` +
       `on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` +
       selectionSet,
   },
 
+  FragmentArgumentDefinition: {
+    leave: ({ description, variable, type, defaultValue, directives }) =>
+      wrap('', description, '\n') +
+      join(
+        [
+          variable + ': ' + type,
+          wrap('= ', defaultValue),
+          join(directives, ' '),
+        ],
+        ' ',
+      ),
+  },
+
   // Value
 
   IntValue: { leave: ({ value }) => value },
diff --git a/src/utilities/buildASTSchema.ts b/src/utilities/buildASTSchema.ts
index 09fd7cd7dd..c9728f56c3 100644
--- a/src/utilities/buildASTSchema.ts
+++ b/src/utilities/buildASTSchema.ts
@@ -93,7 +93,6 @@ export function buildSchema(
 ): GraphQLSchema {
   const document = parse(source, {
     noLocation: options?.noLocation,
-    allowLegacyFragmentVariables: options?.allowLegacyFragmentVariables,
   });
 
   return buildASTSchema(document, {

From 914fd837ec3f92ea2da8f4f0b067afea6a1be17c Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 15:18:30 -0500
Subject: [PATCH 03/13] Execution works with new AST node

---
 src/execution/collectFields.ts               | 28 ++++---
 src/utilities/keyForFragmentSpread.ts        | 23 ++++++
 src/utilities/substituteFragmentArguments.ts | 78 ++++++++++++++++++++
 3 files changed, 118 insertions(+), 11 deletions(-)
 create mode 100644 src/utilities/keyForFragmentSpread.ts
 create mode 100644 src/utilities/substituteFragmentArguments.ts

diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts
index 17468b791f..a5634023ad 100644
--- a/src/execution/collectFields.ts
+++ b/src/execution/collectFields.ts
@@ -22,6 +22,8 @@ import {
 } from '../type/directives.js';
 import type { GraphQLSchema } from '../type/schema.js';
 
+import { keyForFragmentSpread } from '../utilities/keyForFragmentSpread.js';
+import { substituteFragmentArguments } from '../utilities/substituteFragmentArguments.js';
 import { typeFromAST } from '../utilities/typeFromAST.js';
 
 import { getDirectiveValues } from './values.js';
@@ -124,7 +126,7 @@ function collectFieldsImpl(
   selectionSet: SelectionSetNode,
   fields: AccumulatorMap<string, FieldNode>,
   patches: Array<PatchFields>,
-  visitedFragmentNames: Set<string>,
+  visitedFragmentKeys: Set<string>,
 ): void {
   for (const selection of selectionSet.selections) {
     switch (selection.kind) {
@@ -156,7 +158,7 @@ function collectFieldsImpl(
             selection.selectionSet,
             patchFields,
             patches,
-            visitedFragmentNames,
+            visitedFragmentKeys,
           );
           patches.push({
             label: defer.label,
@@ -172,24 +174,24 @@ function collectFieldsImpl(
             selection.selectionSet,
             fields,
             patches,
-            visitedFragmentNames,
+            visitedFragmentKeys,
           );
         }
         break;
       }
       case Kind.FRAGMENT_SPREAD: {
-        const fragName = selection.name.value;
+        const fragmentKey = keyForFragmentSpread(selection);
 
         if (!shouldIncludeNode(variableValues, selection)) {
           continue;
         }
 
         const defer = getDeferValues(operation, variableValues, selection);
-        if (visitedFragmentNames.has(fragName) && !defer) {
+        if (visitedFragmentKeys.has(fragmentKey) && !defer) {
           continue;
         }
 
-        const fragment = fragments[fragName];
+        const fragment = fragments[selection.name.value];
         if (
           !fragment ||
           !doesFragmentConditionMatch(schema, fragment, runtimeType)
@@ -198,9 +200,13 @@ function collectFieldsImpl(
         }
 
         if (!defer) {
-          visitedFragmentNames.add(fragName);
+          visitedFragmentKeys.add(fragmentKey);
         }
 
+        const fragmentSelectionSet = substituteFragmentArguments(
+          fragment,
+          selection,
+        );
         if (defer) {
           const patchFields = new AccumulatorMap<string, FieldNode>();
           collectFieldsImpl(
@@ -209,10 +215,10 @@ function collectFieldsImpl(
             variableValues,
             operation,
             runtimeType,
-            fragment.selectionSet,
+            fragmentSelectionSet,
             patchFields,
             patches,
-            visitedFragmentNames,
+            visitedFragmentKeys,
           );
           patches.push({
             label: defer.label,
@@ -225,10 +231,10 @@ function collectFieldsImpl(
             variableValues,
             operation,
             runtimeType,
-            fragment.selectionSet,
+            fragmentSelectionSet,
             fields,
             patches,
-            visitedFragmentNames,
+            visitedFragmentKeys,
           );
         }
         break;
diff --git a/src/utilities/keyForFragmentSpread.ts b/src/utilities/keyForFragmentSpread.ts
new file mode 100644
index 0000000000..95b2d9e6cc
--- /dev/null
+++ b/src/utilities/keyForFragmentSpread.ts
@@ -0,0 +1,23 @@
+import type { FragmentSpreadNode } from '../language/ast.js';
+import { print } from '../language/printer.js';
+
+/**
+ * Create a key that uniquely identifies common fragment spreads.
+ * Treats the fragment spread as the source of truth for the key: it
+ * does not bother to look up the argument definitions to de-duplicate default-variable args.
+ *
+ * Using the fragment definition to more accurately de-duplicate common spreads
+ * is a potential performance win, but in practice it seems unlikely to be common.
+ */
+export function keyForFragmentSpread(fragmentSpread: FragmentSpreadNode) {
+  const fragmentName = fragmentSpread.name.value;
+  const fragmentArguments = fragmentSpread.arguments;
+  if (fragmentArguments == null || fragmentArguments.length === 0) {
+    return fragmentName;
+  }
+
+  const printedArguments: Array<string> = fragmentArguments
+    .map(print)
+    .sort((a, b) => a.localeCompare(b));
+  return fragmentName + '(' + printedArguments.join(',') + ')';
+}
diff --git a/src/utilities/substituteFragmentArguments.ts b/src/utilities/substituteFragmentArguments.ts
new file mode 100644
index 0000000000..504ae6e8d3
--- /dev/null
+++ b/src/utilities/substituteFragmentArguments.ts
@@ -0,0 +1,78 @@
+import type { Maybe } from '../jsutils/Maybe.js';
+import type { ObjMap } from '../jsutils/ObjMap.js';
+
+import type {
+  ArgumentNode,
+  FragmentArgumentDefinitionNode,
+  FragmentDefinitionNode,
+  FragmentSpreadNode,
+  SelectionSetNode,
+  ValueNode,
+} from '../language/ast.js';
+import { Kind } from '../language/kinds.js';
+import { visit } from '../language/visitor.js';
+
+/**
+ * Replaces all fragment argument values with non-fragment-scoped values.
+ *
+ * NOTE: fragment arguments are scoped to the fragment they're defined on.
+ * Therefore, after we apply the passed-in arguments, all remaining variables
+ * must be either operation defined variables or explicitly unset.
+ */
+export function substituteFragmentArguments(
+  def: FragmentDefinitionNode,
+  fragmentSpread: FragmentSpreadNode,
+): SelectionSetNode {
+  const argumentDefinitions = def.arguments;
+  if (argumentDefinitions == null || argumentDefinitions.length === 0) {
+    return def.selectionSet;
+  }
+  const argumentValues = fragmentArgumentSubstitutions(
+    argumentDefinitions,
+    fragmentSpread.arguments,
+  );
+  return visit(def.selectionSet, {
+    Variable(node) {
+      return argumentValues[node.name.value];
+    },
+  });
+}
+
+export function fragmentArgumentSubstitutions(
+  argumentDefinitions: ReadonlyArray<FragmentArgumentDefinitionNode>,
+  argumentValues: Maybe<ReadonlyArray<ArgumentNode>>,
+): ObjMap<ValueNode> {
+  const substitutions: ObjMap<ValueNode> = {};
+  if (argumentValues) {
+    for (const argument of argumentValues) {
+      substitutions[argument.name.value] = argument.value;
+    }
+  }
+
+  for (const argumentDefinition of argumentDefinitions) {
+    const argumentName = argumentDefinition.variable.name.value;
+    if (substitutions[argumentName]) {
+      continue;
+    }
+
+    const defaultValue = argumentDefinition.defaultValue;
+    if (defaultValue) {
+      substitutions[argumentName] = defaultValue;
+    } else {
+      // We need a way to allow unset arguments without accidentally
+      // replacing an unset fragment argument with an operation
+      // variable value. Fragment arguments must always have LOCAL scope.
+      //
+      // To remove this hack, we need to either:
+      //    - include fragment argument scope when evaluating fields
+      //    - make unset fragment arguments invalid
+      // Requiring the spread to pass all non-default-defined arguments is nice,
+      // but makes field argument default values impossible to use.
+      substitutions[argumentName] = {
+        kind: Kind.VARIABLE,
+        name: { kind: Kind.NAME, value: '__UNSET' },
+      };
+    }
+  }
+  return substitutions;
+}

From 938a983ecc9c8974c7b0aa59f1e5aaa767296fa4 Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 15:40:36 -0500
Subject: [PATCH 04/13] Add Fragment Arg info to TypeInfo

---
 src/utilities/TypeInfo.ts | 92 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 85 insertions(+), 7 deletions(-)

diff --git a/src/utilities/TypeInfo.ts b/src/utilities/TypeInfo.ts
index fe8ea1fbab..06eef1f3cf 100644
--- a/src/utilities/TypeInfo.ts
+++ b/src/utilities/TypeInfo.ts
@@ -1,6 +1,12 @@
 import type { Maybe } from '../jsutils/Maybe.js';
+import type { ObjMap } from '../jsutils/ObjMap.js';
 
-import type { ASTNode, FieldNode } from '../language/ast.js';
+import type {
+  ASTNode,
+  FieldNode,
+  FragmentDefinitionNode,
+  FragmentSpreadNode,
+} from '../language/ast.js';
 import { isNode } from '../language/ast.js';
 import { Kind } from '../language/kinds.js';
 import type { ASTVisitor } from '../language/visitor.js';
@@ -31,6 +37,7 @@ import type { GraphQLDirective } from '../type/directives.js';
 import type { GraphQLSchema } from '../type/schema.js';
 
 import { typeFromAST } from './typeFromAST.js';
+import { valueFromAST } from './valueFromAST.js';
 
 /**
  * TypeInfo is a utility class which, given a GraphQL schema, can keep track
@@ -39,6 +46,7 @@ import { typeFromAST } from './typeFromAST.js';
  */
 export class TypeInfo {
   private _schema: GraphQLSchema;
+
   private _typeStack: Array<Maybe<GraphQLOutputType>>;
   private _parentTypeStack: Array<Maybe<GraphQLCompositeType>>;
   private _inputTypeStack: Array<Maybe<GraphQLInputType>>;
@@ -47,6 +55,8 @@ export class TypeInfo {
   private _directive: Maybe<GraphQLDirective>;
   private _argument: Maybe<GraphQLArgument>;
   private _enumValue: Maybe<GraphQLEnumValue>;
+  private _fragmentSpread: Maybe<FragmentSpreadNode>;
+  private _fragmentDefinitions: ObjMap<FragmentDefinitionNode>;
   private _getFieldDef: GetFieldDefFn;
 
   constructor(
@@ -69,6 +79,8 @@ export class TypeInfo {
     this._directive = null;
     this._argument = null;
     this._enumValue = null;
+    this._fragmentSpread = null;
+    this._fragmentDefinitions = Object.create(null);
     this._getFieldDef = getFieldDefFn ?? getFieldDef;
     if (initialType) {
       if (isInputType(initialType)) {
@@ -142,6 +154,17 @@ export class TypeInfo {
     // checked before continuing since TypeInfo is used as part of validation
     // which occurs before guarantees of schema and document validity.
     switch (node.kind) {
+      case Kind.DOCUMENT: {
+        // A document's fragment definitions are type signatures
+        // referenced via fragment spreads. Ensure we can use definitions
+        // before visiting their call sites.
+        for (const astNode of node.definitions) {
+          if (astNode.kind === Kind.FRAGMENT_DEFINITION) {
+            this._fragmentDefinitions[astNode.name.value] = astNode;
+          }
+        }
+        break;
+      }
       case Kind.SELECTION_SET: {
         const namedType: unknown = getNamedType(this.getType());
         this._parentTypeStack.push(
@@ -180,6 +203,10 @@ export class TypeInfo {
         this._typeStack.push(isOutputType(outputType) ? outputType : undefined);
         break;
       }
+      case Kind.FRAGMENT_SPREAD: {
+        this._fragmentSpread = node;
+        break;
+      }
       case Kind.VARIABLE_DEFINITION: {
         const inputType: unknown = typeFromAST(schema, node.type);
         this._inputTypeStack.push(
@@ -187,18 +214,60 @@ export class TypeInfo {
         );
         break;
       }
+      case Kind.FRAGMENT_ARGUMENT_DEFINITION: {
+        const inputType: unknown = typeFromAST(schema, node.type);
+        this._inputTypeStack.push(
+          isInputType(inputType) ? inputType : undefined,
+        );
+        break;
+      }
       case Kind.ARGUMENT: {
         let argDef;
         let argType: unknown;
-        const fieldOrDirective = this.getDirective() ?? this.getFieldDef();
-        if (fieldOrDirective) {
-          argDef = fieldOrDirective.args.find(
-            (arg) => arg.name === node.name.value,
+        const directive = this.getDirective();
+        const fragmentSpread = this._fragmentSpread;
+        const fieldDef = this.getFieldDef();
+        if (directive) {
+          argDef = directive.args.find((arg) => arg.name === node.name.value);
+        } else if (fragmentSpread) {
+          const fragmentDef =
+            this._fragmentDefinitions[fragmentSpread.name.value];
+          const fragArgDef = fragmentDef?.arguments?.find(
+            (arg) => arg.variable.name.value === node.name.value,
           );
-          if (argDef) {
-            argType = argDef.type;
+          if (fragArgDef) {
+            const fragArgType = typeFromAST(schema, fragArgDef.type);
+            if (isInputType(fragArgType)) {
+              const fragArgDefault = fragArgDef.defaultValue
+                ? valueFromAST(fragArgDef.defaultValue, fragArgType)
+                : undefined;
+
+              // Minor hack: transform the FragmentArgDef
+              // into a schema Argument definition, to
+              // enable visiting identically to field/directive args
+              const schemaArgDef: GraphQLArgument = {
+                name: fragArgDef.variable.name.value,
+                type: fragArgType,
+                defaultValue: fragArgDefault,
+                description: fragArgDef.description?.value,
+                deprecationReason: undefined,
+                extensions: {},
+                astNode: {
+                  ...fragArgDef,
+                  kind: Kind.INPUT_VALUE_DEFINITION,
+                  name: fragArgDef.variable.name,
+                },
+              };
+              argDef = schemaArgDef;
+            }
           }
+        } else if (fieldDef) {
+          argDef = fieldDef.args.find((arg) => arg.name === node.name.value);
+        }
+        if (argDef) {
+          argType = argDef.type;
         }
+
         this._argument = argDef;
         this._defaultValueStack.push(argDef ? argDef.defaultValue : undefined);
         this._inputTypeStack.push(isInputType(argType) ? argType : undefined);
@@ -248,6 +317,9 @@ export class TypeInfo {
 
   leave(node: ASTNode) {
     switch (node.kind) {
+      case Kind.DOCUMENT:
+        this._fragmentDefinitions = Object.create(null);
+        break;
       case Kind.SELECTION_SET:
         this._parentTypeStack.pop();
         break;
@@ -263,6 +335,12 @@ export class TypeInfo {
       case Kind.FRAGMENT_DEFINITION:
         this._typeStack.pop();
         break;
+      case Kind.FRAGMENT_ARGUMENT_DEFINITION:
+        this._inputTypeStack.pop();
+        break;
+      case Kind.FRAGMENT_SPREAD:
+        this._fragmentSpread = null;
+        break;
       case Kind.VARIABLE_DEFINITION:
         this._inputTypeStack.pop();
         break;

From 76fb6a6968c94ddcf4eecd2e021ec27067770c82 Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 16:30:06 -0500
Subject: [PATCH 05/13] Update ValidationContext for Fragment Args. Fixes
 typeInfo doc resetting

---
 src/validation/ValidationContext.ts | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/validation/ValidationContext.ts b/src/validation/ValidationContext.ts
index b0c5524fa7..65c2e27537 100644
--- a/src/validation/ValidationContext.ts
+++ b/src/validation/ValidationContext.ts
@@ -5,6 +5,7 @@ import type { GraphQLError } from '../error/GraphQLError.js';
 
 import type {
   DocumentNode,
+  FragmentArgumentDefinitionNode,
   FragmentDefinitionNode,
   FragmentSpreadNode,
   OperationDefinitionNode,
@@ -26,13 +27,15 @@ import type {
 import type { GraphQLDirective } from '../type/directives.js';
 import type { GraphQLSchema } from '../type/schema.js';
 
-import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo.js';
+import type { TypeInfo } from '../utilities/TypeInfo.js';
+import { visitWithTypeInfo } from '../utilities/TypeInfo.js';
 
 type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode;
 interface VariableUsage {
   readonly node: VariableNode;
   readonly type: Maybe<GraphQLInputType>;
   readonly defaultValue: Maybe<unknown>;
+  readonly fragmentArgDef: Maybe<FragmentArgumentDefinitionNode>;
 }
 
 /**
@@ -199,16 +202,23 @@ export class ValidationContext extends ASTValidationContext {
     let usages = this._variableUsages.get(node);
     if (!usages) {
       const newUsages: Array<VariableUsage> = [];
-      const typeInfo = new TypeInfo(this._schema);
+      const typeInfo = this._typeInfo;
+      const localArgumentDefinitions =
+        node.kind === Kind.FRAGMENT_DEFINITION ? node.arguments : undefined;
       visit(
         node,
         visitWithTypeInfo(typeInfo, {
           VariableDefinition: () => false,
+          FragmentArgumentDefinition: () => false,
           Variable(variable) {
+            const fragmentArgDef = localArgumentDefinitions?.find(
+              (argDef) => argDef.variable.name.value === variable.name.value,
+            );
             newUsages.push({
               node: variable,
               type: typeInfo.getInputType(),
               defaultValue: typeInfo.getDefaultValue(),
+              fragmentArgDef,
             });
           },
         }),

From 686fe565c00eb444179607f629a011d5c1a3c19c Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 16:31:24 -0500
Subject: [PATCH 06/13] Update all validation rules: tests pass

---
 src/language/directiveLocation.ts             |   1 +
 src/utilities/__tests__/TypeInfo-test.ts      | 107 ++++
 .../__tests__/KnownDirectivesRule-test.ts     |  14 +-
 .../NoUnusedFragmentArgumentsRule-test.ts     | 120 +++++
 src/validation/index.ts                       |   3 +
 src/validation/rules/KnownDirectivesRule.ts   |   2 +
 .../rules/NoUndefinedVariablesRule.ts         |   5 +-
 .../rules/NoUnusedFragmentArgumentsRule.ts    |  40 ++
 src/validation/rules/NoUnusedVariablesRule.ts |   5 +-
 .../rules/OverlappingFieldsCanBeMergedRule.ts | 487 +++++++++++-------
 .../rules/ProvidedRequiredArgumentsRule.ts    |  40 +-
 .../rules/VariablesAreInputTypesRule.ts       |  37 +-
 .../rules/VariablesInAllowedPositionRule.ts   |   9 +-
 src/validation/specifiedRules.ts              |   3 +
 14 files changed, 666 insertions(+), 207 deletions(-)
 create mode 100644 src/validation/__tests__/NoUnusedFragmentArgumentsRule-test.ts
 create mode 100644 src/validation/rules/NoUnusedFragmentArgumentsRule.ts

diff --git a/src/language/directiveLocation.ts b/src/language/directiveLocation.ts
index b10214d47f..3ff959e0c1 100644
--- a/src/language/directiveLocation.ts
+++ b/src/language/directiveLocation.ts
@@ -11,6 +11,7 @@ export enum DirectiveLocation {
   FRAGMENT_SPREAD = 'FRAGMENT_SPREAD',
   INLINE_FRAGMENT = 'INLINE_FRAGMENT',
   VARIABLE_DEFINITION = 'VARIABLE_DEFINITION',
+  FRAGMENT_ARGUMENT_DEFINITION = 'FRAGMENT_ARGUMENT_DEFINITION',
   /** Type System Definitions */
   SCHEMA = 'SCHEMA',
   SCALAR = 'SCALAR',
diff --git a/src/utilities/__tests__/TypeInfo-test.ts b/src/utilities/__tests__/TypeInfo-test.ts
index 971316f8b4..8c3f9e2986 100644
--- a/src/utilities/__tests__/TypeInfo-test.ts
+++ b/src/utilities/__tests__/TypeInfo-test.ts
@@ -515,4 +515,111 @@ describe('visitWithTypeInfo', () => {
       ['leave', 'SelectionSet', null, 'Human', 'Human'],
     ]);
   });
+
+  it('supports traversals of fragment arguments', () => {
+    const typeInfo = new TypeInfo(testSchema);
+
+    const ast = parse(`
+      query {
+        ...Foo(x: 4)
+      }
+      
+      fragment Foo(
+        "Human to get"
+        $x: ID!
+      ) on QueryRoot {
+        human(id: $x) { name }
+      }
+    `);
+
+    const visited: Array<any> = [];
+    visit(
+      ast,
+      visitWithTypeInfo(typeInfo, {
+        enter(node) {
+          const type = typeInfo.getType();
+          const inputType = typeInfo.getInputType();
+          visited.push([
+            'enter',
+            node.kind,
+            node.kind === 'Name' ? node.value : null,
+            String(type),
+            String(inputType),
+          ]);
+        },
+        leave(node) {
+          const type = typeInfo.getType();
+          const inputType = typeInfo.getInputType();
+          visited.push([
+            'leave',
+            node.kind,
+            node.kind === 'Name' ? node.value : null,
+            String(type),
+            String(inputType),
+          ]);
+        },
+      }),
+    );
+
+    expect(visited).to.deep.equal([
+      ['enter', 'Document', null, 'undefined', 'undefined'],
+      ['enter', 'OperationDefinition', null, 'QueryRoot', 'undefined'],
+      ['enter', 'SelectionSet', null, 'QueryRoot', 'undefined'],
+      ['enter', 'FragmentSpread', null, 'QueryRoot', 'undefined'],
+      ['enter', 'Name', 'Foo', 'QueryRoot', 'undefined'],
+      ['leave', 'Name', 'Foo', 'QueryRoot', 'undefined'],
+      ['enter', 'Argument', null, 'QueryRoot', 'ID!'],
+      ['enter', 'Name', 'x', 'QueryRoot', 'ID!'],
+      ['leave', 'Name', 'x', 'QueryRoot', 'ID!'],
+      ['enter', 'IntValue', null, 'QueryRoot', 'ID!'],
+      ['leave', 'IntValue', null, 'QueryRoot', 'ID!'],
+      ['leave', 'Argument', null, 'QueryRoot', 'ID!'],
+      ['leave', 'FragmentSpread', null, 'QueryRoot', 'undefined'],
+      ['leave', 'SelectionSet', null, 'QueryRoot', 'undefined'],
+      ['leave', 'OperationDefinition', null, 'QueryRoot', 'undefined'],
+      ['enter', 'FragmentDefinition', null, 'QueryRoot', 'undefined'],
+      ['enter', 'Name', 'Foo', 'QueryRoot', 'undefined'],
+      ['leave', 'Name', 'Foo', 'QueryRoot', 'undefined'],
+      ['enter', 'FragmentArgumentDefinition', null, 'QueryRoot', 'ID!'],
+      ['enter', 'StringValue', null, 'QueryRoot', 'ID!'],
+      ['leave', 'StringValue', null, 'QueryRoot', 'ID!'],
+      ['enter', 'Variable', null, 'QueryRoot', 'ID!'],
+      ['enter', 'Name', 'x', 'QueryRoot', 'ID!'],
+      ['leave', 'Name', 'x', 'QueryRoot', 'ID!'],
+      ['leave', 'Variable', null, 'QueryRoot', 'ID!'],
+      ['enter', 'NonNullType', null, 'QueryRoot', 'ID!'],
+      ['enter', 'NamedType', null, 'QueryRoot', 'ID!'],
+      ['enter', 'Name', 'ID', 'QueryRoot', 'ID!'],
+      ['leave', 'Name', 'ID', 'QueryRoot', 'ID!'],
+      ['leave', 'NamedType', null, 'QueryRoot', 'ID!'],
+      ['leave', 'NonNullType', null, 'QueryRoot', 'ID!'],
+      ['leave', 'FragmentArgumentDefinition', null, 'QueryRoot', 'ID!'],
+      ['enter', 'NamedType', null, 'QueryRoot', 'undefined'],
+      ['enter', 'Name', 'QueryRoot', 'QueryRoot', 'undefined'],
+      ['leave', 'Name', 'QueryRoot', 'QueryRoot', 'undefined'],
+      ['leave', 'NamedType', null, 'QueryRoot', 'undefined'],
+      ['enter', 'SelectionSet', null, 'QueryRoot', 'undefined'],
+      ['enter', 'Field', null, 'Human', 'undefined'],
+      ['enter', 'Name', 'human', 'Human', 'undefined'],
+      ['leave', 'Name', 'human', 'Human', 'undefined'],
+      ['enter', 'Argument', null, 'Human', 'ID'],
+      ['enter', 'Name', 'id', 'Human', 'ID'],
+      ['leave', 'Name', 'id', 'Human', 'ID'],
+      ['enter', 'Variable', null, 'Human', 'ID'],
+      ['enter', 'Name', 'x', 'Human', 'ID'],
+      ['leave', 'Name', 'x', 'Human', 'ID'],
+      ['leave', 'Variable', null, 'Human', 'ID'],
+      ['leave', 'Argument', null, 'Human', 'ID'],
+      ['enter', 'SelectionSet', null, 'Human', 'undefined'],
+      ['enter', 'Field', null, 'String', 'undefined'],
+      ['enter', 'Name', 'name', 'String', 'undefined'],
+      ['leave', 'Name', 'name', 'String', 'undefined'],
+      ['leave', 'Field', null, 'String', 'undefined'],
+      ['leave', 'SelectionSet', null, 'Human', 'undefined'],
+      ['leave', 'Field', null, 'Human', 'undefined'],
+      ['leave', 'SelectionSet', null, 'QueryRoot', 'undefined'],
+      ['leave', 'FragmentDefinition', null, 'QueryRoot', 'undefined'],
+      ['leave', 'Document', null, 'undefined', 'undefined'],
+    ]);
+  });
 });
diff --git a/src/validation/__tests__/KnownDirectivesRule-test.ts b/src/validation/__tests__/KnownDirectivesRule-test.ts
index a3bbc198da..f172ce5ca1 100644
--- a/src/validation/__tests__/KnownDirectivesRule-test.ts
+++ b/src/validation/__tests__/KnownDirectivesRule-test.ts
@@ -41,6 +41,7 @@ const schemaWithDirectives = buildSchema(`
   directive @onSubscription on SUBSCRIPTION
   directive @onField on FIELD
   directive @onFragmentDefinition on FRAGMENT_DEFINITION
+  directive @onFragmentArgumentDefinition on FRAGMENT_ARGUMENT_DEFINITION
   directive @onFragmentSpread on FRAGMENT_SPREAD
   directive @onInlineFragment on INLINE_FRAGMENT
   directive @onVariableDefinition on VARIABLE_DEFINITION
@@ -150,7 +151,9 @@ describe('Validate: Known directives', () => {
         someField @onField
       }
 
-      fragment Frag on Human @onFragmentDefinition {
+      fragment Frag(
+        $arg: Int @onFragmentArgumentDefinition
+      ) on Human @onFragmentDefinition {
         name @onField
       }
     `);
@@ -175,7 +178,7 @@ describe('Validate: Known directives', () => {
         someField @onQuery
       }
 
-      fragment Frag on Human @onQuery {
+      fragment Frag($arg: Int @onField) on Human @onQuery {
         name @onQuery
       }
     `).toDeepEqual([
@@ -219,9 +222,14 @@ describe('Validate: Known directives', () => {
         message: 'Directive "@onQuery" may not be used on FIELD.',
         locations: [{ column: 19, line: 16 }],
       },
+      {
+        message:
+          'Directive "@onField" may not be used on FRAGMENT_ARGUMENT_DEFINITION.',
+        locations: [{ column: 31, line: 19 }],
+      },
       {
         message: 'Directive "@onQuery" may not be used on FRAGMENT_DEFINITION.',
-        locations: [{ column: 30, line: 19 }],
+        locations: [{ column: 50, line: 19 }],
       },
       {
         message: 'Directive "@onQuery" may not be used on FIELD.',
diff --git a/src/validation/__tests__/NoUnusedFragmentArgumentsRule-test.ts b/src/validation/__tests__/NoUnusedFragmentArgumentsRule-test.ts
new file mode 100644
index 0000000000..9ca54af80c
--- /dev/null
+++ b/src/validation/__tests__/NoUnusedFragmentArgumentsRule-test.ts
@@ -0,0 +1,120 @@
+import { describe, it } from 'mocha';
+
+import { NoUnusedFragmentArgumentsRule } from '../rules/NoUnusedFragmentArgumentsRule.js';
+
+import { expectValidationErrors } from './harness.js';
+
+function expectErrors(queryStr: string) {
+  return expectValidationErrors(NoUnusedFragmentArgumentsRule, queryStr);
+}
+
+function expectValid(queryStr: string) {
+  expectErrors(queryStr).toDeepEqual([]);
+}
+
+describe('Validate: No unused fragment arguments', () => {
+  it('uses all arguments', () => {
+    expectValid(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(a: $a, b: $b, c: $c)
+      }
+    `);
+  });
+
+  it('uses all arguments deeply', () => {
+    expectValid(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(a: $a) {
+          field(b: $b) {
+            field(c: $c)
+          }
+        }
+      }
+    `);
+  });
+
+  it('uses all arguments deeply in inline fragments', () => {
+    expectValid(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        ... on Type {
+          field(a: $a) {
+            field(b: $b) {
+              ... on Type {
+                field(c: $c)
+              }
+            }
+          }
+        }
+      }
+    `);
+  });
+
+  it('argument not used', () => {
+    expectErrors(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(a: $a, b: $b)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Argument "$c" is never used in fragment "Foo".',
+        locations: [{ line: 2, column: 44 }],
+      },
+    ]);
+  });
+
+  it('query passes in unused argument', () => {
+    expectErrors(`
+      query Q($c: String) {
+        type {
+          ...Foo(a: "", b: "", c: $c)
+        }
+      }
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(a: $a, b: $b)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Argument "$c" is never used in fragment "Foo".',
+        locations: [{ line: 7, column: 44 }],
+      },
+    ]);
+  });
+
+  it('child fragment uses a variable of the same name', () => {
+    expectErrors(`
+      query Q($a: String) {
+        type {
+          ...Foo
+        }
+      }
+      fragment Foo($a: String) on Type {
+        ...Bar
+      }
+      fragment Bar on Type {
+        field(a: $a)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Argument "$a" is never used in fragment "Foo".',
+        locations: [{ line: 7, column: 20 }],
+      },
+    ]);
+  });
+
+  it('multiple arguments not used', () => {
+    expectErrors(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(b: $b)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Argument "$a" is never used in fragment "Foo".',
+        locations: [{ line: 2, column: 20 }],
+      },
+      {
+        message: 'Argument "$c" is never used in fragment "Foo".',
+        locations: [{ line: 2, column: 44 }],
+      },
+    ]);
+  });
+});
diff --git a/src/validation/index.ts b/src/validation/index.ts
index b0cc754490..b5c4312607 100644
--- a/src/validation/index.ts
+++ b/src/validation/index.ts
@@ -48,6 +48,9 @@ export { NoUndefinedVariablesRule } from './rules/NoUndefinedVariablesRule.js';
 // Spec Section: "Fragments must be used"
 export { NoUnusedFragmentsRule } from './rules/NoUnusedFragmentsRule.js';
 
+// Spec Section: "All Fragment Arguments Used"
+export { NoUnusedFragmentArgumentsRule } from './rules/NoUnusedFragmentArgumentsRule.js';
+
 // Spec Section: "All Variables Used"
 export { NoUnusedVariablesRule } from './rules/NoUnusedVariablesRule.js';
 
diff --git a/src/validation/rules/KnownDirectivesRule.ts b/src/validation/rules/KnownDirectivesRule.ts
index 57641b91e7..daee27a798 100644
--- a/src/validation/rules/KnownDirectivesRule.ts
+++ b/src/validation/rules/KnownDirectivesRule.ts
@@ -86,6 +86,8 @@ function getDirectiveLocationForASTPath(
       return DirectiveLocation.INLINE_FRAGMENT;
     case Kind.FRAGMENT_DEFINITION:
       return DirectiveLocation.FRAGMENT_DEFINITION;
+    case Kind.FRAGMENT_ARGUMENT_DEFINITION:
+      return DirectiveLocation.FRAGMENT_ARGUMENT_DEFINITION;
     case Kind.VARIABLE_DEFINITION:
       return DirectiveLocation.VARIABLE_DEFINITION;
     case Kind.SCHEMA_DEFINITION:
diff --git a/src/validation/rules/NoUndefinedVariablesRule.ts b/src/validation/rules/NoUndefinedVariablesRule.ts
index d1672ecd0b..8375d82b5e 100644
--- a/src/validation/rules/NoUndefinedVariablesRule.ts
+++ b/src/validation/rules/NoUndefinedVariablesRule.ts
@@ -22,7 +22,10 @@ export function NoUndefinedVariablesRule(
       );
 
       const usages = context.getRecursiveVariableUsages(operation);
-      for (const { node } of usages) {
+      for (const { node, fragmentArgDef } of usages) {
+        if (fragmentArgDef) {
+          continue;
+        }
         const varName = node.name.value;
         if (!variableNameDefined.has(varName)) {
           context.reportError(
diff --git a/src/validation/rules/NoUnusedFragmentArgumentsRule.ts b/src/validation/rules/NoUnusedFragmentArgumentsRule.ts
new file mode 100644
index 0000000000..1002d4c1d1
--- /dev/null
+++ b/src/validation/rules/NoUnusedFragmentArgumentsRule.ts
@@ -0,0 +1,40 @@
+import { GraphQLError } from '../../error/GraphQLError.js';
+
+import type { ASTVisitor } from '../../language/visitor.js';
+
+import type { ValidationContext } from '../ValidationContext.js';
+
+/**
+ * No unused variables
+ *
+ * A GraphQL fragment is only valid if all arguments defined by it
+ * are used within the same fragment.
+ *
+ * See https://spec.graphql.org/draft/#sec-All-Variables-Used
+ */
+export function NoUnusedFragmentArgumentsRule(
+  context: ValidationContext,
+): ASTVisitor {
+  return {
+    FragmentDefinition(fragment) {
+      const usages = context.getVariableUsages(fragment);
+      const argumentNameUsed = new Set<string>(
+        usages.map(({ node }) => node.name.value),
+      );
+      // FIXME: https://github.com/graphql/graphql-js/issues/2203
+      /* c8 ignore next */
+      const argumentDefinitions = fragment.arguments ?? [];
+      for (const argDef of argumentDefinitions) {
+        const argName = argDef.variable.name.value;
+        if (!argumentNameUsed.has(argName)) {
+          context.reportError(
+            new GraphQLError(
+              `Argument "$${argName}" is never used in fragment "${fragment.name.value}".`,
+              { nodes: argDef },
+            ),
+          );
+        }
+      }
+    },
+  };
+}
diff --git a/src/validation/rules/NoUnusedVariablesRule.ts b/src/validation/rules/NoUnusedVariablesRule.ts
index 7a0660cce0..7fbeb82764 100644
--- a/src/validation/rules/NoUnusedVariablesRule.ts
+++ b/src/validation/rules/NoUnusedVariablesRule.ts
@@ -17,7 +17,10 @@ export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor {
     OperationDefinition(operation) {
       const usages = context.getRecursiveVariableUsages(operation);
       const variableNameUsed = new Set<string>(
-        usages.map(({ node }) => node.name.value),
+        usages
+          // Skip variables used as fragment arguments
+          .filter(({ fragmentArgDef }) => !fragmentArgDef)
+          .map(({ node }) => node.name.value),
       );
 
       // FIXME: https://github.com/graphql/graphql-js/issues/2203
diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
index e2444047c6..d46aae5a6f 100644
--- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
+++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
@@ -8,6 +8,7 @@ import type {
   DirectiveNode,
   FieldNode,
   FragmentDefinitionNode,
+  FragmentSpreadNode,
   ObjectValueNode,
   SelectionSetNode,
 } from '../../language/ast.js';
@@ -29,7 +30,9 @@ import {
   isObjectType,
 } from '../../type/definition.js';
 
+import { keyForFragmentSpread } from '../../utilities/keyForFragmentSpread.js';
 import { sortValueNode } from '../../utilities/sortValueNode.js';
+import { substituteFragmentArguments } from '../../utilities/substituteFragmentArguments.js';
 import { typeFromAST } from '../../utilities/typeFromAST.js';
 
 import type { ValidationContext } from '../ValidationContext.js';
@@ -38,17 +41,25 @@ import type { ValidationContext } from '../ValidationContext.js';
 // This file contains a lot of such errors but we plan to refactor it anyway
 // so just disable it for entire file.
 
-function reasonMessage(reason: ConflictReasonMessage): string {
-  if (Array.isArray(reason)) {
-    return reason
-      .map(
-        ([responseName, subReason]) =>
-          `subfields "${responseName}" conflict because ` +
-          reasonMessage(subReason),
-      )
+function reasonMessage(message: ConflictReasonMessage): string {
+  if (Array.isArray(message)) {
+    return message
+      .map((subC) => {
+        const subConflict = subC;
+        if (subConflict.kind === 'FIELD') {
+          return (
+            `subfields "${subConflict.responseName}" conflict because ` +
+            reasonMessage(subConflict.reasonMessage)
+          );
+        }
+        return (
+          `child spreads "${subConflict.fragmentName}" conflict because ` +
+          reasonMessage(subConflict.reasonMessage)
+        );
+      })
       .join(' and ');
   }
-  return reason;
+  return message;
 }
 
 /**
@@ -68,36 +79,62 @@ export function OverlappingFieldsCanBeMergedRule(
   // dramatically improve the performance of this validator.
   const comparedFragmentPairs = new PairSet();
 
-  // A cache for the "field map" and list of fragment names found in any given
+  // A cache for the "field map" and list of fragment spreads found in any given
   // selection set. Selection sets may be asked for this information multiple
   // times, so this improves the performance of this validator.
-  const cachedFieldsAndFragmentNames = new Map();
+  const cachedFieldsAndFragmentSpreads = new Map();
 
   return {
     SelectionSet(selectionSet) {
       const conflicts = findConflictsWithinSelectionSet(
         context,
-        cachedFieldsAndFragmentNames,
+        cachedFieldsAndFragmentSpreads,
         comparedFragmentPairs,
         context.getParentType(),
         selectionSet,
       );
-      for (const [[responseName, reason], fields1, fields2] of conflicts) {
-        const reasonMsg = reasonMessage(reason);
-        context.reportError(
-          new GraphQLError(
-            `Fields "${responseName}" conflict because ${reasonMsg}. Use different aliases on the fields to fetch both if this was intentional.`,
-            { nodes: fields1.concat(fields2) },
-          ),
-        );
+      for (const { reason, selectionPath1, selectionPath2 } of conflicts) {
+        const reasonMsg = reasonMessage(reason.reasonMessage);
+        const errorNodes = { nodes: selectionPath1.concat(selectionPath2) };
+        if (reason.kind === 'FIELD') {
+          context.reportError(
+            new GraphQLError(
+              `Fields "${reason.responseName}" conflict because ${reasonMsg}. Use different aliases on the fields to fetch both if this was intentional.`,
+              errorNodes,
+            ),
+          );
+        } else {
+          // FRAGMENT_SPREAD
+          context.reportError(
+            new GraphQLError(
+              // Fragments can't be aliased, so there's no easy way to resolve these conflicts today.
+              `Spreads "${reason.fragmentName}" conflict because ${reasonMsg}.`,
+              errorNodes,
+            ),
+          );
+        }
       }
     },
   };
 }
 
-type Conflict = [ConflictReason, Array<FieldNode>, Array<FieldNode>];
+interface Conflict {
+  reason: ConflictReason;
+  selectionPath1: Array<FieldNode | FragmentSpreadNode>;
+  selectionPath2: Array<FieldNode | FragmentSpreadNode>;
+}
 // Field name and reason.
-type ConflictReason = [string, ConflictReasonMessage];
+type ConflictReason = FieldConflictReason | FragmentSpreadConflictReason;
+interface FieldConflictReason {
+  kind: 'FIELD';
+  responseName: string;
+  reasonMessage: ConflictReasonMessage;
+}
+interface FragmentSpreadConflictReason {
+  kind: 'FRAGMENT_SPREAD';
+  fragmentName: string;
+  reasonMessage: ConflictReasonMessage;
+}
 // Reason is a string, or a nested list of conflicts.
 type ConflictReasonMessage = string | Array<ConflictReason>;
 // Tuple defining a field node in a context.
@@ -108,8 +145,11 @@ type NodeAndDef = [
 ];
 // Map of array of those.
 type NodeAndDefCollection = ObjMap<Array<NodeAndDef>>;
-type FragmentNames = ReadonlyArray<string>;
-type FieldsAndFragmentNames = readonly [NodeAndDefCollection, FragmentNames];
+type FragmentSpreadsByKey = ObjMap<FragmentSpreadNode>;
+type FieldsAndFragmentSpreads = readonly [
+  NodeAndDefCollection,
+  FragmentSpreadsByKey,
+];
 
 /**
  * Algorithm:
@@ -171,58 +211,60 @@ type FieldsAndFragmentNames = readonly [NodeAndDefCollection, FragmentNames];
 // GraphQL Document.
 function findConflictsWithinSelectionSet(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   parentType: Maybe<GraphQLNamedType>,
   selectionSet: SelectionSetNode,
 ): Array<Conflict> {
   const conflicts: Array<Conflict> = [];
 
-  const [fieldMap, fragmentNames] = getFieldsAndFragmentNames(
+  const [fieldMap, spreadCollection] = getFieldsAndFragmentSpreads(
     context,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     parentType,
     selectionSet,
   );
 
-  // (A) Find find all conflicts "within" the fields of this selection set.
+  // (A) First find all conflicts "within" the fields and spreads of this selection set.
   // Note: this is the *only place* `collectConflictsWithin` is called.
   collectConflictsWithin(
     context,
     conflicts,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     comparedFragmentPairs,
     fieldMap,
   );
 
-  if (fragmentNames.length !== 0) {
-    // (B) Then collect conflicts between these fields and those represented by
-    // each spread fragment name found.
-    for (let i = 0; i < fragmentNames.length; i++) {
-      collectConflictsBetweenFieldsAndFragment(
+  // (B) Then collect conflicts between these fields and those represented by
+  // each spread fragment name found.
+  const fragmentSpreads = Object.values(spreadCollection);
+  for (let i = 0; i < fragmentSpreads.length; i++) {
+    collectConflictsBetweenFieldsAndFragment(
+      context,
+      conflicts,
+      cachedFieldsAndFragmentSpreads,
+      comparedFragmentPairs,
+      false,
+      fieldMap,
+      fragmentSpreads[i],
+    );
+    // (C) Then compare this fragment with all other fragments found in this
+    // selection set to collect conflicts between fragments spread together.
+    // This compares each item in the list of fragment names to every other
+    // item in that same list (except for itself).
+    for (let j = i + 1; j < fragmentSpreads.length; j++) {
+      collectConflictsBetweenFragments(
         context,
         conflicts,
-        cachedFieldsAndFragmentNames,
+        cachedFieldsAndFragmentSpreads,
         comparedFragmentPairs,
         false,
-        fieldMap,
-        fragmentNames[i],
+        fragmentSpreads[i],
+        fragmentSpreads[j],
       );
-      // (C) Then compare this fragment with all other fragments found in this
-      // selection set to collect conflicts between fragments spread together.
-      // This compares each item in the list of fragment names to every other
-      // item in that same list (except for itself).
-      for (let j = i + 1; j < fragmentNames.length; j++) {
-        collectConflictsBetweenFragments(
-          context,
-          conflicts,
-          cachedFieldsAndFragmentNames,
-          comparedFragmentPairs,
-          false,
-          fragmentNames[i],
-          fragmentNames[j],
-        );
-      }
     }
   }
   return conflicts;
@@ -233,22 +275,28 @@ function findConflictsWithinSelectionSet(
 function collectConflictsBetweenFieldsAndFragment(
   context: ValidationContext,
   conflicts: Array<Conflict>,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   areMutuallyExclusive: boolean,
   fieldMap: NodeAndDefCollection,
-  fragmentName: string,
+  fragmentSpread: FragmentSpreadNode,
 ): void {
-  const fragment = context.getFragment(fragmentName);
-  if (!fragment) {
+  const fragmentName = fragmentSpread.name.value;
+  const fragmentDef = context.getFragment(fragmentName);
+  if (!fragmentDef) {
     return;
   }
 
-  const [fieldMap2, referencedFragmentNames] =
-    getReferencedFieldsAndFragmentNames(
+  const fragmentKey = keyForFragmentSpread(fragmentSpread);
+  const [fieldMap2, referencedFragmentSpreads] =
+    getReferencedFieldsAndFragmentSpreads(
       context,
-      cachedFieldsAndFragmentNames,
-      fragment,
+      cachedFieldsAndFragmentSpreads,
+      fragmentDef,
+      fragmentSpread,
     );
 
   // Do not compare a fragment's fieldMap to itself.
@@ -261,7 +309,7 @@ function collectConflictsBetweenFieldsAndFragment(
   collectConflictsBetween(
     context,
     conflicts,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     comparedFragmentPairs,
     areMutuallyExclusive,
     fieldMap,
@@ -270,31 +318,34 @@ function collectConflictsBetweenFieldsAndFragment(
 
   // (E) Then collect any conflicts between the provided collection of fields
   // and any fragment names found in the given fragment.
-  for (const referencedFragmentName of referencedFragmentNames) {
+  for (const [
+    referencedFragmentKey,
+    referencedFragmentSpread,
+  ] of Object.entries(referencedFragmentSpreads)) {
     // Memoize so two fragments are not compared for conflicts more than once.
     if (
       comparedFragmentPairs.has(
-        referencedFragmentName,
-        fragmentName,
+        referencedFragmentKey,
+        fragmentKey,
         areMutuallyExclusive,
       )
     ) {
       continue;
     }
     comparedFragmentPairs.add(
-      referencedFragmentName,
-      fragmentName,
+      referencedFragmentKey,
+      fragmentKey,
       areMutuallyExclusive,
     );
 
     collectConflictsBetweenFieldsAndFragment(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
       fieldMap,
-      referencedFragmentName,
+      referencedFragmentSpread,
     );
   }
 }
@@ -304,46 +355,64 @@ function collectConflictsBetweenFieldsAndFragment(
 function collectConflictsBetweenFragments(
   context: ValidationContext,
   conflicts: Array<Conflict>,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   areMutuallyExclusive: boolean,
-  fragmentName1: string,
-  fragmentName2: string,
+  fragmentSpread1: FragmentSpreadNode,
+  fragmentSpread2: FragmentSpreadNode,
 ): void {
+  const fragmentKey1 = keyForFragmentSpread(fragmentSpread1);
+  const fragmentKey2 = keyForFragmentSpread(fragmentSpread2);
   // No need to compare a fragment to itself.
-  if (fragmentName1 === fragmentName2) {
+  if (fragmentKey1 === fragmentKey2) {
     return;
   }
 
   // Memoize so two fragments are not compared for conflicts more than once.
   if (
-    comparedFragmentPairs.has(
-      fragmentName1,
-      fragmentName2,
-      areMutuallyExclusive,
-    )
+    comparedFragmentPairs.has(fragmentKey1, fragmentKey2, areMutuallyExclusive)
   ) {
     return;
   }
-  comparedFragmentPairs.add(fragmentName1, fragmentName2, areMutuallyExclusive);
+  comparedFragmentPairs.add(fragmentKey1, fragmentKey2, areMutuallyExclusive);
+
+  // Two unique fragment spreads reference the same fragment,
+  // which is a conflict
+  if (fragmentSpread1.name.value === fragmentSpread2.name.value) {
+    conflicts.push({
+      reason: {
+        kind: 'FRAGMENT_SPREAD',
+        fragmentName: fragmentSpread1.name.value,
+        reasonMessage: `${fragmentKey1} and ${fragmentKey2} have different fragment arguments`,
+      },
+      selectionPath1: [fragmentSpread1],
+      selectionPath2: [fragmentSpread2],
+    });
+    return;
+  }
 
-  const fragment1 = context.getFragment(fragmentName1);
-  const fragment2 = context.getFragment(fragmentName2);
-  if (!fragment1 || !fragment2) {
+  const fragmentDef1 = context.getFragment(fragmentSpread1.name.value);
+  const fragmentDef2 = context.getFragment(fragmentSpread2.name.value);
+  if (!fragmentDef1 || !fragmentDef2) {
     return;
   }
 
-  const [fieldMap1, referencedFragmentNames1] =
-    getReferencedFieldsAndFragmentNames(
+  const [fieldMap1, referencedFragmentSpreads1] =
+    getReferencedFieldsAndFragmentSpreads(
       context,
-      cachedFieldsAndFragmentNames,
-      fragment1,
+      cachedFieldsAndFragmentSpreads,
+      fragmentDef1,
+      fragmentSpread1,
     );
-  const [fieldMap2, referencedFragmentNames2] =
-    getReferencedFieldsAndFragmentNames(
+  const [fieldMap2, referencedFragmentSpreads2] =
+    getReferencedFieldsAndFragmentSpreads(
       context,
-      cachedFieldsAndFragmentNames,
-      fragment2,
+      cachedFieldsAndFragmentSpreads,
+      fragmentDef2,
+      fragmentSpread2,
     );
 
   // (F) First, collect all conflicts between these two collections of fields
@@ -351,7 +420,7 @@ function collectConflictsBetweenFragments(
   collectConflictsBetween(
     context,
     conflicts,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     comparedFragmentPairs,
     areMutuallyExclusive,
     fieldMap1,
@@ -360,29 +429,33 @@ function collectConflictsBetweenFragments(
 
   // (G) Then collect conflicts between the first fragment and any nested
   // fragments spread in the second fragment.
-  for (const referencedFragmentName2 of referencedFragmentNames2) {
+  for (const referencedFragmentSpread2 of Object.values(
+    referencedFragmentSpreads2,
+  )) {
     collectConflictsBetweenFragments(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
-      fragmentName1,
-      referencedFragmentName2,
+      fragmentSpread1,
+      referencedFragmentSpread2,
     );
   }
 
   // (G) Then collect conflicts between the second fragment and any nested
   // fragments spread in the first fragment.
-  for (const referencedFragmentName1 of referencedFragmentNames1) {
+  for (const referencedFragmentSpread1 of Object.values(
+    referencedFragmentSpreads1,
+  )) {
     collectConflictsBetweenFragments(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
-      referencedFragmentName1,
-      fragmentName2,
+      referencedFragmentSpread1,
+      fragmentSpread2,
     );
   }
 }
@@ -392,7 +465,10 @@ function collectConflictsBetweenFragments(
 // between the sub-fields of two overlapping fields.
 function findConflictsBetweenSubSelectionSets(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   areMutuallyExclusive: boolean,
   parentType1: Maybe<GraphQLNamedType>,
@@ -402,15 +478,15 @@ function findConflictsBetweenSubSelectionSets(
 ): Array<Conflict> {
   const conflicts: Array<Conflict> = [];
 
-  const [fieldMap1, fragmentNames1] = getFieldsAndFragmentNames(
+  const [fieldMap1, fragmentSpreads1] = getFieldsAndFragmentSpreads(
     context,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     parentType1,
     selectionSet1,
   );
-  const [fieldMap2, fragmentNames2] = getFieldsAndFragmentNames(
+  const [fieldMap2, fragmentSpreads2] = getFieldsAndFragmentSpreads(
     context,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     parentType2,
     selectionSet2,
   );
@@ -419,54 +495,56 @@ function findConflictsBetweenSubSelectionSets(
   collectConflictsBetween(
     context,
     conflicts,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     comparedFragmentPairs,
     areMutuallyExclusive,
     fieldMap1,
     fieldMap2,
   );
 
+  // (H.1) Collect all conflicts between the two sets of fragment spreads
+
   // (I) Then collect conflicts between the first collection of fields and
-  // those referenced by each fragment name associated with the second.
-  for (const fragmentName2 of fragmentNames2) {
+  // those referenced by each fragment spread associated with the second.
+  for (const fragmentSpread2 of Object.values(fragmentSpreads2)) {
     collectConflictsBetweenFieldsAndFragment(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
       fieldMap1,
-      fragmentName2,
+      fragmentSpread2,
     );
   }
 
   // (I) Then collect conflicts between the second collection of fields and
-  // those referenced by each fragment name associated with the first.
-  for (const fragmentName1 of fragmentNames1) {
+  // those referenced by each fragment spread associated with the first.
+  for (const fragmentSpread1 of Object.values(fragmentSpreads1)) {
     collectConflictsBetweenFieldsAndFragment(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
       fieldMap2,
-      fragmentName1,
+      fragmentSpread1,
     );
   }
 
-  // (J) Also collect conflicts between any fragment names by the first and
-  // fragment names by the second. This compares each item in the first set of
-  // names to each item in the second set of names.
-  for (const fragmentName1 of fragmentNames1) {
-    for (const fragmentName2 of fragmentNames2) {
+  // (J) Also collect conflicts between any fragment spreads by the first and
+  // fragment spreads by the second. This compares each item in the first set of
+  // spreads to each item in the second set of spreads.
+  for (const fragmentSpread1 of Object.values(fragmentSpreads1)) {
+    for (const fragmentSpread2 of Object.values(fragmentSpreads2)) {
       collectConflictsBetweenFragments(
         context,
         conflicts,
-        cachedFieldsAndFragmentNames,
+        cachedFieldsAndFragmentSpreads,
         comparedFragmentPairs,
         areMutuallyExclusive,
-        fragmentName1,
-        fragmentName2,
+        fragmentSpread1,
+        fragmentSpread2,
       );
     }
   }
@@ -477,7 +555,10 @@ function findConflictsBetweenSubSelectionSets(
 function collectConflictsWithin(
   context: ValidationContext,
   conflicts: Array<Conflict>,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   fieldMap: NodeAndDefCollection,
 ): void {
@@ -492,9 +573,9 @@ function collectConflictsWithin(
     if (fields.length > 1) {
       for (let i = 0; i < fields.length; i++) {
         for (let j = i + 1; j < fields.length; j++) {
-          const conflict = findConflict(
+          const conflict = findFieldConflict(
             context,
-            cachedFieldsAndFragmentNames,
+            cachedFieldsAndFragmentSpreads,
             comparedFragmentPairs,
             false, // within one collection is never mutually exclusive
             responseName,
@@ -518,7 +599,10 @@ function collectConflictsWithin(
 function collectConflictsBetween(
   context: ValidationContext,
   conflicts: Array<Conflict>,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   parentFieldsAreMutuallyExclusive: boolean,
   fieldMap1: NodeAndDefCollection,
@@ -534,9 +618,9 @@ function collectConflictsBetween(
     if (fields2) {
       for (const field1 of fields1) {
         for (const field2 of fields2) {
-          const conflict = findConflict(
+          const conflict = findFieldConflict(
             context,
-            cachedFieldsAndFragmentNames,
+            cachedFieldsAndFragmentSpreads,
             comparedFragmentPairs,
             parentFieldsAreMutuallyExclusive,
             responseName,
@@ -554,9 +638,12 @@ function collectConflictsBetween(
 
 // Determines if there is a conflict between two particular fields, including
 // comparing their sub-fields.
-function findConflict(
+function findFieldConflict(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   parentFieldsAreMutuallyExclusive: boolean,
   responseName: string,
@@ -585,20 +672,28 @@ function findConflict(
     const name1 = node1.name.value;
     const name2 = node2.name.value;
     if (name1 !== name2) {
-      return [
-        [responseName, `"${name1}" and "${name2}" are different fields`],
-        [node1],
-        [node2],
-      ];
+      return {
+        reason: {
+          kind: 'FIELD',
+          responseName,
+          reasonMessage: `"${name1}" and "${name2}" are different fields`,
+        },
+        selectionPath1: [node1],
+        selectionPath2: [node2],
+      };
     }
 
     // Two field calls must have the same arguments.
     if (stringifyArguments(node1) !== stringifyArguments(node2)) {
-      return [
-        [responseName, 'they have differing arguments'],
-        [node1],
-        [node2],
-      ];
+      return {
+        reason: {
+          kind: 'FIELD',
+          responseName,
+          reasonMessage: 'they have differing arguments',
+        },
+        selectionPath1: [node1],
+        selectionPath2: [node2],
+      };
     }
   }
 
@@ -606,11 +701,15 @@ function findConflict(
   const directives1 = /* c8 ignore next */ node1.directives ?? [];
   const directives2 = /* c8 ignore next */ node2.directives ?? [];
   if (!sameStreams(directives1, directives2)) {
-    return [
-      [responseName, 'they have differing stream directives'],
-      [node1],
-      [node2],
-    ];
+    return {
+      reason: {
+        kind: 'FIELD',
+        responseName,
+        reasonMessage: 'they have differing stream directives',
+      },
+      selectionPath1: [node1],
+      selectionPath2: [node2],
+    };
   }
 
   // The return type for each field.
@@ -618,16 +717,17 @@ function findConflict(
   const type2 = def2?.type;
 
   if (type1 && type2 && doTypesConflict(type1, type2)) {
-    return [
-      [
+    return {
+      reason: {
+        kind: 'FIELD',
         responseName,
-        `they return conflicting types "${inspect(type1)}" and "${inspect(
-          type2,
-        )}"`,
-      ],
-      [node1],
-      [node2],
-    ];
+        reasonMessage: `they return conflicting types "${inspect(
+          type1,
+        )}" and "${inspect(type2)}"`,
+      },
+      selectionPath1: [node1],
+      selectionPath2: [node2],
+    };
   }
 
   // Collect and compare sub-fields. Use the same "visited fragment names" list
@@ -638,7 +738,7 @@ function findConflict(
   if (selectionSet1 && selectionSet2) {
     const conflicts = findConflictsBetweenSubSelectionSets(
       context,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
       getNamedType(type1),
@@ -720,58 +820,73 @@ function doTypesConflict(
 // Given a selection set, return the collection of fields (a mapping of response
 // name to field nodes and definitions) as well as a list of fragment names
 // referenced via fragment spreads.
-function getFieldsAndFragmentNames(
+function getFieldsAndFragmentSpreads(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   parentType: Maybe<GraphQLNamedType>,
   selectionSet: SelectionSetNode,
-): FieldsAndFragmentNames {
-  const cached = cachedFieldsAndFragmentNames.get(selectionSet);
+): FieldsAndFragmentSpreads {
+  const cached = cachedFieldsAndFragmentSpreads.get(selectionSet);
   if (cached) {
     return cached;
   }
   const nodeAndDefs: NodeAndDefCollection = Object.create(null);
-  const fragmentNames = new Set<string>();
-  _collectFieldsAndFragmentNames(
+  const fragmentSpreads: ObjMap<FragmentSpreadNode> = {};
+  _collectFieldsAndFragmentSpreads(
     context,
     parentType,
     selectionSet,
     nodeAndDefs,
-    fragmentNames,
+    fragmentSpreads,
   );
-  const result = [nodeAndDefs, [...fragmentNames]] as const;
-  cachedFieldsAndFragmentNames.set(selectionSet, result);
+  const result = [nodeAndDefs, { ...fragmentSpreads }] as const;
+  cachedFieldsAndFragmentSpreads.set(selectionSet, result);
   return result;
 }
 
 // Given a reference to a fragment, return the represented collection of fields
-// as well as a list of nested fragment names referenced via fragment spreads.
-function getReferencedFieldsAndFragmentNames(
+// as well as a list of nested referenced fragment spreads.
+function getReferencedFieldsAndFragmentSpreads(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
-  fragment: FragmentDefinitionNode,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
+  fragmentDef: FragmentDefinitionNode,
+  fragmentSpread: FragmentSpreadNode,
 ) {
+  const fragmentSelectionSet = substituteFragmentArguments(
+    fragmentDef,
+    fragmentSpread,
+  );
+
   // Short-circuit building a type from the node if possible.
-  const cached = cachedFieldsAndFragmentNames.get(fragment.selectionSet);
+  const cached = cachedFieldsAndFragmentSpreads.get(fragmentSelectionSet);
   if (cached) {
     return cached;
   }
 
-  const fragmentType = typeFromAST(context.getSchema(), fragment.typeCondition);
-  return getFieldsAndFragmentNames(
+  const fragmentType = typeFromAST(
+    context.getSchema(),
+    fragmentDef.typeCondition,
+  );
+  return getFieldsAndFragmentSpreads(
     context,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     fragmentType,
-    fragment.selectionSet,
+    fragmentSelectionSet,
   );
 }
 
-function _collectFieldsAndFragmentNames(
+function _collectFieldsAndFragmentSpreads(
   context: ValidationContext,
   parentType: Maybe<GraphQLNamedType>,
   selectionSet: SelectionSetNode,
   nodeAndDefs: NodeAndDefCollection,
-  fragmentNames: Set<string>,
+  fragmentSpreads: FragmentSpreadsByKey,
 ): void {
   for (const selection of selectionSet.selections) {
     switch (selection.kind) {
@@ -791,19 +906,19 @@ function _collectFieldsAndFragmentNames(
         break;
       }
       case Kind.FRAGMENT_SPREAD:
-        fragmentNames.add(selection.name.value);
+        fragmentSpreads[keyForFragmentSpread(selection)] = selection;
         break;
       case Kind.INLINE_FRAGMENT: {
         const typeCondition = selection.typeCondition;
         const inlineFragmentType = typeCondition
           ? typeFromAST(context.getSchema(), typeCondition)
           : parentType;
-        _collectFieldsAndFragmentNames(
+        _collectFieldsAndFragmentSpreads(
           context,
           inlineFragmentType,
           selection.selectionSet,
           nodeAndDefs,
-          fragmentNames,
+          fragmentSpreads,
         );
         break;
       }
@@ -820,11 +935,21 @@ function subfieldConflicts(
   node2: FieldNode,
 ): Maybe<Conflict> {
   if (conflicts.length > 0) {
-    return [
-      [responseName, conflicts.map(([reason]) => reason)],
-      [node1, ...conflicts.map(([, fields1]) => fields1).flat()],
-      [node2, ...conflicts.map(([, , fields2]) => fields2).flat()],
-    ];
+    return {
+      reason: {
+        kind: 'FIELD',
+        responseName,
+        reasonMessage: conflicts.map((conflict) => conflict.reason),
+      },
+      selectionPath1: [
+        node1,
+        ...conflicts.map((subConflict) => subConflict.selectionPath1).flat(),
+      ],
+      selectionPath2: [
+        node2,
+        ...conflicts.map((subConflict) => subConflict.selectionPath2).flat(),
+      ],
+    };
   }
 }
 
diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts
index 350264496f..c513a721c4 100644
--- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts
+++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts
@@ -4,7 +4,10 @@ import type { ObjMap } from '../../jsutils/ObjMap.js';
 
 import { GraphQLError } from '../../error/GraphQLError.js';
 
-import type { InputValueDefinitionNode } from '../../language/ast.js';
+import type {
+  FragmentArgumentDefinitionNode,
+  InputValueDefinitionNode,
+} from '../../language/ast.js';
 import { Kind } from '../../language/kinds.js';
 import { print } from '../../language/printer.js';
 import type { ASTVisitor } from '../../language/visitor.js';
@@ -56,6 +59,37 @@ export function ProvidedRequiredArgumentsRule(
         }
       },
     },
+    FragmentSpread: {
+      // Validate on leave to allow for directive errors to appear first.
+      leave(spreadNode) {
+        const fragmentDef = context.getFragment(spreadNode.name.value);
+        if (!fragmentDef) {
+          return false;
+        }
+
+        const providedArgs = new Set(
+          // FIXME: https://github.com/graphql/graphql-js/issues/2203
+          /* c8 ignore next */
+          spreadNode.arguments?.map((arg) => arg.name.value),
+        );
+        // FIXME: https://github.com/graphql/graphql-js/issues/2203
+        /* c8 ignore next */
+        for (const argDef of fragmentDef.arguments ?? []) {
+          if (
+            !providedArgs.has(argDef.variable.name.value) &&
+            isRequiredArgumentNode(argDef)
+          ) {
+            const argTypeStr = inspect(argDef.type);
+            context.reportError(
+              new GraphQLError(
+                `Fragment "${spreadNode.name.value}" argument "${argDef.variable.name.value}" of type "${argTypeStr}" is required, but it was not provided.`,
+                { nodes: [spreadNode, argDef] },
+              ),
+            );
+          }
+        }
+      },
+    },
   };
 }
 
@@ -122,6 +156,8 @@ export function ProvidedRequiredArgumentsOnDirectivesRule(
   };
 }
 
-function isRequiredArgumentNode(arg: InputValueDefinitionNode): boolean {
+function isRequiredArgumentNode(
+  arg: InputValueDefinitionNode | FragmentArgumentDefinitionNode,
+): boolean {
   return arg.type.kind === Kind.NON_NULL_TYPE && arg.defaultValue == null;
 }
diff --git a/src/validation/rules/VariablesAreInputTypesRule.ts b/src/validation/rules/VariablesAreInputTypesRule.ts
index 019f3e762c..b4a7ad1de8 100644
--- a/src/validation/rules/VariablesAreInputTypesRule.ts
+++ b/src/validation/rules/VariablesAreInputTypesRule.ts
@@ -1,6 +1,9 @@
 import { GraphQLError } from '../../error/GraphQLError.js';
 
-import type { VariableDefinitionNode } from '../../language/ast.js';
+import type {
+  FragmentArgumentDefinitionNode,
+  VariableDefinitionNode,
+} from '../../language/ast.js';
 import { print } from '../../language/printer.js';
 import type { ASTVisitor } from '../../language/visitor.js';
 
@@ -21,21 +24,25 @@ import type { ValidationContext } from '../ValidationContext.js';
 export function VariablesAreInputTypesRule(
   context: ValidationContext,
 ): ASTVisitor {
-  return {
-    VariableDefinition(node: VariableDefinitionNode) {
-      const type = typeFromAST(context.getSchema(), node.type);
+  const validateNode = (
+    node: VariableDefinitionNode | FragmentArgumentDefinitionNode,
+  ) => {
+    const type = typeFromAST(context.getSchema(), node.type);
 
-      if (type !== undefined && !isInputType(type)) {
-        const variableName = node.variable.name.value;
-        const typeName = print(node.type);
+    if (type !== undefined && !isInputType(type)) {
+      const variableName = node.variable.name.value;
+      const typeName = print(node.type);
 
-        context.reportError(
-          new GraphQLError(
-            `Variable "$${variableName}" cannot be non-input type "${typeName}".`,
-            { nodes: node.type },
-          ),
-        );
-      }
-    },
+      context.reportError(
+        new GraphQLError(
+          `Variable "$${variableName}" cannot be non-input type "${typeName}".`,
+          { nodes: node.type },
+        ),
+      );
+    }
+  };
+  return {
+    VariableDefinition: validateNode,
+    FragmentArgumentDefinition: validateNode,
   };
 }
diff --git a/src/validation/rules/VariablesInAllowedPositionRule.ts b/src/validation/rules/VariablesInAllowedPositionRule.ts
index 4039540eba..6f4f013bff 100644
--- a/src/validation/rules/VariablesInAllowedPositionRule.ts
+++ b/src/validation/rules/VariablesInAllowedPositionRule.ts
@@ -1,9 +1,10 @@
 import { inspect } from '../../jsutils/inspect.js';
 import type { Maybe } from '../../jsutils/Maybe.js';
+import type { ObjMap } from '../../jsutils/ObjMap.js';
 
 import { GraphQLError } from '../../error/GraphQLError.js';
 
-import type { ValueNode } from '../../language/ast.js';
+import type { ValueNode, VariableDefinitionNode } from '../../language/ast.js';
 import { Kind } from '../../language/kinds.js';
 import type { ASTVisitor } from '../../language/visitor.js';
 
@@ -26,7 +27,7 @@ import type { ValidationContext } from '../ValidationContext.js';
 export function VariablesInAllowedPositionRule(
   context: ValidationContext,
 ): ASTVisitor {
-  let varDefMap = Object.create(null);
+  let varDefMap: ObjMap<VariableDefinitionNode> = Object.create(null);
 
   return {
     OperationDefinition: {
@@ -36,9 +37,9 @@ export function VariablesInAllowedPositionRule(
       leave(operation) {
         const usages = context.getRecursiveVariableUsages(operation);
 
-        for (const { node, type, defaultValue } of usages) {
+        for (const { node, type, defaultValue, fragmentArgDef } of usages) {
           const varName = node.name.value;
-          const varDef = varDefMap[varName];
+          const varDef = fragmentArgDef ?? varDefMap[varName];
           if (varDef && type) {
             // A var type is allowed if it is the same or more strict (e.g. is
             // a subtype of) than the expected type. It can be more strict if
diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts
index 60c967f8f0..0911474c50 100644
--- a/src/validation/specifiedRules.ts
+++ b/src/validation/specifiedRules.ts
@@ -29,6 +29,8 @@ import { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule.js';
 import { NoFragmentCyclesRule } from './rules/NoFragmentCyclesRule.js';
 // Spec Section: "All Variable Used Defined"
 import { NoUndefinedVariablesRule } from './rules/NoUndefinedVariablesRule.js';
+// Spec Section: "All Fragment Arguments Used"
+import { NoUnusedFragmentArgumentsRule } from './rules/NoUnusedFragmentArgumentsRule.js';
 // Spec Section: "Fragments must be used"
 import { NoUnusedFragmentsRule } from './rules/NoUnusedFragmentsRule.js';
 // Spec Section: "All Variables Used"
@@ -99,6 +101,7 @@ export const specifiedRules: ReadonlyArray<ValidationRule> = Object.freeze([
   UniqueVariableNamesRule,
   NoUndefinedVariablesRule,
   NoUnusedVariablesRule,
+  NoUnusedFragmentArgumentsRule,
   KnownDirectivesRule,
   UniqueDirectivesPerLocationRule,
   DeferStreamDirectiveOnRootFieldRule,

From 505d096cf6586dc9af0eee50217683d9dd619b53 Mon Sep 17 00:00:00 2001
From: Yaacov Rydzinski <yaacovCR@gmail.com>
Date: Fri, 6 Jan 2023 09:31:52 +0200
Subject: [PATCH 07/13] polish: clarify filtering test semantics (#3816)

`expect.fail()` was mistakenly introduced in #3760 under the assumption that expect.fail() will always cause the test to fail, and that `.next()` should be called at most once.

Actually, thought, `expect.fail()` just throws an error, `.next()` is called more than once, the error is produced and wrapped by GraphQL, but then it is filtered, so the test ultimately passes.

Fortunately, that's actually the desired behavior! It's ok if the number of calls to `.next()` is more than 1 (in our case it is 2). The exact number of calls to `.next()` depends on the # of ticks that it takes for the erroring deferred fragment to resolve, which should be as low as possible, but that is a separate objective (with other PRs in the mix already aimed at reducing).

So the test as written is intending to be overly strict, but is not actually meeting that goal and so functions as desires. This PR makes no change to the test semantics, but changes the comment, the coverage ignore statement, and removes the potentially confusing use of `expect.fail`, so that the test semantics are more clearly apparent.
---
 src/execution/__tests__/stream-test.ts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts
index aed5211ae1..cd9b9b3965 100644
--- a/src/execution/__tests__/stream-test.ts
+++ b/src/execution/__tests__/stream-test.ts
@@ -1,4 +1,4 @@
-import { assert, expect } from 'chai';
+import { assert } from 'chai';
 import { describe, it } from 'mocha';
 
 import { expectJSON } from '../../__testUtils__/expectJSON.js';
@@ -1422,9 +1422,8 @@ describe('Execute: stream directive', () => {
       [Symbol.asyncIterator]: () => ({
         next: () => {
           if (requested) {
-            /* c8 ignore next 3 */
-            // Not reached, iterator should end immediately.
-            expect.fail('Not reached');
+            // Ignores further errors when filtered.
+            return Promise.reject(new Error('Oops'));
           }
           requested = true;
           const friend = friends[0];
@@ -1438,6 +1437,7 @@ describe('Execute: stream directive', () => {
         },
         return: () => {
           returned = true;
+          // Ignores errors from return.
           return Promise.reject(new Error('Oops'));
         },
       }),

From 19dba20963126c9b30da3d2ca597af241cb5acca Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 14:20:02 -0500
Subject: [PATCH 08/13] Tests for fragment args

---
 src/execution/__tests__/variables-test.ts     | 227 ++++++++++++++++++
 src/language/__tests__/parser-test.ts         |  13 +-
 src/language/__tests__/printer-test.ts        |  40 ++-
 src/language/__tests__/visitor-test.ts        |  46 +++-
 .../NoUndefinedVariablesRule-test.ts          |  63 +++++
 .../__tests__/NoUnusedVariablesRule-test.ts   |  22 ++
 .../OverlappingFieldsCanBeMergedRule-test.ts  |  59 +++++
 .../ProvidedRequiredArgumentsRule-test.ts     | 108 +++++++++
 .../__tests__/ValuesOfCorrectTypeRule-test.ts |  31 +++
 .../VariablesAreInputTypesRule-test.ts        |  27 +++
 .../VariablesInAllowedPositionRule-test.ts    | 104 ++++++++
 11 files changed, 725 insertions(+), 15 deletions(-)

diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts
index 7c74d8ee92..8f3db65a21 100644
--- a/src/execution/__tests__/variables-test.ts
+++ b/src/execution/__tests__/variables-test.ts
@@ -82,6 +82,13 @@ function fieldWithInputArg(
   };
 }
 
+const NestedType: GraphQLObjectType = new GraphQLObjectType({
+  name: 'NestedType',
+  fields: {
+    echo: fieldWithInputArg({ type: GraphQLString }),
+  },
+});
+
 const TestType = new GraphQLObjectType({
   name: 'TestType',
   fields: {
@@ -107,6 +114,10 @@ const TestType = new GraphQLObjectType({
       defaultValue: 'Hello World',
     }),
     list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }),
+    nested: {
+      type: NestedType,
+      resolve: () => ({}),
+    },
     nnList: fieldWithInputArg({
       type: new GraphQLNonNull(new GraphQLList(GraphQLString)),
     }),
@@ -1006,6 +1017,222 @@ describe('Execute: Handles inputs', () => {
     });
   });
 
+  describe('using fragment arguments', () => {
+    it('when there are no fragment arguments', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+        fragment a on TestType {
+          fieldWithNonNullableStringInput(input: "A")
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when a value is required and provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "A")
+        }
+        fragment a($value: String!) on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when a value is required and not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+        fragment a($value: String!) on TestType {
+          fieldWithNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNullableStringInput: null,
+        },
+      });
+    });
+
+    it('when the definition has a default and is provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "A")
+        }
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"A"',
+        },
+      });
+    });
+
+    it('when the definition has a default and is not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNonNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInput: '"B"',
+        },
+      });
+    });
+
+    it('when the definition has a non-nullable default and is provided null', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: null)
+        }
+        fragment a($value: String! = "B") on TestType {
+          fieldWithNullableStringInput(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNullableStringInput: 'null',
+        },
+      });
+    });
+
+    it('when the definition has no default and is not provided', () => {
+      const result = executeQuery(`
+        query {
+          ...a
+        }
+        fragment a($value: String) on TestType {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $value)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue:
+            '"Hello World"',
+        },
+      });
+    });
+
+    it('when an argument is shadowed by an operation variable', () => {
+      const result = executeQuery(`
+        query($x: String! = "A") {
+          ...a(x: "B")
+        }
+        fragment a($x: String) on TestType {
+          fieldWithNullableStringInput(input: $x)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNullableStringInput: '"B"',
+        },
+      });
+    });
+
+    it('when a nullable argument with a field default is not provided and shadowed by an operation variable', () => {
+      const result = executeQuery(`
+        query($x: String = "A") {
+          ...a
+        }
+        fragment a($x: String) on TestType {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $x)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue:
+            '"Hello World"',
+        },
+      });
+    });
+
+    it('when a fragment is used with different args', () => {
+      const result = executeQuery(`
+        query($x: String = "Hello") {
+          a: nested {
+            ...a(x: "a")
+          }
+          b: nested {
+            ...a(x: "b", b: true)
+          }
+          hello: nested {
+            ...a(x: $x)
+          }
+        }
+        fragment a($x: String, $b: Boolean = false) on NestedType {
+          a: echo(input: $x) @skip(if: $b)
+          b: echo(input: $x) @include(if: $b)
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          a: {
+            a: '"a"',
+          },
+          b: {
+            b: '"b"',
+          },
+          hello: {
+            a: '"Hello"',
+          },
+        },
+      });
+    });
+
+    it('when the argument variable is nested in a complex type', () => {
+      const result = executeQuery(`
+        query {
+          ...a(value: "C")
+        }
+        fragment a($value: String) on TestType {
+          list(input: ["A", "B", $value, "D"])
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          list: '["A", "B", "C", "D"]',
+        },
+      });
+    });
+
+    it('when argument variables are used recursively', () => {
+      const result = executeQuery(`
+        query {
+          ...a(aValue: "C")
+        }
+        fragment a($aValue: String) on TestType {
+          ...b(bValue: $aValue)
+        }
+        fragment b($bValue: String) on TestType {
+          list(input: ["A", "B", $bValue, "D"])
+        }
+      `);
+      expect(result).to.deep.equal({
+        data: {
+          list: '["A", "B", "C", "D"]',
+        },
+      });
+    });
+  });
+
   describe('getVariableValues: limit maximum number of coercion errors', () => {
     const doc = parse(`
       query ($input: [String!]) {
diff --git a/src/language/__tests__/parser-test.ts b/src/language/__tests__/parser-test.ts
index 34e9dff4b9..687180846e 100644
--- a/src/language/__tests__/parser-test.ts
+++ b/src/language/__tests__/parser-test.ts
@@ -607,13 +607,16 @@ describe('Parser', () => {
     expect('loc' in result).to.equal(false);
   });
 
-  it('Legacy: allows parsing fragment defined variables', () => {
+  it('allows parsing fragment defined arguments', () => {
     const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';
 
-    expect(() =>
-      parse(document, { allowLegacyFragmentVariables: true }),
-    ).to.not.throw();
-    expect(() => parse(document)).to.throw('Syntax Error');
+    expect(() => parse(document)).to.not.throw();
+  });
+
+  it('allows parsing fragment spread arguments', () => {
+    const document = 'fragment a on t { ...b(v: $v) }';
+
+    expect(() => parse(document)).to.not.throw();
   });
 
   it('contains location that can be Object.toStringified, JSON.stringified, or jsutils.inspected', () => {
diff --git a/src/language/__tests__/printer-test.ts b/src/language/__tests__/printer-test.ts
index 0585cae6d9..94152fe079 100644
--- a/src/language/__tests__/printer-test.ts
+++ b/src/language/__tests__/printer-test.ts
@@ -110,34 +110,58 @@ describe('Printer: Query document', () => {
     `);
   });
 
-  it('Legacy: prints fragment with variable directives', () => {
-    const queryASTWithVariableDirective = parse(
+  it('prints fragment with argument definition directives', () => {
+    const fragmentWithArgumentDefinitionDirective = parse(
       'fragment Foo($foo: TestType @test) on TestType @testDirective { id }',
-      { allowLegacyFragmentVariables: true },
     );
-    expect(print(queryASTWithVariableDirective)).to.equal(dedent`
+    expect(print(fragmentWithArgumentDefinitionDirective)).to.equal(dedent`
       fragment Foo($foo: TestType @test) on TestType @testDirective {
         id
       }
     `);
   });
 
-  it('Legacy: correctly prints fragment defined variables', () => {
-    const fragmentWithVariable = parse(
+  it('correctly prints fragment defined arguments', () => {
+    const fragmentWithArgumentDefinition = parse(
       `
         fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
           id
         }
       `,
-      { allowLegacyFragmentVariables: true },
     );
-    expect(print(fragmentWithVariable)).to.equal(dedent`
+    expect(print(fragmentWithArgumentDefinition)).to.equal(dedent`
       fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
         id
       }
     `);
   });
 
+  it('prints fragment spread with arguments', () => {
+    const fragmentSpreadWithArguments = parse(
+      'fragment Foo on TestType { ...Bar(a: {x: $x}, b: true) }',
+    );
+    expect(print(fragmentSpreadWithArguments)).to.equal(dedent`
+      fragment Foo on TestType {
+        ...Bar(a: { x: $x }, b: true)
+      }
+    `);
+  });
+
+  it('prints fragment spread with multi-line arguments', () => {
+    const fragmentSpreadWithArguments = 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") }',
+    );
+    expect(print(fragmentSpreadWithArguments)).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
index 270c948225..7ee17a13a7 100644
--- a/src/language/__tests__/visitor-test.ts
+++ b/src/language/__tests__/visitor-test.ts
@@ -455,10 +455,9 @@ describe('Visitor', () => {
     ]);
   });
 
-  it('Legacy: visits variables defined in fragments', () => {
+  it('visits arguments defined on fragments', () => {
     const ast = parse('fragment a($v: Boolean = false) on t { f }', {
       noLocation: true,
-      allowLegacyFragmentVariables: true,
     });
     const visited: Array<any> = [];
 
@@ -505,6 +504,49 @@ describe('Visitor', () => {
     ]);
   });
 
+  it('visits arguments on fragment spreads', () => {
+    const ast = parse('fragment a on t { ...s(v: false) }', {
+      noLocation: 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('n', () => {
     const ast = parse(kitchenSinkQuery, {
       experimentalClientControlledNullability: true,
diff --git a/src/validation/__tests__/NoUndefinedVariablesRule-test.ts b/src/validation/__tests__/NoUndefinedVariablesRule-test.ts
index c6ed758cad..9d53ea8c2e 100644
--- a/src/validation/__tests__/NoUndefinedVariablesRule-test.ts
+++ b/src/validation/__tests__/NoUndefinedVariablesRule-test.ts
@@ -404,4 +404,67 @@ describe('Validate: No undefined variables', () => {
       },
     ]);
   });
+
+  it('fragment defined arguments are not undefined variables', () => {
+    expectValid(`
+      query Foo {
+        ...FragA
+      }
+      fragment FragA($a: String) on Type {
+        field1(a: $a)
+      }
+    `);
+  });
+
+  it('defined variables used as fragment arguments are not undefined variables', () => {
+    expectValid(`
+      query Foo($b: String) {
+        ...FragA(a: $b)
+      }
+      fragment FragA($a: String) on Type {
+        field1
+      }
+    `);
+  });
+
+  it('variables used as fragment arguments may be undefined variables', () => {
+    expectErrors(`
+      query Foo {
+        ...FragA(a: $a)
+      }
+      fragment FragA($a: String) on Type {
+        field1
+      }
+    `).toDeepEqual([
+      {
+        message: 'Variable "$a" is not defined by operation "Foo".',
+        locations: [
+          { line: 3, column: 21 },
+          { line: 2, column: 7 },
+        ],
+      },
+    ]);
+  });
+
+  it('variables shadowed by parent fragment arguments are still undefined variables', () => {
+    expectErrors(`
+      query Foo {
+        ...FragA
+      }
+      fragment FragA($a: String) on Type {
+        ...FragB
+      }
+      fragment FragB on Type {
+        field1(a: $a)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Variable "$a" is not defined by operation "Foo".',
+        locations: [
+          { line: 9, column: 19 },
+          { line: 2, column: 7 },
+        ],
+      },
+    ]);
+  });
 });
diff --git a/src/validation/__tests__/NoUnusedVariablesRule-test.ts b/src/validation/__tests__/NoUnusedVariablesRule-test.ts
index 47dac39c99..d08d547931 100644
--- a/src/validation/__tests__/NoUnusedVariablesRule-test.ts
+++ b/src/validation/__tests__/NoUnusedVariablesRule-test.ts
@@ -230,4 +230,26 @@ describe('Validate: No unused variables', () => {
       },
     ]);
   });
+
+  it('fragment defined arguments are not unused variables', () => {
+    expectValid(`
+      query Foo {
+        ...FragA
+      }
+      fragment FragA($a: String) on Type {
+        field1(a: $a)
+      }
+    `);
+  });
+
+  it('defined variables used as fragment arguments are not unused variables', () => {
+    expectValid(`
+      query Foo($b: String) {
+        ...FragA(a: $b)
+      }
+      fragment FragA($a: String) on Type {
+        field1
+      }
+    `);
+  });
 });
diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
index 52c2deb1a0..e395df52c8 100644
--- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
+++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
@@ -1246,4 +1246,63 @@ describe('Validate: Overlapping fields can be merged', () => {
       },
     ]);
   });
+
+  describe('fragment arguments must produce fields that can be merged', () => {
+    it('encounters conflict in fragments', () => {
+      expectErrors(`
+        {
+          ...WithArgs(x: 3)
+          ...WithArgs(x: 4)
+        }
+        fragment WithArgs($x: Int) on Type {
+          a(x: $x)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Spreads "WithArgs" conflict because WithArgs(x: 3) and WithArgs(x: 4) have different fragment arguments.',
+          locations: [
+            { line: 3, column: 11 },
+            { line: 4, column: 11 },
+          ],
+        },
+      ]);
+    });
+    it('encounters nested conflict in fragments', () => {
+      expectErrors(`
+        {
+          connection {
+            edges {
+              ...WithArgs(x: 3)
+            }
+          }
+          ...Connection
+        }
+
+        fragment Connection on Type {
+          connection {
+            edges {
+              ...WithArgs(x: 4)
+            }
+          }
+        }
+        fragment WithArgs($x: Int) on Type {
+          a(x: $x)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Fields "connection" conflict because subfields "edges" conflict because child spreads "WithArgs" conflict because WithArgs(x: 3) and WithArgs(x: 4) have different fragment arguments. Use different aliases on the fields to fetch both if this was intentional.',
+          locations: [
+            { line: 3, column: 11 },
+            { line: 4, column: 13 },
+            { line: 5, column: 15 },
+            { line: 12, column: 11 },
+            { line: 13, column: 13 },
+            { line: 14, column: 15 },
+          ],
+        },
+      ]);
+    });
+  });
 });
diff --git a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts
index 6f0d223c15..66545ddb59 100644
--- a/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts
+++ b/src/validation/__tests__/ProvidedRequiredArgumentsRule-test.ts
@@ -356,4 +356,112 @@ describe('Validate: Provided required arguments', () => {
       ]);
     });
   });
+
+  describe('Fragment required arguments', () => {
+    it('ignores unknown arguments', () => {
+      expectValid(`
+        {
+          ...Foo(unknownArgument: true)
+        }
+        fragment Foo on Query {
+          dog
+        }
+      `);
+    });
+
+    // Query: should this be allowed?
+    // We could differentiate between required/optional (i.e. no default value)
+    // vs. nullable/non-nullable (i.e. no !), whereas now they are conflated.
+    // So today:
+    // $x: Int!     `x:` is required and must not be null (NOT a nullable variable)
+    // $x: Int! = 3 `x:` is not required and must not be null (MAY BE a nullable variable)
+    // $x: Int      `x:` is not required and may be null
+    // $x: Int = 3  `x:` is not required and may be null
+    //
+    // It feels weird to collapse the nullable cases but not the non-nullable ones.
+    // Whereas all four feel like they ought to mean something explicitly different.
+    //
+    // Potential proposal:
+    // $x: Int!     `x:` is required and must not be null (NOT a nullable variable)
+    // $x: Int! = 3 `x:` is not required and must not be null (NOT a nullable variable)
+    // $x: Int      `x:` is required and may be null
+    // $x: Int = 3  `x:` is not required and may be null
+    //
+    // Required then is whether there's a default value,
+    // and nullable is whether there's a !
+    it('Missing nullable argument with default is allowed', () => {
+      expectValid(`
+          {
+            ...F
+            
+          }
+          fragment F($x: Int = 3) on Query {
+            foo
+          }
+        `);
+    });
+    // Above proposal: this should be an error
+    it('Missing nullable argument is allowed', () => {
+      expectValid(`
+          {
+            ...F
+            
+          }
+          fragment F($x: Int) on Query {
+            foo
+          }
+        `);
+    });
+    it('Missing non-nullable argument with default is allowed', () => {
+      expectValid(`
+          {
+            ...F
+            
+          }
+          fragment F($x: Int! = 3) on Query {
+            foo
+          }
+        `);
+    });
+    it('Missing non-nullable argument is not allowed', () => {
+      expectErrors(`
+          {
+            ...F
+            
+          }
+          fragment F($x: Int!) on Query {
+            foo
+          }
+        `).toDeepEqual([
+        {
+          message:
+            'Fragment "F" argument "x" of type "{ kind: "NonNullType", type: { kind: "NamedType", name: [Object], loc: [Object] }, loc: [Object] }" is required, but it was not provided.',
+          locations: [
+            { line: 3, column: 13 },
+            { line: 6, column: 22 },
+          ],
+        },
+      ]);
+    });
+
+    it('Supplies required variables', () => {
+      expectValid(`
+          {
+            ...F(x: 3)
+            
+          }
+          fragment F($x: Int!) on Query {
+            foo
+          }
+        `);
+    });
+
+    it('Skips missing fragments', () => {
+      expectValid(`
+          {
+            ...Missing(x: 3)
+          }
+        `);
+    });
+  });
 });
diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts
index f0b7dfa57e..d843139d89 100644
--- a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts
+++ b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts
@@ -1198,4 +1198,35 @@ describe('Validate: Values of correct type', () => {
       ]);
     });
   });
+
+  describe('Fragment argument values', () => {
+    it('list variables with invalid item', () => {
+      expectErrors(`
+        fragment InvalidItem($a: [String] = ["one", 2]) on Query {
+          dog { name }
+        }
+      `).toDeepEqual([
+        {
+          message: 'String cannot represent a non string value: 2',
+          locations: [{ line: 2, column: 53 }],
+        },
+      ]);
+    });
+
+    it('fragment spread with invalid argument value', () => {
+      expectErrors(`
+        fragment GivesString on Query {
+          ...ExpectsInt(a: "three")
+        }
+        fragment ExpectsInt($a: Int) on Query {
+          dog { name }
+        }
+      `).toDeepEqual([
+        {
+          message: 'Int cannot represent non-integer value: "three"',
+          locations: [{ line: 3, column: 28 }],
+        },
+      ]);
+    });
+  });
 });
diff --git a/src/validation/__tests__/VariablesAreInputTypesRule-test.ts b/src/validation/__tests__/VariablesAreInputTypesRule-test.ts
index 8027a35826..eddd202df8 100644
--- a/src/validation/__tests__/VariablesAreInputTypesRule-test.ts
+++ b/src/validation/__tests__/VariablesAreInputTypesRule-test.ts
@@ -18,6 +18,9 @@ describe('Validate: Variables are input types', () => {
       query Foo($a: Unknown, $b: [[Unknown!]]!) {
         field(a: $a, b: $b)
       }
+      fragment Bar($a: Unknown, $b: [[Unknown!]]!) on Query {
+        field(a: $a, b: $b)
+      }
     `);
   });
 
@@ -26,6 +29,9 @@ describe('Validate: Variables are input types', () => {
       query Foo($a: String, $b: [Boolean!]!, $c: ComplexInput) {
         field(a: $a, b: $b, c: $c)
       }
+      fragment Bar($a: String, $b: [Boolean!]!, $c: ComplexInput) on Query {
+        field(a: $a, b: $b, c: $c)
+      }
     `);
   });
 
@@ -49,4 +55,25 @@ describe('Validate: Variables are input types', () => {
       },
     ]);
   });
+
+  it('output types on fragment arguments are invalid', () => {
+    expectErrors(`
+      fragment Bar($a: Dog, $b: [[CatOrDog!]]!, $c: Pet) on Query {
+        field(a: $a, b: $b, c: $c)
+      }
+    `).toDeepEqual([
+      {
+        locations: [{ line: 2, column: 24 }],
+        message: 'Variable "$a" cannot be non-input type "Dog".',
+      },
+      {
+        locations: [{ line: 2, column: 33 }],
+        message: 'Variable "$b" cannot be non-input type "[[CatOrDog!]]!".',
+      },
+      {
+        locations: [{ line: 2, column: 53 }],
+        message: 'Variable "$c" cannot be non-input type "Pet".',
+      },
+    ]);
+  });
 });
diff --git a/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts b/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts
index 16467741bb..d0da8f5305 100644
--- a/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts
+++ b/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts
@@ -356,5 +356,109 @@ describe('Validate: Variables are in allowed positions', () => {
           dog @include(if: $boolVar)
         }`);
     });
+
+    it('undefined in directive with default value with option', () => {
+      expectValid(`
+        {
+          dog @include(if: $x)
+        }`);
+    });
+  });
+
+  describe('Fragment arguments are validated', () => {
+    it('Boolean => Boolean', () => {
+      expectValid(`
+        query Query($booleanArg: Boolean)
+        {
+          complicatedArgs {
+            ...A(b: $booleanArg)
+          }
+        }
+        fragment A($b: Boolean) on ComplicatedArgs {
+          booleanArgField(booleanArg: $b)
+        }
+      `);
+    });
+
+    it('Boolean => Boolean with default value', () => {
+      expectValid(`
+        query Query($booleanArg: Boolean)
+        {
+          complicatedArgs {
+            ...A(b: $booleanArg)
+          }
+        }
+        fragment A($b: Boolean = true) on ComplicatedArgs {
+          booleanArgField(booleanArg: $b)
+        }
+      `);
+    });
+
+    it('Boolean => Boolean!', () => {
+      expectErrors(`
+        query Query($ab: Boolean)
+        {
+          complicatedArgs {
+            ...A(b: $ab)
+          }
+        }
+        fragment A($b: Boolean!) on ComplicatedArgs {
+          booleanArgField(booleanArg: $b)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Variable "$ab" of type "Boolean" used in position expecting type "Boolean!".',
+          locations: [
+            { line: 2, column: 21 },
+            { line: 5, column: 21 },
+          ],
+        },
+      ]);
+    });
+
+    it('Int => Int! fails when variable provides null default value', () => {
+      expectErrors(`
+        query Query($intVar: Int = null) {
+          complicatedArgs {
+            ...A(i: $intVar)
+          }
+        }
+        fragment A($i: Int!) on ComplicatedArgs {
+          nonNullIntArgField(nonNullIntArg: $i)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Variable "$intVar" of type "Int" used in position expecting type "Int!".',
+          locations: [
+            { line: 2, column: 21 },
+            { line: 4, column: 21 },
+          ],
+        },
+      ]);
+    });
+
+    it('Int fragment arg => Int! field arg fails even when shadowed by Int! variable', () => {
+      expectErrors(`
+        query Query($intVar: Int!) {
+          complicatedArgs {
+            ...A(i: $intVar)
+          }
+        }
+        fragment A($intVar: Int) on ComplicatedArgs {
+          nonNullIntArgField(nonNullIntArg: $intVar)
+        }
+      `).toDeepEqual([
+        {
+          message:
+            'Variable "$intVar" of type "Int" used in position expecting type "Int!".',
+          locations: [
+            { line: 7, column: 20 },
+            { line: 8, column: 45 },
+          ],
+        },
+      ]);
+    });
   });
 });

From a91fba8aac76db9e237e3d5e69da718aefd42bbf Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 15:09:48 -0500
Subject: [PATCH 09/13] Make FragmentArgumentDefinition a unique AST node to
 see downstream effects

---
 src/language/__tests__/visitor-test.ts |  4 +-
 src/language/ast.ts                    | 29 +++++++--
 src/language/kinds.ts                  |  1 +
 src/language/parser.ts                 | 89 ++++++++++++++++----------
 src/language/printer.ts                | 28 ++++++--
 src/utilities/buildASTSchema.ts        |  1 -
 6 files changed, 106 insertions(+), 46 deletions(-)

diff --git a/src/language/__tests__/visitor-test.ts b/src/language/__tests__/visitor-test.ts
index 7ee17a13a7..b960fe5109 100644
--- a/src/language/__tests__/visitor-test.ts
+++ b/src/language/__tests__/visitor-test.ts
@@ -477,7 +477,7 @@ describe('Visitor', () => {
       ['enter', 'FragmentDefinition', undefined],
       ['enter', 'Name', 'a'],
       ['leave', 'Name', 'a'],
-      ['enter', 'VariableDefinition', undefined],
+      ['enter', 'FragmentArgumentDefinition', undefined],
       ['enter', 'Variable', undefined],
       ['enter', 'Name', 'v'],
       ['leave', 'Name', 'v'],
@@ -488,7 +488,7 @@ describe('Visitor', () => {
       ['leave', 'NamedType', undefined],
       ['enter', 'BooleanValue', false],
       ['leave', 'BooleanValue', false],
-      ['leave', 'VariableDefinition', undefined],
+      ['leave', 'FragmentArgumentDefinition', undefined],
       ['enter', 'NamedType', undefined],
       ['enter', 'Name', 't'],
       ['leave', 'Name', 't'],
diff --git a/src/language/ast.ts b/src/language/ast.ts
index 22a7cc253c..d06d15c395 100644
--- a/src/language/ast.ts
+++ b/src/language/ast.ts
@@ -149,6 +149,7 @@ export type ASTNode =
   | FragmentSpreadNode
   | InlineFragmentNode
   | FragmentDefinitionNode
+  | FragmentArgumentDefinitionNode
   | IntValueNode
   | FloatValueNode
   | StringValueNode
@@ -227,16 +228,22 @@ export const QueryDocumentKeys: {
   NonNullAssertion: ['nullabilityAssertion'],
   ErrorBoundary: ['nullabilityAssertion'],
 
-  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',
+    'arguments',
     'typeCondition',
     'directives',
     'selectionSet',
   ],
+  FragmentArgumentDefinition: [
+    'description',
+    'variable',
+    'type',
+    'defaultValue',
+    'directives',
+  ],
 
   IntValue: [],
   FloatValue: [],
@@ -428,6 +435,7 @@ export interface FragmentSpreadNode {
   readonly kind: Kind.FRAGMENT_SPREAD;
   readonly loc?: Location | undefined;
   readonly name: NameNode;
+  readonly arguments?: ReadonlyArray<ArgumentNode> | undefined;
   readonly directives?: ReadonlyArray<DirectiveNode> | undefined;
 }
 
@@ -443,15 +451,24 @@ export interface FragmentDefinitionNode {
   readonly kind: Kind.FRAGMENT_DEFINITION;
   readonly loc?: Location | undefined;
   readonly name: NameNode;
-  /** @deprecated variableDefinitions will be removed in v17.0.0 */
-  readonly variableDefinitions?:
-    | ReadonlyArray<VariableDefinitionNode>
+  readonly arguments?:
+    | ReadonlyArray<FragmentArgumentDefinitionNode>
     | undefined;
   readonly typeCondition: NamedTypeNode;
   readonly directives?: ReadonlyArray<DirectiveNode> | undefined;
   readonly selectionSet: SelectionSetNode;
 }
 
+export interface FragmentArgumentDefinitionNode {
+  readonly kind: Kind.FRAGMENT_ARGUMENT_DEFINITION;
+  readonly loc?: Location | undefined;
+  readonly description?: StringValueNode | undefined;
+  readonly variable: VariableNode;
+  readonly type: TypeNode;
+  readonly defaultValue?: ConstValueNode | undefined;
+  readonly directives?: ReadonlyArray<ConstDirectiveNode> | undefined;
+}
+
 /** Values */
 
 export type ValueNode =
diff --git a/src/language/kinds.ts b/src/language/kinds.ts
index d606c12cbe..8d9098ed4e 100644
--- a/src/language/kinds.ts
+++ b/src/language/kinds.ts
@@ -22,6 +22,7 @@ export enum Kind {
   FRAGMENT_SPREAD = 'FragmentSpread',
   INLINE_FRAGMENT = 'InlineFragment',
   FRAGMENT_DEFINITION = 'FragmentDefinition',
+  FRAGMENT_ARGUMENT_DEFINITION = 'FragmentArgumentDefinition',
 
   /** Values */
   VARIABLE = 'Variable',
diff --git a/src/language/parser.ts b/src/language/parser.ts
index bc58875e9d..6cad241033 100644
--- a/src/language/parser.ts
+++ b/src/language/parser.ts
@@ -24,6 +24,7 @@ import type {
   FieldDefinitionNode,
   FieldNode,
   FloatValueNode,
+  FragmentArgumentDefinitionNode,
   FragmentDefinitionNode,
   FragmentSpreadNode,
   InlineFragmentNode,
@@ -91,23 +92,6 @@ export interface ParseOptions {
    */
   maxTokens?: number | undefined;
 
-  /**
-   * @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.
-   *
-   * The syntax is identical to normal, query-defined variables. For example:
-   *
-   * ```graphql
-   * fragment A($var: Boolean = false) on T {
-   *   ...
-   * }
-   * ```
-   */
-  allowLegacyFragmentVariables?: boolean | undefined;
-
   /**
    * EXPERIMENTAL:
    *
@@ -550,7 +534,7 @@ export class Parser {
   /**
    * Corresponds to both FragmentSpread and InlineFragment in the spec.
    *
-   * FragmentSpread : ... FragmentName Directives?
+   * FragmentSpread : ... FragmentName Arguments? Directives?
    *
    * InlineFragment : ... TypeCondition? Directives? SelectionSet
    */
@@ -560,9 +544,18 @@ export class Parser {
 
     const hasTypeCondition = this.expectOptionalKeyword('on');
     if (!hasTypeCondition && this.peek(TokenKind.NAME)) {
+      const name = this.parseFragmentName();
+      if (this.peek(TokenKind.PAREN_L)) {
+        return this.node<FragmentSpreadNode>(start, {
+          kind: Kind.FRAGMENT_SPREAD,
+          name,
+          arguments: this.parseArguments(false),
+          directives: this.parseDirectives(false),
+        });
+      }
       return this.node<FragmentSpreadNode>(start, {
         kind: Kind.FRAGMENT_SPREAD,
-        name: this.parseFragmentName(),
+        name,
         directives: this.parseDirectives(false),
       });
     }
@@ -576,29 +569,17 @@ export class Parser {
 
   /**
    * FragmentDefinition :
-   *   - fragment FragmentName on TypeCondition Directives? SelectionSet
+   *   - fragment FragmentName FragmentArgumentsDefinition? on TypeCondition Directives? SelectionSet
    *
    * TypeCondition : NamedType
    */
   parseFragmentDefinition(): FragmentDefinitionNode {
     const start = this._lexer.token;
     this.expectKeyword('fragment');
-    // Legacy support for defining variables within fragments changes
-    // the grammar of FragmentDefinition:
-    //   - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet
-    if (this._options.allowLegacyFragmentVariables === true) {
-      return this.node<FragmentDefinitionNode>(start, {
-        kind: Kind.FRAGMENT_DEFINITION,
-        name: this.parseFragmentName(),
-        variableDefinitions: this.parseVariableDefinitions(),
-        typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
-        directives: this.parseDirectives(false),
-        selectionSet: this.parseSelectionSet(),
-      });
-    }
     return this.node<FragmentDefinitionNode>(start, {
       kind: Kind.FRAGMENT_DEFINITION,
       name: this.parseFragmentName(),
+      arguments: this.parseFragmentArgumentDefs(),
       typeCondition: (this.expectKeyword('on'), this.parseNamedType()),
       directives: this.parseDirectives(false),
       selectionSet: this.parseSelectionSet(),
@@ -615,6 +596,48 @@ export class Parser {
     return this.parseName();
   }
 
+  /**
+   * FragmentArgumentsDefinition : ( FragmentArgumentDefinition+ )
+   */
+  parseFragmentArgumentDefs(): Array<FragmentArgumentDefinitionNode> {
+    return this.optionalMany(
+      TokenKind.PAREN_L,
+      this.parseFragmentArgumentDef,
+      TokenKind.PAREN_R,
+    );
+  }
+
+  /**
+   * FragmentArgumentDefinition :
+   *   - Description? Variable : Type DefaultValue? Directives[Const]?
+   *
+   * Note: identical to InputValueDefinition, EXCEPT Name always begins
+   * with $, so we need to parse a Variable out instead of a plain Name.
+   *
+   * Note: identical to VariableDefinition, EXCEPT we allow Description.
+   * Fragments are re-used, and their arguments may need documentation.
+   */
+  parseFragmentArgumentDef(): FragmentArgumentDefinitionNode {
+    const start = this._lexer.token;
+    const description = this.parseDescription();
+    const variable = this.parseVariable();
+    this.expectToken(TokenKind.COLON);
+    const type = this.parseTypeReference();
+    let defaultValue;
+    if (this.expectOptionalToken(TokenKind.EQUALS)) {
+      defaultValue = this.parseConstValueLiteral();
+    }
+    const directives = this.parseConstDirectives();
+    return this.node<FragmentArgumentDefinitionNode>(start, {
+      kind: Kind.FRAGMENT_ARGUMENT_DEFINITION,
+      description,
+      variable,
+      type,
+      defaultValue,
+      directives,
+    });
+  }
+
   // Implements the parsing rules in the Values section.
 
   /**
diff --git a/src/language/printer.ts b/src/language/printer.ts
index a07decc11d..7de5916322 100644
--- a/src/language/printer.ts
+++ b/src/language/printer.ts
@@ -105,8 +105,15 @@ 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: {
@@ -126,17 +133,30 @@ const printDocASTReducer: ASTReducer<string> = {
     leave: ({
       name,
       typeCondition,
-      variableDefinitions,
+      arguments: args,
       directives,
       selectionSet,
     }) =>
       // Note: fragment variable definitions are experimental and may be changed
       // or removed in the future.
-      `fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` +
+      `fragment ${name}${wrap('(', join(args, ', '), ')')} ` +
       `on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` +
       selectionSet,
   },
 
+  FragmentArgumentDefinition: {
+    leave: ({ description, variable, type, defaultValue, directives }) =>
+      wrap('', description, '\n') +
+      join(
+        [
+          variable + ': ' + type,
+          wrap('= ', defaultValue),
+          join(directives, ' '),
+        ],
+        ' ',
+      ),
+  },
+
   // Value
 
   IntValue: { leave: ({ value }) => value },
diff --git a/src/utilities/buildASTSchema.ts b/src/utilities/buildASTSchema.ts
index 09fd7cd7dd..c9728f56c3 100644
--- a/src/utilities/buildASTSchema.ts
+++ b/src/utilities/buildASTSchema.ts
@@ -93,7 +93,6 @@ export function buildSchema(
 ): GraphQLSchema {
   const document = parse(source, {
     noLocation: options?.noLocation,
-    allowLegacyFragmentVariables: options?.allowLegacyFragmentVariables,
   });
 
   return buildASTSchema(document, {

From 87498db5bbc811ff01ba31bab72997998881e819 Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 15:18:30 -0500
Subject: [PATCH 10/13] Execution works with new AST node

---
 src/execution/collectFields.ts               | 28 ++++---
 src/utilities/keyForFragmentSpread.ts        | 23 ++++++
 src/utilities/substituteFragmentArguments.ts | 78 ++++++++++++++++++++
 3 files changed, 118 insertions(+), 11 deletions(-)
 create mode 100644 src/utilities/keyForFragmentSpread.ts
 create mode 100644 src/utilities/substituteFragmentArguments.ts

diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts
index 17468b791f..a5634023ad 100644
--- a/src/execution/collectFields.ts
+++ b/src/execution/collectFields.ts
@@ -22,6 +22,8 @@ import {
 } from '../type/directives.js';
 import type { GraphQLSchema } from '../type/schema.js';
 
+import { keyForFragmentSpread } from '../utilities/keyForFragmentSpread.js';
+import { substituteFragmentArguments } from '../utilities/substituteFragmentArguments.js';
 import { typeFromAST } from '../utilities/typeFromAST.js';
 
 import { getDirectiveValues } from './values.js';
@@ -124,7 +126,7 @@ function collectFieldsImpl(
   selectionSet: SelectionSetNode,
   fields: AccumulatorMap<string, FieldNode>,
   patches: Array<PatchFields>,
-  visitedFragmentNames: Set<string>,
+  visitedFragmentKeys: Set<string>,
 ): void {
   for (const selection of selectionSet.selections) {
     switch (selection.kind) {
@@ -156,7 +158,7 @@ function collectFieldsImpl(
             selection.selectionSet,
             patchFields,
             patches,
-            visitedFragmentNames,
+            visitedFragmentKeys,
           );
           patches.push({
             label: defer.label,
@@ -172,24 +174,24 @@ function collectFieldsImpl(
             selection.selectionSet,
             fields,
             patches,
-            visitedFragmentNames,
+            visitedFragmentKeys,
           );
         }
         break;
       }
       case Kind.FRAGMENT_SPREAD: {
-        const fragName = selection.name.value;
+        const fragmentKey = keyForFragmentSpread(selection);
 
         if (!shouldIncludeNode(variableValues, selection)) {
           continue;
         }
 
         const defer = getDeferValues(operation, variableValues, selection);
-        if (visitedFragmentNames.has(fragName) && !defer) {
+        if (visitedFragmentKeys.has(fragmentKey) && !defer) {
           continue;
         }
 
-        const fragment = fragments[fragName];
+        const fragment = fragments[selection.name.value];
         if (
           !fragment ||
           !doesFragmentConditionMatch(schema, fragment, runtimeType)
@@ -198,9 +200,13 @@ function collectFieldsImpl(
         }
 
         if (!defer) {
-          visitedFragmentNames.add(fragName);
+          visitedFragmentKeys.add(fragmentKey);
         }
 
+        const fragmentSelectionSet = substituteFragmentArguments(
+          fragment,
+          selection,
+        );
         if (defer) {
           const patchFields = new AccumulatorMap<string, FieldNode>();
           collectFieldsImpl(
@@ -209,10 +215,10 @@ function collectFieldsImpl(
             variableValues,
             operation,
             runtimeType,
-            fragment.selectionSet,
+            fragmentSelectionSet,
             patchFields,
             patches,
-            visitedFragmentNames,
+            visitedFragmentKeys,
           );
           patches.push({
             label: defer.label,
@@ -225,10 +231,10 @@ function collectFieldsImpl(
             variableValues,
             operation,
             runtimeType,
-            fragment.selectionSet,
+            fragmentSelectionSet,
             fields,
             patches,
-            visitedFragmentNames,
+            visitedFragmentKeys,
           );
         }
         break;
diff --git a/src/utilities/keyForFragmentSpread.ts b/src/utilities/keyForFragmentSpread.ts
new file mode 100644
index 0000000000..95b2d9e6cc
--- /dev/null
+++ b/src/utilities/keyForFragmentSpread.ts
@@ -0,0 +1,23 @@
+import type { FragmentSpreadNode } from '../language/ast.js';
+import { print } from '../language/printer.js';
+
+/**
+ * Create a key that uniquely identifies common fragment spreads.
+ * Treats the fragment spread as the source of truth for the key: it
+ * does not bother to look up the argument definitions to de-duplicate default-variable args.
+ *
+ * Using the fragment definition to more accurately de-duplicate common spreads
+ * is a potential performance win, but in practice it seems unlikely to be common.
+ */
+export function keyForFragmentSpread(fragmentSpread: FragmentSpreadNode) {
+  const fragmentName = fragmentSpread.name.value;
+  const fragmentArguments = fragmentSpread.arguments;
+  if (fragmentArguments == null || fragmentArguments.length === 0) {
+    return fragmentName;
+  }
+
+  const printedArguments: Array<string> = fragmentArguments
+    .map(print)
+    .sort((a, b) => a.localeCompare(b));
+  return fragmentName + '(' + printedArguments.join(',') + ')';
+}
diff --git a/src/utilities/substituteFragmentArguments.ts b/src/utilities/substituteFragmentArguments.ts
new file mode 100644
index 0000000000..504ae6e8d3
--- /dev/null
+++ b/src/utilities/substituteFragmentArguments.ts
@@ -0,0 +1,78 @@
+import type { Maybe } from '../jsutils/Maybe.js';
+import type { ObjMap } from '../jsutils/ObjMap.js';
+
+import type {
+  ArgumentNode,
+  FragmentArgumentDefinitionNode,
+  FragmentDefinitionNode,
+  FragmentSpreadNode,
+  SelectionSetNode,
+  ValueNode,
+} from '../language/ast.js';
+import { Kind } from '../language/kinds.js';
+import { visit } from '../language/visitor.js';
+
+/**
+ * Replaces all fragment argument values with non-fragment-scoped values.
+ *
+ * NOTE: fragment arguments are scoped to the fragment they're defined on.
+ * Therefore, after we apply the passed-in arguments, all remaining variables
+ * must be either operation defined variables or explicitly unset.
+ */
+export function substituteFragmentArguments(
+  def: FragmentDefinitionNode,
+  fragmentSpread: FragmentSpreadNode,
+): SelectionSetNode {
+  const argumentDefinitions = def.arguments;
+  if (argumentDefinitions == null || argumentDefinitions.length === 0) {
+    return def.selectionSet;
+  }
+  const argumentValues = fragmentArgumentSubstitutions(
+    argumentDefinitions,
+    fragmentSpread.arguments,
+  );
+  return visit(def.selectionSet, {
+    Variable(node) {
+      return argumentValues[node.name.value];
+    },
+  });
+}
+
+export function fragmentArgumentSubstitutions(
+  argumentDefinitions: ReadonlyArray<FragmentArgumentDefinitionNode>,
+  argumentValues: Maybe<ReadonlyArray<ArgumentNode>>,
+): ObjMap<ValueNode> {
+  const substitutions: ObjMap<ValueNode> = {};
+  if (argumentValues) {
+    for (const argument of argumentValues) {
+      substitutions[argument.name.value] = argument.value;
+    }
+  }
+
+  for (const argumentDefinition of argumentDefinitions) {
+    const argumentName = argumentDefinition.variable.name.value;
+    if (substitutions[argumentName]) {
+      continue;
+    }
+
+    const defaultValue = argumentDefinition.defaultValue;
+    if (defaultValue) {
+      substitutions[argumentName] = defaultValue;
+    } else {
+      // We need a way to allow unset arguments without accidentally
+      // replacing an unset fragment argument with an operation
+      // variable value. Fragment arguments must always have LOCAL scope.
+      //
+      // To remove this hack, we need to either:
+      //    - include fragment argument scope when evaluating fields
+      //    - make unset fragment arguments invalid
+      // Requiring the spread to pass all non-default-defined arguments is nice,
+      // but makes field argument default values impossible to use.
+      substitutions[argumentName] = {
+        kind: Kind.VARIABLE,
+        name: { kind: Kind.NAME, value: '__UNSET' },
+      };
+    }
+  }
+  return substitutions;
+}

From baa0e425ea4bc61ac3c5f2c1221d7dd50b231c8d Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 15:40:36 -0500
Subject: [PATCH 11/13] Add Fragment Arg info to TypeInfo

---
 src/utilities/TypeInfo.ts | 92 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 85 insertions(+), 7 deletions(-)

diff --git a/src/utilities/TypeInfo.ts b/src/utilities/TypeInfo.ts
index fe8ea1fbab..06eef1f3cf 100644
--- a/src/utilities/TypeInfo.ts
+++ b/src/utilities/TypeInfo.ts
@@ -1,6 +1,12 @@
 import type { Maybe } from '../jsutils/Maybe.js';
+import type { ObjMap } from '../jsutils/ObjMap.js';
 
-import type { ASTNode, FieldNode } from '../language/ast.js';
+import type {
+  ASTNode,
+  FieldNode,
+  FragmentDefinitionNode,
+  FragmentSpreadNode,
+} from '../language/ast.js';
 import { isNode } from '../language/ast.js';
 import { Kind } from '../language/kinds.js';
 import type { ASTVisitor } from '../language/visitor.js';
@@ -31,6 +37,7 @@ import type { GraphQLDirective } from '../type/directives.js';
 import type { GraphQLSchema } from '../type/schema.js';
 
 import { typeFromAST } from './typeFromAST.js';
+import { valueFromAST } from './valueFromAST.js';
 
 /**
  * TypeInfo is a utility class which, given a GraphQL schema, can keep track
@@ -39,6 +46,7 @@ import { typeFromAST } from './typeFromAST.js';
  */
 export class TypeInfo {
   private _schema: GraphQLSchema;
+
   private _typeStack: Array<Maybe<GraphQLOutputType>>;
   private _parentTypeStack: Array<Maybe<GraphQLCompositeType>>;
   private _inputTypeStack: Array<Maybe<GraphQLInputType>>;
@@ -47,6 +55,8 @@ export class TypeInfo {
   private _directive: Maybe<GraphQLDirective>;
   private _argument: Maybe<GraphQLArgument>;
   private _enumValue: Maybe<GraphQLEnumValue>;
+  private _fragmentSpread: Maybe<FragmentSpreadNode>;
+  private _fragmentDefinitions: ObjMap<FragmentDefinitionNode>;
   private _getFieldDef: GetFieldDefFn;
 
   constructor(
@@ -69,6 +79,8 @@ export class TypeInfo {
     this._directive = null;
     this._argument = null;
     this._enumValue = null;
+    this._fragmentSpread = null;
+    this._fragmentDefinitions = Object.create(null);
     this._getFieldDef = getFieldDefFn ?? getFieldDef;
     if (initialType) {
       if (isInputType(initialType)) {
@@ -142,6 +154,17 @@ export class TypeInfo {
     // checked before continuing since TypeInfo is used as part of validation
     // which occurs before guarantees of schema and document validity.
     switch (node.kind) {
+      case Kind.DOCUMENT: {
+        // A document's fragment definitions are type signatures
+        // referenced via fragment spreads. Ensure we can use definitions
+        // before visiting their call sites.
+        for (const astNode of node.definitions) {
+          if (astNode.kind === Kind.FRAGMENT_DEFINITION) {
+            this._fragmentDefinitions[astNode.name.value] = astNode;
+          }
+        }
+        break;
+      }
       case Kind.SELECTION_SET: {
         const namedType: unknown = getNamedType(this.getType());
         this._parentTypeStack.push(
@@ -180,6 +203,10 @@ export class TypeInfo {
         this._typeStack.push(isOutputType(outputType) ? outputType : undefined);
         break;
       }
+      case Kind.FRAGMENT_SPREAD: {
+        this._fragmentSpread = node;
+        break;
+      }
       case Kind.VARIABLE_DEFINITION: {
         const inputType: unknown = typeFromAST(schema, node.type);
         this._inputTypeStack.push(
@@ -187,18 +214,60 @@ export class TypeInfo {
         );
         break;
       }
+      case Kind.FRAGMENT_ARGUMENT_DEFINITION: {
+        const inputType: unknown = typeFromAST(schema, node.type);
+        this._inputTypeStack.push(
+          isInputType(inputType) ? inputType : undefined,
+        );
+        break;
+      }
       case Kind.ARGUMENT: {
         let argDef;
         let argType: unknown;
-        const fieldOrDirective = this.getDirective() ?? this.getFieldDef();
-        if (fieldOrDirective) {
-          argDef = fieldOrDirective.args.find(
-            (arg) => arg.name === node.name.value,
+        const directive = this.getDirective();
+        const fragmentSpread = this._fragmentSpread;
+        const fieldDef = this.getFieldDef();
+        if (directive) {
+          argDef = directive.args.find((arg) => arg.name === node.name.value);
+        } else if (fragmentSpread) {
+          const fragmentDef =
+            this._fragmentDefinitions[fragmentSpread.name.value];
+          const fragArgDef = fragmentDef?.arguments?.find(
+            (arg) => arg.variable.name.value === node.name.value,
           );
-          if (argDef) {
-            argType = argDef.type;
+          if (fragArgDef) {
+            const fragArgType = typeFromAST(schema, fragArgDef.type);
+            if (isInputType(fragArgType)) {
+              const fragArgDefault = fragArgDef.defaultValue
+                ? valueFromAST(fragArgDef.defaultValue, fragArgType)
+                : undefined;
+
+              // Minor hack: transform the FragmentArgDef
+              // into a schema Argument definition, to
+              // enable visiting identically to field/directive args
+              const schemaArgDef: GraphQLArgument = {
+                name: fragArgDef.variable.name.value,
+                type: fragArgType,
+                defaultValue: fragArgDefault,
+                description: fragArgDef.description?.value,
+                deprecationReason: undefined,
+                extensions: {},
+                astNode: {
+                  ...fragArgDef,
+                  kind: Kind.INPUT_VALUE_DEFINITION,
+                  name: fragArgDef.variable.name,
+                },
+              };
+              argDef = schemaArgDef;
+            }
           }
+        } else if (fieldDef) {
+          argDef = fieldDef.args.find((arg) => arg.name === node.name.value);
+        }
+        if (argDef) {
+          argType = argDef.type;
         }
+
         this._argument = argDef;
         this._defaultValueStack.push(argDef ? argDef.defaultValue : undefined);
         this._inputTypeStack.push(isInputType(argType) ? argType : undefined);
@@ -248,6 +317,9 @@ export class TypeInfo {
 
   leave(node: ASTNode) {
     switch (node.kind) {
+      case Kind.DOCUMENT:
+        this._fragmentDefinitions = Object.create(null);
+        break;
       case Kind.SELECTION_SET:
         this._parentTypeStack.pop();
         break;
@@ -263,6 +335,12 @@ export class TypeInfo {
       case Kind.FRAGMENT_DEFINITION:
         this._typeStack.pop();
         break;
+      case Kind.FRAGMENT_ARGUMENT_DEFINITION:
+        this._inputTypeStack.pop();
+        break;
+      case Kind.FRAGMENT_SPREAD:
+        this._fragmentSpread = null;
+        break;
       case Kind.VARIABLE_DEFINITION:
         this._inputTypeStack.pop();
         break;

From 1af6fdfd5f52080e4de2d6fb831a228cba004f9c Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 16:30:06 -0500
Subject: [PATCH 12/13] Update ValidationContext for Fragment Args. Fixes
 typeInfo doc resetting

---
 src/validation/ValidationContext.ts | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/validation/ValidationContext.ts b/src/validation/ValidationContext.ts
index b0c5524fa7..65c2e27537 100644
--- a/src/validation/ValidationContext.ts
+++ b/src/validation/ValidationContext.ts
@@ -5,6 +5,7 @@ import type { GraphQLError } from '../error/GraphQLError.js';
 
 import type {
   DocumentNode,
+  FragmentArgumentDefinitionNode,
   FragmentDefinitionNode,
   FragmentSpreadNode,
   OperationDefinitionNode,
@@ -26,13 +27,15 @@ import type {
 import type { GraphQLDirective } from '../type/directives.js';
 import type { GraphQLSchema } from '../type/schema.js';
 
-import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo.js';
+import type { TypeInfo } from '../utilities/TypeInfo.js';
+import { visitWithTypeInfo } from '../utilities/TypeInfo.js';
 
 type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode;
 interface VariableUsage {
   readonly node: VariableNode;
   readonly type: Maybe<GraphQLInputType>;
   readonly defaultValue: Maybe<unknown>;
+  readonly fragmentArgDef: Maybe<FragmentArgumentDefinitionNode>;
 }
 
 /**
@@ -199,16 +202,23 @@ export class ValidationContext extends ASTValidationContext {
     let usages = this._variableUsages.get(node);
     if (!usages) {
       const newUsages: Array<VariableUsage> = [];
-      const typeInfo = new TypeInfo(this._schema);
+      const typeInfo = this._typeInfo;
+      const localArgumentDefinitions =
+        node.kind === Kind.FRAGMENT_DEFINITION ? node.arguments : undefined;
       visit(
         node,
         visitWithTypeInfo(typeInfo, {
           VariableDefinition: () => false,
+          FragmentArgumentDefinition: () => false,
           Variable(variable) {
+            const fragmentArgDef = localArgumentDefinitions?.find(
+              (argDef) => argDef.variable.name.value === variable.name.value,
+            );
             newUsages.push({
               node: variable,
               type: typeInfo.getInputType(),
               defaultValue: typeInfo.getDefaultValue(),
+              fragmentArgDef,
             });
           },
         }),

From e0f432efa5ffe9e74a6c34ed9fcad51c2759ab7c Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 16:31:24 -0500
Subject: [PATCH 13/13] Update all validation rules: tests pass

---
 src/language/directiveLocation.ts             |   1 +
 src/utilities/__tests__/TypeInfo-test.ts      | 107 ++++
 .../__tests__/KnownDirectivesRule-test.ts     |  14 +-
 .../NoUnusedFragmentArgumentsRule-test.ts     | 120 +++++
 src/validation/index.ts                       |   3 +
 src/validation/rules/KnownDirectivesRule.ts   |   2 +
 .../rules/NoUndefinedVariablesRule.ts         |   5 +-
 .../rules/NoUnusedFragmentArgumentsRule.ts    |  40 ++
 src/validation/rules/NoUnusedVariablesRule.ts |   5 +-
 .../rules/OverlappingFieldsCanBeMergedRule.ts | 487 +++++++++++-------
 .../rules/ProvidedRequiredArgumentsRule.ts    |  40 +-
 .../rules/VariablesAreInputTypesRule.ts       |  37 +-
 .../rules/VariablesInAllowedPositionRule.ts   |   9 +-
 src/validation/specifiedRules.ts              |   3 +
 14 files changed, 666 insertions(+), 207 deletions(-)
 create mode 100644 src/validation/__tests__/NoUnusedFragmentArgumentsRule-test.ts
 create mode 100644 src/validation/rules/NoUnusedFragmentArgumentsRule.ts

diff --git a/src/language/directiveLocation.ts b/src/language/directiveLocation.ts
index b10214d47f..3ff959e0c1 100644
--- a/src/language/directiveLocation.ts
+++ b/src/language/directiveLocation.ts
@@ -11,6 +11,7 @@ export enum DirectiveLocation {
   FRAGMENT_SPREAD = 'FRAGMENT_SPREAD',
   INLINE_FRAGMENT = 'INLINE_FRAGMENT',
   VARIABLE_DEFINITION = 'VARIABLE_DEFINITION',
+  FRAGMENT_ARGUMENT_DEFINITION = 'FRAGMENT_ARGUMENT_DEFINITION',
   /** Type System Definitions */
   SCHEMA = 'SCHEMA',
   SCALAR = 'SCALAR',
diff --git a/src/utilities/__tests__/TypeInfo-test.ts b/src/utilities/__tests__/TypeInfo-test.ts
index 971316f8b4..8c3f9e2986 100644
--- a/src/utilities/__tests__/TypeInfo-test.ts
+++ b/src/utilities/__tests__/TypeInfo-test.ts
@@ -515,4 +515,111 @@ describe('visitWithTypeInfo', () => {
       ['leave', 'SelectionSet', null, 'Human', 'Human'],
     ]);
   });
+
+  it('supports traversals of fragment arguments', () => {
+    const typeInfo = new TypeInfo(testSchema);
+
+    const ast = parse(`
+      query {
+        ...Foo(x: 4)
+      }
+      
+      fragment Foo(
+        "Human to get"
+        $x: ID!
+      ) on QueryRoot {
+        human(id: $x) { name }
+      }
+    `);
+
+    const visited: Array<any> = [];
+    visit(
+      ast,
+      visitWithTypeInfo(typeInfo, {
+        enter(node) {
+          const type = typeInfo.getType();
+          const inputType = typeInfo.getInputType();
+          visited.push([
+            'enter',
+            node.kind,
+            node.kind === 'Name' ? node.value : null,
+            String(type),
+            String(inputType),
+          ]);
+        },
+        leave(node) {
+          const type = typeInfo.getType();
+          const inputType = typeInfo.getInputType();
+          visited.push([
+            'leave',
+            node.kind,
+            node.kind === 'Name' ? node.value : null,
+            String(type),
+            String(inputType),
+          ]);
+        },
+      }),
+    );
+
+    expect(visited).to.deep.equal([
+      ['enter', 'Document', null, 'undefined', 'undefined'],
+      ['enter', 'OperationDefinition', null, 'QueryRoot', 'undefined'],
+      ['enter', 'SelectionSet', null, 'QueryRoot', 'undefined'],
+      ['enter', 'FragmentSpread', null, 'QueryRoot', 'undefined'],
+      ['enter', 'Name', 'Foo', 'QueryRoot', 'undefined'],
+      ['leave', 'Name', 'Foo', 'QueryRoot', 'undefined'],
+      ['enter', 'Argument', null, 'QueryRoot', 'ID!'],
+      ['enter', 'Name', 'x', 'QueryRoot', 'ID!'],
+      ['leave', 'Name', 'x', 'QueryRoot', 'ID!'],
+      ['enter', 'IntValue', null, 'QueryRoot', 'ID!'],
+      ['leave', 'IntValue', null, 'QueryRoot', 'ID!'],
+      ['leave', 'Argument', null, 'QueryRoot', 'ID!'],
+      ['leave', 'FragmentSpread', null, 'QueryRoot', 'undefined'],
+      ['leave', 'SelectionSet', null, 'QueryRoot', 'undefined'],
+      ['leave', 'OperationDefinition', null, 'QueryRoot', 'undefined'],
+      ['enter', 'FragmentDefinition', null, 'QueryRoot', 'undefined'],
+      ['enter', 'Name', 'Foo', 'QueryRoot', 'undefined'],
+      ['leave', 'Name', 'Foo', 'QueryRoot', 'undefined'],
+      ['enter', 'FragmentArgumentDefinition', null, 'QueryRoot', 'ID!'],
+      ['enter', 'StringValue', null, 'QueryRoot', 'ID!'],
+      ['leave', 'StringValue', null, 'QueryRoot', 'ID!'],
+      ['enter', 'Variable', null, 'QueryRoot', 'ID!'],
+      ['enter', 'Name', 'x', 'QueryRoot', 'ID!'],
+      ['leave', 'Name', 'x', 'QueryRoot', 'ID!'],
+      ['leave', 'Variable', null, 'QueryRoot', 'ID!'],
+      ['enter', 'NonNullType', null, 'QueryRoot', 'ID!'],
+      ['enter', 'NamedType', null, 'QueryRoot', 'ID!'],
+      ['enter', 'Name', 'ID', 'QueryRoot', 'ID!'],
+      ['leave', 'Name', 'ID', 'QueryRoot', 'ID!'],
+      ['leave', 'NamedType', null, 'QueryRoot', 'ID!'],
+      ['leave', 'NonNullType', null, 'QueryRoot', 'ID!'],
+      ['leave', 'FragmentArgumentDefinition', null, 'QueryRoot', 'ID!'],
+      ['enter', 'NamedType', null, 'QueryRoot', 'undefined'],
+      ['enter', 'Name', 'QueryRoot', 'QueryRoot', 'undefined'],
+      ['leave', 'Name', 'QueryRoot', 'QueryRoot', 'undefined'],
+      ['leave', 'NamedType', null, 'QueryRoot', 'undefined'],
+      ['enter', 'SelectionSet', null, 'QueryRoot', 'undefined'],
+      ['enter', 'Field', null, 'Human', 'undefined'],
+      ['enter', 'Name', 'human', 'Human', 'undefined'],
+      ['leave', 'Name', 'human', 'Human', 'undefined'],
+      ['enter', 'Argument', null, 'Human', 'ID'],
+      ['enter', 'Name', 'id', 'Human', 'ID'],
+      ['leave', 'Name', 'id', 'Human', 'ID'],
+      ['enter', 'Variable', null, 'Human', 'ID'],
+      ['enter', 'Name', 'x', 'Human', 'ID'],
+      ['leave', 'Name', 'x', 'Human', 'ID'],
+      ['leave', 'Variable', null, 'Human', 'ID'],
+      ['leave', 'Argument', null, 'Human', 'ID'],
+      ['enter', 'SelectionSet', null, 'Human', 'undefined'],
+      ['enter', 'Field', null, 'String', 'undefined'],
+      ['enter', 'Name', 'name', 'String', 'undefined'],
+      ['leave', 'Name', 'name', 'String', 'undefined'],
+      ['leave', 'Field', null, 'String', 'undefined'],
+      ['leave', 'SelectionSet', null, 'Human', 'undefined'],
+      ['leave', 'Field', null, 'Human', 'undefined'],
+      ['leave', 'SelectionSet', null, 'QueryRoot', 'undefined'],
+      ['leave', 'FragmentDefinition', null, 'QueryRoot', 'undefined'],
+      ['leave', 'Document', null, 'undefined', 'undefined'],
+    ]);
+  });
 });
diff --git a/src/validation/__tests__/KnownDirectivesRule-test.ts b/src/validation/__tests__/KnownDirectivesRule-test.ts
index a3bbc198da..f172ce5ca1 100644
--- a/src/validation/__tests__/KnownDirectivesRule-test.ts
+++ b/src/validation/__tests__/KnownDirectivesRule-test.ts
@@ -41,6 +41,7 @@ const schemaWithDirectives = buildSchema(`
   directive @onSubscription on SUBSCRIPTION
   directive @onField on FIELD
   directive @onFragmentDefinition on FRAGMENT_DEFINITION
+  directive @onFragmentArgumentDefinition on FRAGMENT_ARGUMENT_DEFINITION
   directive @onFragmentSpread on FRAGMENT_SPREAD
   directive @onInlineFragment on INLINE_FRAGMENT
   directive @onVariableDefinition on VARIABLE_DEFINITION
@@ -150,7 +151,9 @@ describe('Validate: Known directives', () => {
         someField @onField
       }
 
-      fragment Frag on Human @onFragmentDefinition {
+      fragment Frag(
+        $arg: Int @onFragmentArgumentDefinition
+      ) on Human @onFragmentDefinition {
         name @onField
       }
     `);
@@ -175,7 +178,7 @@ describe('Validate: Known directives', () => {
         someField @onQuery
       }
 
-      fragment Frag on Human @onQuery {
+      fragment Frag($arg: Int @onField) on Human @onQuery {
         name @onQuery
       }
     `).toDeepEqual([
@@ -219,9 +222,14 @@ describe('Validate: Known directives', () => {
         message: 'Directive "@onQuery" may not be used on FIELD.',
         locations: [{ column: 19, line: 16 }],
       },
+      {
+        message:
+          'Directive "@onField" may not be used on FRAGMENT_ARGUMENT_DEFINITION.',
+        locations: [{ column: 31, line: 19 }],
+      },
       {
         message: 'Directive "@onQuery" may not be used on FRAGMENT_DEFINITION.',
-        locations: [{ column: 30, line: 19 }],
+        locations: [{ column: 50, line: 19 }],
       },
       {
         message: 'Directive "@onQuery" may not be used on FIELD.',
diff --git a/src/validation/__tests__/NoUnusedFragmentArgumentsRule-test.ts b/src/validation/__tests__/NoUnusedFragmentArgumentsRule-test.ts
new file mode 100644
index 0000000000..9ca54af80c
--- /dev/null
+++ b/src/validation/__tests__/NoUnusedFragmentArgumentsRule-test.ts
@@ -0,0 +1,120 @@
+import { describe, it } from 'mocha';
+
+import { NoUnusedFragmentArgumentsRule } from '../rules/NoUnusedFragmentArgumentsRule.js';
+
+import { expectValidationErrors } from './harness.js';
+
+function expectErrors(queryStr: string) {
+  return expectValidationErrors(NoUnusedFragmentArgumentsRule, queryStr);
+}
+
+function expectValid(queryStr: string) {
+  expectErrors(queryStr).toDeepEqual([]);
+}
+
+describe('Validate: No unused fragment arguments', () => {
+  it('uses all arguments', () => {
+    expectValid(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(a: $a, b: $b, c: $c)
+      }
+    `);
+  });
+
+  it('uses all arguments deeply', () => {
+    expectValid(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(a: $a) {
+          field(b: $b) {
+            field(c: $c)
+          }
+        }
+      }
+    `);
+  });
+
+  it('uses all arguments deeply in inline fragments', () => {
+    expectValid(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        ... on Type {
+          field(a: $a) {
+            field(b: $b) {
+              ... on Type {
+                field(c: $c)
+              }
+            }
+          }
+        }
+      }
+    `);
+  });
+
+  it('argument not used', () => {
+    expectErrors(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(a: $a, b: $b)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Argument "$c" is never used in fragment "Foo".',
+        locations: [{ line: 2, column: 44 }],
+      },
+    ]);
+  });
+
+  it('query passes in unused argument', () => {
+    expectErrors(`
+      query Q($c: String) {
+        type {
+          ...Foo(a: "", b: "", c: $c)
+        }
+      }
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(a: $a, b: $b)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Argument "$c" is never used in fragment "Foo".',
+        locations: [{ line: 7, column: 44 }],
+      },
+    ]);
+  });
+
+  it('child fragment uses a variable of the same name', () => {
+    expectErrors(`
+      query Q($a: String) {
+        type {
+          ...Foo
+        }
+      }
+      fragment Foo($a: String) on Type {
+        ...Bar
+      }
+      fragment Bar on Type {
+        field(a: $a)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Argument "$a" is never used in fragment "Foo".',
+        locations: [{ line: 7, column: 20 }],
+      },
+    ]);
+  });
+
+  it('multiple arguments not used', () => {
+    expectErrors(`
+      fragment Foo($a: String, $b: String, $c: String) on Type {
+        field(b: $b)
+      }
+    `).toDeepEqual([
+      {
+        message: 'Argument "$a" is never used in fragment "Foo".',
+        locations: [{ line: 2, column: 20 }],
+      },
+      {
+        message: 'Argument "$c" is never used in fragment "Foo".',
+        locations: [{ line: 2, column: 44 }],
+      },
+    ]);
+  });
+});
diff --git a/src/validation/index.ts b/src/validation/index.ts
index b0cc754490..b5c4312607 100644
--- a/src/validation/index.ts
+++ b/src/validation/index.ts
@@ -48,6 +48,9 @@ export { NoUndefinedVariablesRule } from './rules/NoUndefinedVariablesRule.js';
 // Spec Section: "Fragments must be used"
 export { NoUnusedFragmentsRule } from './rules/NoUnusedFragmentsRule.js';
 
+// Spec Section: "All Fragment Arguments Used"
+export { NoUnusedFragmentArgumentsRule } from './rules/NoUnusedFragmentArgumentsRule.js';
+
 // Spec Section: "All Variables Used"
 export { NoUnusedVariablesRule } from './rules/NoUnusedVariablesRule.js';
 
diff --git a/src/validation/rules/KnownDirectivesRule.ts b/src/validation/rules/KnownDirectivesRule.ts
index 57641b91e7..daee27a798 100644
--- a/src/validation/rules/KnownDirectivesRule.ts
+++ b/src/validation/rules/KnownDirectivesRule.ts
@@ -86,6 +86,8 @@ function getDirectiveLocationForASTPath(
       return DirectiveLocation.INLINE_FRAGMENT;
     case Kind.FRAGMENT_DEFINITION:
       return DirectiveLocation.FRAGMENT_DEFINITION;
+    case Kind.FRAGMENT_ARGUMENT_DEFINITION:
+      return DirectiveLocation.FRAGMENT_ARGUMENT_DEFINITION;
     case Kind.VARIABLE_DEFINITION:
       return DirectiveLocation.VARIABLE_DEFINITION;
     case Kind.SCHEMA_DEFINITION:
diff --git a/src/validation/rules/NoUndefinedVariablesRule.ts b/src/validation/rules/NoUndefinedVariablesRule.ts
index d1672ecd0b..8375d82b5e 100644
--- a/src/validation/rules/NoUndefinedVariablesRule.ts
+++ b/src/validation/rules/NoUndefinedVariablesRule.ts
@@ -22,7 +22,10 @@ export function NoUndefinedVariablesRule(
       );
 
       const usages = context.getRecursiveVariableUsages(operation);
-      for (const { node } of usages) {
+      for (const { node, fragmentArgDef } of usages) {
+        if (fragmentArgDef) {
+          continue;
+        }
         const varName = node.name.value;
         if (!variableNameDefined.has(varName)) {
           context.reportError(
diff --git a/src/validation/rules/NoUnusedFragmentArgumentsRule.ts b/src/validation/rules/NoUnusedFragmentArgumentsRule.ts
new file mode 100644
index 0000000000..1002d4c1d1
--- /dev/null
+++ b/src/validation/rules/NoUnusedFragmentArgumentsRule.ts
@@ -0,0 +1,40 @@
+import { GraphQLError } from '../../error/GraphQLError.js';
+
+import type { ASTVisitor } from '../../language/visitor.js';
+
+import type { ValidationContext } from '../ValidationContext.js';
+
+/**
+ * No unused variables
+ *
+ * A GraphQL fragment is only valid if all arguments defined by it
+ * are used within the same fragment.
+ *
+ * See https://spec.graphql.org/draft/#sec-All-Variables-Used
+ */
+export function NoUnusedFragmentArgumentsRule(
+  context: ValidationContext,
+): ASTVisitor {
+  return {
+    FragmentDefinition(fragment) {
+      const usages = context.getVariableUsages(fragment);
+      const argumentNameUsed = new Set<string>(
+        usages.map(({ node }) => node.name.value),
+      );
+      // FIXME: https://github.com/graphql/graphql-js/issues/2203
+      /* c8 ignore next */
+      const argumentDefinitions = fragment.arguments ?? [];
+      for (const argDef of argumentDefinitions) {
+        const argName = argDef.variable.name.value;
+        if (!argumentNameUsed.has(argName)) {
+          context.reportError(
+            new GraphQLError(
+              `Argument "$${argName}" is never used in fragment "${fragment.name.value}".`,
+              { nodes: argDef },
+            ),
+          );
+        }
+      }
+    },
+  };
+}
diff --git a/src/validation/rules/NoUnusedVariablesRule.ts b/src/validation/rules/NoUnusedVariablesRule.ts
index 7a0660cce0..7fbeb82764 100644
--- a/src/validation/rules/NoUnusedVariablesRule.ts
+++ b/src/validation/rules/NoUnusedVariablesRule.ts
@@ -17,7 +17,10 @@ export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor {
     OperationDefinition(operation) {
       const usages = context.getRecursiveVariableUsages(operation);
       const variableNameUsed = new Set<string>(
-        usages.map(({ node }) => node.name.value),
+        usages
+          // Skip variables used as fragment arguments
+          .filter(({ fragmentArgDef }) => !fragmentArgDef)
+          .map(({ node }) => node.name.value),
       );
 
       // FIXME: https://github.com/graphql/graphql-js/issues/2203
diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
index e2444047c6..d46aae5a6f 100644
--- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
+++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
@@ -8,6 +8,7 @@ import type {
   DirectiveNode,
   FieldNode,
   FragmentDefinitionNode,
+  FragmentSpreadNode,
   ObjectValueNode,
   SelectionSetNode,
 } from '../../language/ast.js';
@@ -29,7 +30,9 @@ import {
   isObjectType,
 } from '../../type/definition.js';
 
+import { keyForFragmentSpread } from '../../utilities/keyForFragmentSpread.js';
 import { sortValueNode } from '../../utilities/sortValueNode.js';
+import { substituteFragmentArguments } from '../../utilities/substituteFragmentArguments.js';
 import { typeFromAST } from '../../utilities/typeFromAST.js';
 
 import type { ValidationContext } from '../ValidationContext.js';
@@ -38,17 +41,25 @@ import type { ValidationContext } from '../ValidationContext.js';
 // This file contains a lot of such errors but we plan to refactor it anyway
 // so just disable it for entire file.
 
-function reasonMessage(reason: ConflictReasonMessage): string {
-  if (Array.isArray(reason)) {
-    return reason
-      .map(
-        ([responseName, subReason]) =>
-          `subfields "${responseName}" conflict because ` +
-          reasonMessage(subReason),
-      )
+function reasonMessage(message: ConflictReasonMessage): string {
+  if (Array.isArray(message)) {
+    return message
+      .map((subC) => {
+        const subConflict = subC;
+        if (subConflict.kind === 'FIELD') {
+          return (
+            `subfields "${subConflict.responseName}" conflict because ` +
+            reasonMessage(subConflict.reasonMessage)
+          );
+        }
+        return (
+          `child spreads "${subConflict.fragmentName}" conflict because ` +
+          reasonMessage(subConflict.reasonMessage)
+        );
+      })
       .join(' and ');
   }
-  return reason;
+  return message;
 }
 
 /**
@@ -68,36 +79,62 @@ export function OverlappingFieldsCanBeMergedRule(
   // dramatically improve the performance of this validator.
   const comparedFragmentPairs = new PairSet();
 
-  // A cache for the "field map" and list of fragment names found in any given
+  // A cache for the "field map" and list of fragment spreads found in any given
   // selection set. Selection sets may be asked for this information multiple
   // times, so this improves the performance of this validator.
-  const cachedFieldsAndFragmentNames = new Map();
+  const cachedFieldsAndFragmentSpreads = new Map();
 
   return {
     SelectionSet(selectionSet) {
       const conflicts = findConflictsWithinSelectionSet(
         context,
-        cachedFieldsAndFragmentNames,
+        cachedFieldsAndFragmentSpreads,
         comparedFragmentPairs,
         context.getParentType(),
         selectionSet,
       );
-      for (const [[responseName, reason], fields1, fields2] of conflicts) {
-        const reasonMsg = reasonMessage(reason);
-        context.reportError(
-          new GraphQLError(
-            `Fields "${responseName}" conflict because ${reasonMsg}. Use different aliases on the fields to fetch both if this was intentional.`,
-            { nodes: fields1.concat(fields2) },
-          ),
-        );
+      for (const { reason, selectionPath1, selectionPath2 } of conflicts) {
+        const reasonMsg = reasonMessage(reason.reasonMessage);
+        const errorNodes = { nodes: selectionPath1.concat(selectionPath2) };
+        if (reason.kind === 'FIELD') {
+          context.reportError(
+            new GraphQLError(
+              `Fields "${reason.responseName}" conflict because ${reasonMsg}. Use different aliases on the fields to fetch both if this was intentional.`,
+              errorNodes,
+            ),
+          );
+        } else {
+          // FRAGMENT_SPREAD
+          context.reportError(
+            new GraphQLError(
+              // Fragments can't be aliased, so there's no easy way to resolve these conflicts today.
+              `Spreads "${reason.fragmentName}" conflict because ${reasonMsg}.`,
+              errorNodes,
+            ),
+          );
+        }
       }
     },
   };
 }
 
-type Conflict = [ConflictReason, Array<FieldNode>, Array<FieldNode>];
+interface Conflict {
+  reason: ConflictReason;
+  selectionPath1: Array<FieldNode | FragmentSpreadNode>;
+  selectionPath2: Array<FieldNode | FragmentSpreadNode>;
+}
 // Field name and reason.
-type ConflictReason = [string, ConflictReasonMessage];
+type ConflictReason = FieldConflictReason | FragmentSpreadConflictReason;
+interface FieldConflictReason {
+  kind: 'FIELD';
+  responseName: string;
+  reasonMessage: ConflictReasonMessage;
+}
+interface FragmentSpreadConflictReason {
+  kind: 'FRAGMENT_SPREAD';
+  fragmentName: string;
+  reasonMessage: ConflictReasonMessage;
+}
 // Reason is a string, or a nested list of conflicts.
 type ConflictReasonMessage = string | Array<ConflictReason>;
 // Tuple defining a field node in a context.
@@ -108,8 +145,11 @@ type NodeAndDef = [
 ];
 // Map of array of those.
 type NodeAndDefCollection = ObjMap<Array<NodeAndDef>>;
-type FragmentNames = ReadonlyArray<string>;
-type FieldsAndFragmentNames = readonly [NodeAndDefCollection, FragmentNames];
+type FragmentSpreadsByKey = ObjMap<FragmentSpreadNode>;
+type FieldsAndFragmentSpreads = readonly [
+  NodeAndDefCollection,
+  FragmentSpreadsByKey,
+];
 
 /**
  * Algorithm:
@@ -171,58 +211,60 @@ type FieldsAndFragmentNames = readonly [NodeAndDefCollection, FragmentNames];
 // GraphQL Document.
 function findConflictsWithinSelectionSet(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   parentType: Maybe<GraphQLNamedType>,
   selectionSet: SelectionSetNode,
 ): Array<Conflict> {
   const conflicts: Array<Conflict> = [];
 
-  const [fieldMap, fragmentNames] = getFieldsAndFragmentNames(
+  const [fieldMap, spreadCollection] = getFieldsAndFragmentSpreads(
     context,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     parentType,
     selectionSet,
   );
 
-  // (A) Find find all conflicts "within" the fields of this selection set.
+  // (A) First find all conflicts "within" the fields and spreads of this selection set.
   // Note: this is the *only place* `collectConflictsWithin` is called.
   collectConflictsWithin(
     context,
     conflicts,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     comparedFragmentPairs,
     fieldMap,
   );
 
-  if (fragmentNames.length !== 0) {
-    // (B) Then collect conflicts between these fields and those represented by
-    // each spread fragment name found.
-    for (let i = 0; i < fragmentNames.length; i++) {
-      collectConflictsBetweenFieldsAndFragment(
+  // (B) Then collect conflicts between these fields and those represented by
+  // each spread fragment name found.
+  const fragmentSpreads = Object.values(spreadCollection);
+  for (let i = 0; i < fragmentSpreads.length; i++) {
+    collectConflictsBetweenFieldsAndFragment(
+      context,
+      conflicts,
+      cachedFieldsAndFragmentSpreads,
+      comparedFragmentPairs,
+      false,
+      fieldMap,
+      fragmentSpreads[i],
+    );
+    // (C) Then compare this fragment with all other fragments found in this
+    // selection set to collect conflicts between fragments spread together.
+    // This compares each item in the list of fragment names to every other
+    // item in that same list (except for itself).
+    for (let j = i + 1; j < fragmentSpreads.length; j++) {
+      collectConflictsBetweenFragments(
         context,
         conflicts,
-        cachedFieldsAndFragmentNames,
+        cachedFieldsAndFragmentSpreads,
         comparedFragmentPairs,
         false,
-        fieldMap,
-        fragmentNames[i],
+        fragmentSpreads[i],
+        fragmentSpreads[j],
       );
-      // (C) Then compare this fragment with all other fragments found in this
-      // selection set to collect conflicts between fragments spread together.
-      // This compares each item in the list of fragment names to every other
-      // item in that same list (except for itself).
-      for (let j = i + 1; j < fragmentNames.length; j++) {
-        collectConflictsBetweenFragments(
-          context,
-          conflicts,
-          cachedFieldsAndFragmentNames,
-          comparedFragmentPairs,
-          false,
-          fragmentNames[i],
-          fragmentNames[j],
-        );
-      }
     }
   }
   return conflicts;
@@ -233,22 +275,28 @@ function findConflictsWithinSelectionSet(
 function collectConflictsBetweenFieldsAndFragment(
   context: ValidationContext,
   conflicts: Array<Conflict>,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   areMutuallyExclusive: boolean,
   fieldMap: NodeAndDefCollection,
-  fragmentName: string,
+  fragmentSpread: FragmentSpreadNode,
 ): void {
-  const fragment = context.getFragment(fragmentName);
-  if (!fragment) {
+  const fragmentName = fragmentSpread.name.value;
+  const fragmentDef = context.getFragment(fragmentName);
+  if (!fragmentDef) {
     return;
   }
 
-  const [fieldMap2, referencedFragmentNames] =
-    getReferencedFieldsAndFragmentNames(
+  const fragmentKey = keyForFragmentSpread(fragmentSpread);
+  const [fieldMap2, referencedFragmentSpreads] =
+    getReferencedFieldsAndFragmentSpreads(
       context,
-      cachedFieldsAndFragmentNames,
-      fragment,
+      cachedFieldsAndFragmentSpreads,
+      fragmentDef,
+      fragmentSpread,
     );
 
   // Do not compare a fragment's fieldMap to itself.
@@ -261,7 +309,7 @@ function collectConflictsBetweenFieldsAndFragment(
   collectConflictsBetween(
     context,
     conflicts,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     comparedFragmentPairs,
     areMutuallyExclusive,
     fieldMap,
@@ -270,31 +318,34 @@ function collectConflictsBetweenFieldsAndFragment(
 
   // (E) Then collect any conflicts between the provided collection of fields
   // and any fragment names found in the given fragment.
-  for (const referencedFragmentName of referencedFragmentNames) {
+  for (const [
+    referencedFragmentKey,
+    referencedFragmentSpread,
+  ] of Object.entries(referencedFragmentSpreads)) {
     // Memoize so two fragments are not compared for conflicts more than once.
     if (
       comparedFragmentPairs.has(
-        referencedFragmentName,
-        fragmentName,
+        referencedFragmentKey,
+        fragmentKey,
         areMutuallyExclusive,
       )
     ) {
       continue;
     }
     comparedFragmentPairs.add(
-      referencedFragmentName,
-      fragmentName,
+      referencedFragmentKey,
+      fragmentKey,
       areMutuallyExclusive,
     );
 
     collectConflictsBetweenFieldsAndFragment(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
       fieldMap,
-      referencedFragmentName,
+      referencedFragmentSpread,
     );
   }
 }
@@ -304,46 +355,64 @@ function collectConflictsBetweenFieldsAndFragment(
 function collectConflictsBetweenFragments(
   context: ValidationContext,
   conflicts: Array<Conflict>,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   areMutuallyExclusive: boolean,
-  fragmentName1: string,
-  fragmentName2: string,
+  fragmentSpread1: FragmentSpreadNode,
+  fragmentSpread2: FragmentSpreadNode,
 ): void {
+  const fragmentKey1 = keyForFragmentSpread(fragmentSpread1);
+  const fragmentKey2 = keyForFragmentSpread(fragmentSpread2);
   // No need to compare a fragment to itself.
-  if (fragmentName1 === fragmentName2) {
+  if (fragmentKey1 === fragmentKey2) {
     return;
   }
 
   // Memoize so two fragments are not compared for conflicts more than once.
   if (
-    comparedFragmentPairs.has(
-      fragmentName1,
-      fragmentName2,
-      areMutuallyExclusive,
-    )
+    comparedFragmentPairs.has(fragmentKey1, fragmentKey2, areMutuallyExclusive)
   ) {
     return;
   }
-  comparedFragmentPairs.add(fragmentName1, fragmentName2, areMutuallyExclusive);
+  comparedFragmentPairs.add(fragmentKey1, fragmentKey2, areMutuallyExclusive);
+
+  // Two unique fragment spreads reference the same fragment,
+  // which is a conflict
+  if (fragmentSpread1.name.value === fragmentSpread2.name.value) {
+    conflicts.push({
+      reason: {
+        kind: 'FRAGMENT_SPREAD',
+        fragmentName: fragmentSpread1.name.value,
+        reasonMessage: `${fragmentKey1} and ${fragmentKey2} have different fragment arguments`,
+      },
+      selectionPath1: [fragmentSpread1],
+      selectionPath2: [fragmentSpread2],
+    });
+    return;
+  }
 
-  const fragment1 = context.getFragment(fragmentName1);
-  const fragment2 = context.getFragment(fragmentName2);
-  if (!fragment1 || !fragment2) {
+  const fragmentDef1 = context.getFragment(fragmentSpread1.name.value);
+  const fragmentDef2 = context.getFragment(fragmentSpread2.name.value);
+  if (!fragmentDef1 || !fragmentDef2) {
     return;
   }
 
-  const [fieldMap1, referencedFragmentNames1] =
-    getReferencedFieldsAndFragmentNames(
+  const [fieldMap1, referencedFragmentSpreads1] =
+    getReferencedFieldsAndFragmentSpreads(
       context,
-      cachedFieldsAndFragmentNames,
-      fragment1,
+      cachedFieldsAndFragmentSpreads,
+      fragmentDef1,
+      fragmentSpread1,
     );
-  const [fieldMap2, referencedFragmentNames2] =
-    getReferencedFieldsAndFragmentNames(
+  const [fieldMap2, referencedFragmentSpreads2] =
+    getReferencedFieldsAndFragmentSpreads(
       context,
-      cachedFieldsAndFragmentNames,
-      fragment2,
+      cachedFieldsAndFragmentSpreads,
+      fragmentDef2,
+      fragmentSpread2,
     );
 
   // (F) First, collect all conflicts between these two collections of fields
@@ -351,7 +420,7 @@ function collectConflictsBetweenFragments(
   collectConflictsBetween(
     context,
     conflicts,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     comparedFragmentPairs,
     areMutuallyExclusive,
     fieldMap1,
@@ -360,29 +429,33 @@ function collectConflictsBetweenFragments(
 
   // (G) Then collect conflicts between the first fragment and any nested
   // fragments spread in the second fragment.
-  for (const referencedFragmentName2 of referencedFragmentNames2) {
+  for (const referencedFragmentSpread2 of Object.values(
+    referencedFragmentSpreads2,
+  )) {
     collectConflictsBetweenFragments(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
-      fragmentName1,
-      referencedFragmentName2,
+      fragmentSpread1,
+      referencedFragmentSpread2,
     );
   }
 
   // (G) Then collect conflicts between the second fragment and any nested
   // fragments spread in the first fragment.
-  for (const referencedFragmentName1 of referencedFragmentNames1) {
+  for (const referencedFragmentSpread1 of Object.values(
+    referencedFragmentSpreads1,
+  )) {
     collectConflictsBetweenFragments(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
-      referencedFragmentName1,
-      fragmentName2,
+      referencedFragmentSpread1,
+      fragmentSpread2,
     );
   }
 }
@@ -392,7 +465,10 @@ function collectConflictsBetweenFragments(
 // between the sub-fields of two overlapping fields.
 function findConflictsBetweenSubSelectionSets(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   areMutuallyExclusive: boolean,
   parentType1: Maybe<GraphQLNamedType>,
@@ -402,15 +478,15 @@ function findConflictsBetweenSubSelectionSets(
 ): Array<Conflict> {
   const conflicts: Array<Conflict> = [];
 
-  const [fieldMap1, fragmentNames1] = getFieldsAndFragmentNames(
+  const [fieldMap1, fragmentSpreads1] = getFieldsAndFragmentSpreads(
     context,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     parentType1,
     selectionSet1,
   );
-  const [fieldMap2, fragmentNames2] = getFieldsAndFragmentNames(
+  const [fieldMap2, fragmentSpreads2] = getFieldsAndFragmentSpreads(
     context,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     parentType2,
     selectionSet2,
   );
@@ -419,54 +495,56 @@ function findConflictsBetweenSubSelectionSets(
   collectConflictsBetween(
     context,
     conflicts,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     comparedFragmentPairs,
     areMutuallyExclusive,
     fieldMap1,
     fieldMap2,
   );
 
+  // (H.1) Collect all conflicts between the two sets of fragment spreads
+
   // (I) Then collect conflicts between the first collection of fields and
-  // those referenced by each fragment name associated with the second.
-  for (const fragmentName2 of fragmentNames2) {
+  // those referenced by each fragment spread associated with the second.
+  for (const fragmentSpread2 of Object.values(fragmentSpreads2)) {
     collectConflictsBetweenFieldsAndFragment(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
       fieldMap1,
-      fragmentName2,
+      fragmentSpread2,
     );
   }
 
   // (I) Then collect conflicts between the second collection of fields and
-  // those referenced by each fragment name associated with the first.
-  for (const fragmentName1 of fragmentNames1) {
+  // those referenced by each fragment spread associated with the first.
+  for (const fragmentSpread1 of Object.values(fragmentSpreads1)) {
     collectConflictsBetweenFieldsAndFragment(
       context,
       conflicts,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
       fieldMap2,
-      fragmentName1,
+      fragmentSpread1,
     );
   }
 
-  // (J) Also collect conflicts between any fragment names by the first and
-  // fragment names by the second. This compares each item in the first set of
-  // names to each item in the second set of names.
-  for (const fragmentName1 of fragmentNames1) {
-    for (const fragmentName2 of fragmentNames2) {
+  // (J) Also collect conflicts between any fragment spreads by the first and
+  // fragment spreads by the second. This compares each item in the first set of
+  // spreads to each item in the second set of spreads.
+  for (const fragmentSpread1 of Object.values(fragmentSpreads1)) {
+    for (const fragmentSpread2 of Object.values(fragmentSpreads2)) {
       collectConflictsBetweenFragments(
         context,
         conflicts,
-        cachedFieldsAndFragmentNames,
+        cachedFieldsAndFragmentSpreads,
         comparedFragmentPairs,
         areMutuallyExclusive,
-        fragmentName1,
-        fragmentName2,
+        fragmentSpread1,
+        fragmentSpread2,
       );
     }
   }
@@ -477,7 +555,10 @@ function findConflictsBetweenSubSelectionSets(
 function collectConflictsWithin(
   context: ValidationContext,
   conflicts: Array<Conflict>,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   fieldMap: NodeAndDefCollection,
 ): void {
@@ -492,9 +573,9 @@ function collectConflictsWithin(
     if (fields.length > 1) {
       for (let i = 0; i < fields.length; i++) {
         for (let j = i + 1; j < fields.length; j++) {
-          const conflict = findConflict(
+          const conflict = findFieldConflict(
             context,
-            cachedFieldsAndFragmentNames,
+            cachedFieldsAndFragmentSpreads,
             comparedFragmentPairs,
             false, // within one collection is never mutually exclusive
             responseName,
@@ -518,7 +599,10 @@ function collectConflictsWithin(
 function collectConflictsBetween(
   context: ValidationContext,
   conflicts: Array<Conflict>,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   parentFieldsAreMutuallyExclusive: boolean,
   fieldMap1: NodeAndDefCollection,
@@ -534,9 +618,9 @@ function collectConflictsBetween(
     if (fields2) {
       for (const field1 of fields1) {
         for (const field2 of fields2) {
-          const conflict = findConflict(
+          const conflict = findFieldConflict(
             context,
-            cachedFieldsAndFragmentNames,
+            cachedFieldsAndFragmentSpreads,
             comparedFragmentPairs,
             parentFieldsAreMutuallyExclusive,
             responseName,
@@ -554,9 +638,12 @@ function collectConflictsBetween(
 
 // Determines if there is a conflict between two particular fields, including
 // comparing their sub-fields.
-function findConflict(
+function findFieldConflict(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   comparedFragmentPairs: PairSet,
   parentFieldsAreMutuallyExclusive: boolean,
   responseName: string,
@@ -585,20 +672,28 @@ function findConflict(
     const name1 = node1.name.value;
     const name2 = node2.name.value;
     if (name1 !== name2) {
-      return [
-        [responseName, `"${name1}" and "${name2}" are different fields`],
-        [node1],
-        [node2],
-      ];
+      return {
+        reason: {
+          kind: 'FIELD',
+          responseName,
+          reasonMessage: `"${name1}" and "${name2}" are different fields`,
+        },
+        selectionPath1: [node1],
+        selectionPath2: [node2],
+      };
     }
 
     // Two field calls must have the same arguments.
     if (stringifyArguments(node1) !== stringifyArguments(node2)) {
-      return [
-        [responseName, 'they have differing arguments'],
-        [node1],
-        [node2],
-      ];
+      return {
+        reason: {
+          kind: 'FIELD',
+          responseName,
+          reasonMessage: 'they have differing arguments',
+        },
+        selectionPath1: [node1],
+        selectionPath2: [node2],
+      };
     }
   }
 
@@ -606,11 +701,15 @@ function findConflict(
   const directives1 = /* c8 ignore next */ node1.directives ?? [];
   const directives2 = /* c8 ignore next */ node2.directives ?? [];
   if (!sameStreams(directives1, directives2)) {
-    return [
-      [responseName, 'they have differing stream directives'],
-      [node1],
-      [node2],
-    ];
+    return {
+      reason: {
+        kind: 'FIELD',
+        responseName,
+        reasonMessage: 'they have differing stream directives',
+      },
+      selectionPath1: [node1],
+      selectionPath2: [node2],
+    };
   }
 
   // The return type for each field.
@@ -618,16 +717,17 @@ function findConflict(
   const type2 = def2?.type;
 
   if (type1 && type2 && doTypesConflict(type1, type2)) {
-    return [
-      [
+    return {
+      reason: {
+        kind: 'FIELD',
         responseName,
-        `they return conflicting types "${inspect(type1)}" and "${inspect(
-          type2,
-        )}"`,
-      ],
-      [node1],
-      [node2],
-    ];
+        reasonMessage: `they return conflicting types "${inspect(
+          type1,
+        )}" and "${inspect(type2)}"`,
+      },
+      selectionPath1: [node1],
+      selectionPath2: [node2],
+    };
   }
 
   // Collect and compare sub-fields. Use the same "visited fragment names" list
@@ -638,7 +738,7 @@ function findConflict(
   if (selectionSet1 && selectionSet2) {
     const conflicts = findConflictsBetweenSubSelectionSets(
       context,
-      cachedFieldsAndFragmentNames,
+      cachedFieldsAndFragmentSpreads,
       comparedFragmentPairs,
       areMutuallyExclusive,
       getNamedType(type1),
@@ -720,58 +820,73 @@ function doTypesConflict(
 // Given a selection set, return the collection of fields (a mapping of response
 // name to field nodes and definitions) as well as a list of fragment names
 // referenced via fragment spreads.
-function getFieldsAndFragmentNames(
+function getFieldsAndFragmentSpreads(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
   parentType: Maybe<GraphQLNamedType>,
   selectionSet: SelectionSetNode,
-): FieldsAndFragmentNames {
-  const cached = cachedFieldsAndFragmentNames.get(selectionSet);
+): FieldsAndFragmentSpreads {
+  const cached = cachedFieldsAndFragmentSpreads.get(selectionSet);
   if (cached) {
     return cached;
   }
   const nodeAndDefs: NodeAndDefCollection = Object.create(null);
-  const fragmentNames = new Set<string>();
-  _collectFieldsAndFragmentNames(
+  const fragmentSpreads: ObjMap<FragmentSpreadNode> = {};
+  _collectFieldsAndFragmentSpreads(
     context,
     parentType,
     selectionSet,
     nodeAndDefs,
-    fragmentNames,
+    fragmentSpreads,
   );
-  const result = [nodeAndDefs, [...fragmentNames]] as const;
-  cachedFieldsAndFragmentNames.set(selectionSet, result);
+  const result = [nodeAndDefs, { ...fragmentSpreads }] as const;
+  cachedFieldsAndFragmentSpreads.set(selectionSet, result);
   return result;
 }
 
 // Given a reference to a fragment, return the represented collection of fields
-// as well as a list of nested fragment names referenced via fragment spreads.
-function getReferencedFieldsAndFragmentNames(
+// as well as a list of nested referenced fragment spreads.
+function getReferencedFieldsAndFragmentSpreads(
   context: ValidationContext,
-  cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
-  fragment: FragmentDefinitionNode,
+  cachedFieldsAndFragmentSpreads: Map<
+    SelectionSetNode,
+    FieldsAndFragmentSpreads
+  >,
+  fragmentDef: FragmentDefinitionNode,
+  fragmentSpread: FragmentSpreadNode,
 ) {
+  const fragmentSelectionSet = substituteFragmentArguments(
+    fragmentDef,
+    fragmentSpread,
+  );
+
   // Short-circuit building a type from the node if possible.
-  const cached = cachedFieldsAndFragmentNames.get(fragment.selectionSet);
+  const cached = cachedFieldsAndFragmentSpreads.get(fragmentSelectionSet);
   if (cached) {
     return cached;
   }
 
-  const fragmentType = typeFromAST(context.getSchema(), fragment.typeCondition);
-  return getFieldsAndFragmentNames(
+  const fragmentType = typeFromAST(
+    context.getSchema(),
+    fragmentDef.typeCondition,
+  );
+  return getFieldsAndFragmentSpreads(
     context,
-    cachedFieldsAndFragmentNames,
+    cachedFieldsAndFragmentSpreads,
     fragmentType,
-    fragment.selectionSet,
+    fragmentSelectionSet,
   );
 }
 
-function _collectFieldsAndFragmentNames(
+function _collectFieldsAndFragmentSpreads(
   context: ValidationContext,
   parentType: Maybe<GraphQLNamedType>,
   selectionSet: SelectionSetNode,
   nodeAndDefs: NodeAndDefCollection,
-  fragmentNames: Set<string>,
+  fragmentSpreads: FragmentSpreadsByKey,
 ): void {
   for (const selection of selectionSet.selections) {
     switch (selection.kind) {
@@ -791,19 +906,19 @@ function _collectFieldsAndFragmentNames(
         break;
       }
       case Kind.FRAGMENT_SPREAD:
-        fragmentNames.add(selection.name.value);
+        fragmentSpreads[keyForFragmentSpread(selection)] = selection;
         break;
       case Kind.INLINE_FRAGMENT: {
         const typeCondition = selection.typeCondition;
         const inlineFragmentType = typeCondition
           ? typeFromAST(context.getSchema(), typeCondition)
           : parentType;
-        _collectFieldsAndFragmentNames(
+        _collectFieldsAndFragmentSpreads(
           context,
           inlineFragmentType,
           selection.selectionSet,
           nodeAndDefs,
-          fragmentNames,
+          fragmentSpreads,
         );
         break;
       }
@@ -820,11 +935,21 @@ function subfieldConflicts(
   node2: FieldNode,
 ): Maybe<Conflict> {
   if (conflicts.length > 0) {
-    return [
-      [responseName, conflicts.map(([reason]) => reason)],
-      [node1, ...conflicts.map(([, fields1]) => fields1).flat()],
-      [node2, ...conflicts.map(([, , fields2]) => fields2).flat()],
-    ];
+    return {
+      reason: {
+        kind: 'FIELD',
+        responseName,
+        reasonMessage: conflicts.map((conflict) => conflict.reason),
+      },
+      selectionPath1: [
+        node1,
+        ...conflicts.map((subConflict) => subConflict.selectionPath1).flat(),
+      ],
+      selectionPath2: [
+        node2,
+        ...conflicts.map((subConflict) => subConflict.selectionPath2).flat(),
+      ],
+    };
   }
 }
 
diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.ts b/src/validation/rules/ProvidedRequiredArgumentsRule.ts
index 350264496f..c513a721c4 100644
--- a/src/validation/rules/ProvidedRequiredArgumentsRule.ts
+++ b/src/validation/rules/ProvidedRequiredArgumentsRule.ts
@@ -4,7 +4,10 @@ import type { ObjMap } from '../../jsutils/ObjMap.js';
 
 import { GraphQLError } from '../../error/GraphQLError.js';
 
-import type { InputValueDefinitionNode } from '../../language/ast.js';
+import type {
+  FragmentArgumentDefinitionNode,
+  InputValueDefinitionNode,
+} from '../../language/ast.js';
 import { Kind } from '../../language/kinds.js';
 import { print } from '../../language/printer.js';
 import type { ASTVisitor } from '../../language/visitor.js';
@@ -56,6 +59,37 @@ export function ProvidedRequiredArgumentsRule(
         }
       },
     },
+    FragmentSpread: {
+      // Validate on leave to allow for directive errors to appear first.
+      leave(spreadNode) {
+        const fragmentDef = context.getFragment(spreadNode.name.value);
+        if (!fragmentDef) {
+          return false;
+        }
+
+        const providedArgs = new Set(
+          // FIXME: https://github.com/graphql/graphql-js/issues/2203
+          /* c8 ignore next */
+          spreadNode.arguments?.map((arg) => arg.name.value),
+        );
+        // FIXME: https://github.com/graphql/graphql-js/issues/2203
+        /* c8 ignore next */
+        for (const argDef of fragmentDef.arguments ?? []) {
+          if (
+            !providedArgs.has(argDef.variable.name.value) &&
+            isRequiredArgumentNode(argDef)
+          ) {
+            const argTypeStr = inspect(argDef.type);
+            context.reportError(
+              new GraphQLError(
+                `Fragment "${spreadNode.name.value}" argument "${argDef.variable.name.value}" of type "${argTypeStr}" is required, but it was not provided.`,
+                { nodes: [spreadNode, argDef] },
+              ),
+            );
+          }
+        }
+      },
+    },
   };
 }
 
@@ -122,6 +156,8 @@ export function ProvidedRequiredArgumentsOnDirectivesRule(
   };
 }
 
-function isRequiredArgumentNode(arg: InputValueDefinitionNode): boolean {
+function isRequiredArgumentNode(
+  arg: InputValueDefinitionNode | FragmentArgumentDefinitionNode,
+): boolean {
   return arg.type.kind === Kind.NON_NULL_TYPE && arg.defaultValue == null;
 }
diff --git a/src/validation/rules/VariablesAreInputTypesRule.ts b/src/validation/rules/VariablesAreInputTypesRule.ts
index 019f3e762c..b4a7ad1de8 100644
--- a/src/validation/rules/VariablesAreInputTypesRule.ts
+++ b/src/validation/rules/VariablesAreInputTypesRule.ts
@@ -1,6 +1,9 @@
 import { GraphQLError } from '../../error/GraphQLError.js';
 
-import type { VariableDefinitionNode } from '../../language/ast.js';
+import type {
+  FragmentArgumentDefinitionNode,
+  VariableDefinitionNode,
+} from '../../language/ast.js';
 import { print } from '../../language/printer.js';
 import type { ASTVisitor } from '../../language/visitor.js';
 
@@ -21,21 +24,25 @@ import type { ValidationContext } from '../ValidationContext.js';
 export function VariablesAreInputTypesRule(
   context: ValidationContext,
 ): ASTVisitor {
-  return {
-    VariableDefinition(node: VariableDefinitionNode) {
-      const type = typeFromAST(context.getSchema(), node.type);
+  const validateNode = (
+    node: VariableDefinitionNode | FragmentArgumentDefinitionNode,
+  ) => {
+    const type = typeFromAST(context.getSchema(), node.type);
 
-      if (type !== undefined && !isInputType(type)) {
-        const variableName = node.variable.name.value;
-        const typeName = print(node.type);
+    if (type !== undefined && !isInputType(type)) {
+      const variableName = node.variable.name.value;
+      const typeName = print(node.type);
 
-        context.reportError(
-          new GraphQLError(
-            `Variable "$${variableName}" cannot be non-input type "${typeName}".`,
-            { nodes: node.type },
-          ),
-        );
-      }
-    },
+      context.reportError(
+        new GraphQLError(
+          `Variable "$${variableName}" cannot be non-input type "${typeName}".`,
+          { nodes: node.type },
+        ),
+      );
+    }
+  };
+  return {
+    VariableDefinition: validateNode,
+    FragmentArgumentDefinition: validateNode,
   };
 }
diff --git a/src/validation/rules/VariablesInAllowedPositionRule.ts b/src/validation/rules/VariablesInAllowedPositionRule.ts
index 4039540eba..6f4f013bff 100644
--- a/src/validation/rules/VariablesInAllowedPositionRule.ts
+++ b/src/validation/rules/VariablesInAllowedPositionRule.ts
@@ -1,9 +1,10 @@
 import { inspect } from '../../jsutils/inspect.js';
 import type { Maybe } from '../../jsutils/Maybe.js';
+import type { ObjMap } from '../../jsutils/ObjMap.js';
 
 import { GraphQLError } from '../../error/GraphQLError.js';
 
-import type { ValueNode } from '../../language/ast.js';
+import type { ValueNode, VariableDefinitionNode } from '../../language/ast.js';
 import { Kind } from '../../language/kinds.js';
 import type { ASTVisitor } from '../../language/visitor.js';
 
@@ -26,7 +27,7 @@ import type { ValidationContext } from '../ValidationContext.js';
 export function VariablesInAllowedPositionRule(
   context: ValidationContext,
 ): ASTVisitor {
-  let varDefMap = Object.create(null);
+  let varDefMap: ObjMap<VariableDefinitionNode> = Object.create(null);
 
   return {
     OperationDefinition: {
@@ -36,9 +37,9 @@ export function VariablesInAllowedPositionRule(
       leave(operation) {
         const usages = context.getRecursiveVariableUsages(operation);
 
-        for (const { node, type, defaultValue } of usages) {
+        for (const { node, type, defaultValue, fragmentArgDef } of usages) {
           const varName = node.name.value;
-          const varDef = varDefMap[varName];
+          const varDef = fragmentArgDef ?? varDefMap[varName];
           if (varDef && type) {
             // A var type is allowed if it is the same or more strict (e.g. is
             // a subtype of) than the expected type. It can be more strict if
diff --git a/src/validation/specifiedRules.ts b/src/validation/specifiedRules.ts
index 60c967f8f0..0911474c50 100644
--- a/src/validation/specifiedRules.ts
+++ b/src/validation/specifiedRules.ts
@@ -29,6 +29,8 @@ import { LoneSchemaDefinitionRule } from './rules/LoneSchemaDefinitionRule.js';
 import { NoFragmentCyclesRule } from './rules/NoFragmentCyclesRule.js';
 // Spec Section: "All Variable Used Defined"
 import { NoUndefinedVariablesRule } from './rules/NoUndefinedVariablesRule.js';
+// Spec Section: "All Fragment Arguments Used"
+import { NoUnusedFragmentArgumentsRule } from './rules/NoUnusedFragmentArgumentsRule.js';
 // Spec Section: "Fragments must be used"
 import { NoUnusedFragmentsRule } from './rules/NoUnusedFragmentsRule.js';
 // Spec Section: "All Variables Used"
@@ -99,6 +101,7 @@ export const specifiedRules: ReadonlyArray<ValidationRule> = Object.freeze([
   UniqueVariableNamesRule,
   NoUndefinedVariablesRule,
   NoUnusedVariablesRule,
+  NoUnusedFragmentArgumentsRule,
   KnownDirectivesRule,
   UniqueDirectivesPerLocationRule,
   DeferStreamDirectiveOnRootFieldRule,