required-fields: inline fragments without a field ancestor (#240)

We are planning to use the `required-fields` rule to enforce the `id` field but we have some code that was causing `TypeError: Cannot read property 'selectionSet' of undefined` errors.

I tracked down the issue to the case where an inline fragment is missing a required field but does not have a field ancestor. The existing code for the rule will walk up the ancestors until it finds a field, but an inline fragment can be part of any selection set and not only a field (in our case a fragment definition). This example based on the unit tests should demonstrate the issue:

```graphql
fragment GreetingOrStoryFragment on GreetingOrStory {
  ... on Greetings {
    hello
  }
}
```

The `id` field is missing from the inline fragment My fix was just to walk up the ancestors until it fields the first field, operation definition or fragment definition. It basically stops when it finds something that contains a selection set with the exception of an inline fragment, which we intentionally keep walking up.

TODO:

- [x] Make sure all of the significant new logic is covered by tests
- [x] Rebase your changes on master so that they can be merged easily
- [x] Make sure all tests pass
- [x] Update CHANGELOG.md with your change
- [x] If this was a change that affects the external API, update the README

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ### vNEXT
 
+- Update the `required-fields` rule to handle inline fragments without field ancestors. [PR #240](https://github.com/apollographql/eslint-plugin-graphql/pull/240) by [Henry Q. Dineen](https://github.com/henryqdineen)
+
 ### v3.1.0
 
 - Fix an issue that caused `graphql/required-fields` to throw on non-existent field references. [PR #231](https://github.com/apollographql/eslint-plugin-graphql/pull/231) by [Vitor Balocco](https://github.com/vitorbal)

src/customGraphQLValidationRules.js

@@ -65,11 +65,11 @@ export function RequiredFields(context, options) {
 
           const ancestorClone = [...ancestors];
 
-          let nearestField;
+          let nearestFieldOrExecutableDefinition;
           let nextAncestor;
 
-          // Now, walk up the ancestors, until you see a field.
-          while (!nearestField) {
+          // Now, walk up the ancestors, until you see a field or executable definition.
+          while (!nearestFieldOrExecutableDefinition) {
             nextAncestor = ancestorClone.pop();
 
             if (
@@ -79,19 +79,23 @@ export function RequiredFields(context, options) {
               return true;
             }
 
-            if (nextAncestor.kind === "Field") {
-              nearestField = nextAncestor;
+            if (
+              nextAncestor.kind === "Field" ||
+              nextAncestor.kind === "FragmentDefinition" ||
+              nextAncestor.kind === "OperationDefiniton"
+            ) {
+              nearestFieldOrExecutableDefinition = nextAncestor;
             }
           }
 
-          // If we never found a field, the query is malformed
-          if (!nearestField) {
+          // If we never found a field or executable definition, the query is malformed
+          if (!nearestFieldOrExecutableDefinition) {
             throw new Error(
-              "Inline fragment found inside document without a parent field."
+              "Inline fragment found inside document without a parent field, fragment definition, or operation definition."
             );
           }
 
-          // We found a field, but we never saw the field we were looking for in
+          // We found a field or executable definition, but we never saw the field we were looking for in
           // the intermediate selection sets.
           context.reportError(
             new GraphQLError(

test/validationRules/required-fields.js

@@ -10,6 +10,10 @@ const requiredFieldsTestCases = {
     "const x = gql`query { greetings { id, hello, hi } }`",
     "const x = gql`query { greetings { id, hello, foo } }`",
     "const x = gql`query { greetings { id ... on Greetings { hello } } }`",
+    "const x = gql`query { greetingOrStory { ... on Greetings { id hello } } }`",
+    "const x = gql`query { greetingOrStory { id ... on Greetings { hello } } }`",
+    "const x = gql`fragment Name on GreetingOrStory { ... on Greetings { id hello } }`",
+    "const x = gql`fragment Name on GreetingOrStory { id ... on Greetings { hello } }`",
     "const x = gql`fragment Name on Greetings { id hello }`",
     "const x = gql`fragment Foo on FooBar { id, hello, foo }`",
     "const x = gql`fragment Id on Node { id ... on NodeA { fieldA } }`",
@@ -82,6 +86,16 @@ const requiredFieldsTestCases = {
         }
       ]
     },
+    {
+      code:
+        "const x = gql`fragment Name on GreetingOrStory { ... on Greetings { hello } }`",
+      errors: [
+        {
+          message: `'id' field required on '... on Greetings'`,
+          type: "TaggedTemplateExpression"
+        }
+      ]
+    },
   ]
 };