[RFC] CCN: Validation

src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts

@@ -1049,6 +1049,137 @@ describe('Validate: Overlapping fields can be merged', () => {
       );
     });
 
+    it('allows non-conflicting overlapping required statuses', () => {
+      expectValidationErrorsWithSchema(
+        schema,
+        OverlappingFieldsCanBeMergedRule,
+        `
+          {
+            someBox {
+              ... on IntBox {
+                scalar: unrelatedField!
+              }
+              ... on StringBox {
+                scalar!
+              }
+            }
+          }
+        `,
+        {
+          experimentalClientControlledNullability: true,
+        },
+      );
+
+      expectValidationErrorsWithSchema(
+        schema,
+        OverlappingFieldsCanBeMergedRule,
+        `
+          {
+            someBox {
+              ... on IntBox {
+                scalar: unrelatedField?
+              }
+              ... on StringBox {
+                scalar?
+              }
+            }
+          }
+        `,
+        {
+          experimentalClientControlledNullability: true,
+        },
+      );
+    });
+
+    it('disallows conflicting overlapping required statuses', () => {
+      expectValidationErrorsWithSchema(
+        schema,
+        OverlappingFieldsCanBeMergedRule,
+        `
+          {
+            someBox {
+              ... on IntBox {
+                scalar: unrelatedField!
+              }
+              ... on StringBox {
+                scalar
+              }
+            }
+          }
+        `,
+        {
+          experimentalClientControlledNullability: true,
+        },
+      ).toDeepEqual([
+        {
+          message:
+            'Fields "scalar" conflict because they have conflicting nullability designators "!" and "". Use different aliases on the fields to fetch both if this was intentional.',
+          locations: [
+            { line: 5, column: 17 },
+            { line: 8, column: 17 },
+          ],
+        },
+      ]);
+
+      expectValidationErrorsWithSchema(
+        schema,
+        OverlappingFieldsCanBeMergedRule,
+        `
+          {
+            someBox {
+              ... on IntBox {
+                scalar: unrelatedField!
+              }
+              ... on StringBox {
+                scalar?
+              }
+            }
+          }
+        `,
+        {
+          experimentalClientControlledNullability: true,
+        },
+      ).toDeepEqual([
+        {
+          message:
+            'Fields "scalar" conflict because they have conflicting nullability designators "!" and "?". Use different aliases on the fields to fetch both if this was intentional.',
+          locations: [
+            { line: 5, column: 17 },
+            { line: 8, column: 17 },
+          ],
+        },
+      ]);
+
+      expectValidationErrorsWithSchema(
+        schema,
+        OverlappingFieldsCanBeMergedRule,
+        `
+          {
+            someBox {
+              ... on IntBox {
+                scalar: unrelatedField
+              }
+              ... on StringBox {
+                scalar?
+              }
+            }
+          }
+        `,
+        {
+          experimentalClientControlledNullability: true,
+        },
+      ).toDeepEqual([
+        {
+          message:
+            'Fields "scalar" conflict because they have conflicting nullability designators "" and "?". Use different aliases on the fields to fetch both if this was intentional.',
+          locations: [
+            { line: 5, column: 17 },
+            { line: 8, column: 17 },
+          ],
+        },
+      ]);
+    });
+
     it('same wrapped scalar return types', () => {
       expectValidWithSchema(
         schema,

src/validation/__tests__/RequiredStatusOnFieldMatchesDefinitionRule-test.ts

@@ -0,0 +1,83 @@
+import { describe, it } from 'mocha';
+
+import { buildSchema } from '../../utilities/buildASTSchema.js';
+
+import { RequiredStatusOnFieldMatchesDefinitionRule } from '../rules/RequiredStatusOnFieldMatchesDefinitionRule.js';
+
+import { expectValidationErrorsWithSchema } from './harness.js';
+
+function expectErrors(queryStr: string) {
+  return expectValidationErrorsWithSchema(
+    testSchema,
+    RequiredStatusOnFieldMatchesDefinitionRule,
+    queryStr,
+    { experimentalClientControlledNullability: true },
+  );
+}
+
+function expectValid(queryStr: string) {
+  expectErrors(queryStr).toDeepEqual([]);
+}
+
+const testSchema = buildSchema(`
+  type Lists {
+    nonList: Int
+    list: [Int]
+    requiredList: [Int]!
+    mixedThreeDList: [[[Int]]]
+  } 
+  type Query {
+    lists: Lists
+  }
+`);
+
+describe('Validate: Field uses correct list depth', () => {
+  it('Fields are valid', () => {
+    expectValid(`
+      fragment listFragment on Lists {
+        list[!]
+        nonList!
+        nonList?
+        mixedThreeDList[[[!]!]!]!
+        requiredList[]
+        unmodifiedList: list
+      }
+    `);
+  });
+
+  it('reports errors when list depth is too high', () => {
+    expectErrors(`
+      fragment listFragment on Lists {
+        notAList: nonList[!]
+        list[[]]
+      }
+    `).toDeepEqual([
+      {
+        message: 'List nullability modifier is too deep.',
+        locations: [{ line: 3, column: 26 }],
+      },
+      {
+        message: 'List nullability modifier is too deep.',
+        locations: [{ line: 4, column: 13 }],
+      },
+    ]);
+  });
+
+  it('reports errors when list depth is too low', () => {
+    expectErrors(`
+      fragment listFragment on Lists {
+        list!
+        mixedThreeDList[[]!]!
+      }
+    `).toDeepEqual([
+      {
+        message: 'List nullability modifier is too shallow.',
+        locations: [{ line: 3, column: 13 }],
+      },
+      {
+        message: 'List nullability modifier is too shallow.',
+        locations: [{ line: 4, column: 24 }],
+      },
+    ]);
+  });
+});

src/validation/__tests__/harness.ts

@@ -2,6 +2,7 @@ import { expectJSON } from '../../__testUtils__/expectJSON.js';
 
 import type { Maybe } from '../../jsutils/Maybe.js';
 
+import type { ParseOptions } from '../../language/parser.js';
 import { parse } from '../../language/parser.js';
 
 import type { GraphQLSchema } from '../../type/schema.js';
@@ -122,17 +123,24 @@ export function expectValidationErrorsWithSchema(
   schema: GraphQLSchema,
   rule: ValidationRule,
   queryStr: string,
+  parseOptions?: ParseOptions,
 ): any {
-  const doc = parse(queryStr);
+  const doc = parse(queryStr, parseOptions);
   const errors = validate(schema, doc, [rule]);
   return expectJSON(errors);
 }
 
 export function expectValidationErrors(
   rule: ValidationRule,
   queryStr: string,
+  parseOptions?: ParseOptions,
 ): any {
-  return expectValidationErrorsWithSchema(testSchema, rule, queryStr);
+  return expectValidationErrorsWithSchema(
+    testSchema,
+    rule,
+    queryStr,
+    parseOptions,
+  );
 }
 
 export function expectSDLValidationErrors(

src/validation/rules/OverlappingFieldsCanBeMergedRule.ts

@@ -617,17 +617,40 @@ function findConflict(
   const type1 = def1?.type;
   const type2 = def2?.type;
 
-  if (type1 && type2 && doTypesConflict(type1, type2)) {
-    return [
-      [
-        responseName,
-        `they return conflicting types "${inspect(type1)}" and "${inspect(
-          type2,
-        )}"`,
-      ],
-      [node1],
-      [node2],
-    ];
+  if (type1 && type2) {
+    // Two fields have different types
+    if (doTypesConflict(type1, type2)) {
+      return [
+        [
+          responseName,
+          `they return conflicting types "${inspect(type1)}" and "${inspect(
+            type2,
+          )}"`,
+        ],
+        [node1],
+        [node2],
+      ];
+    }
+
+    // Two fields have different required operators
+    if (node1.nullabilityAssertion !== node2.nullabilityAssertion) {
+      return [
+        [
+          responseName,
+          `they have conflicting nullability designators "${
+            node1.nullabilityAssertion === undefined
+              ? ''
+              : print(node1.nullabilityAssertion)
+          }" and "${
+            node2.nullabilityAssertion === undefined
+              ? ''
+              : print(node2.nullabilityAssertion)
+          }"`,
+        ],
+        [node1],
+        [node2],
+      ];
+    }
   }
 
   // Collect and compare sub-fields. Use the same "visited fragment names" list