fix require-id-when-available check unions as well (#1467)

* fix `require-id-when-available` check unions as well * fixes * fixes
dimaMachina · Feb 26, 2023 · 12f802c · 12f802c
1 parent 458c448
commit 12f802c
Show file tree

Hide file tree

Showing 61 changed files with 498 additions and 243 deletions.
diff --git a/.changeset/afraid-pugs-fail.md b/.changeset/afraid-pugs-fail.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': patch
+---
+
+fix `require-id-when-available` check unions as well
diff --git a/examples/basic/eslint.config.js b/examples/basic/eslint.config.js
@@ -1,7 +1,11 @@
 import * as graphqlESLint from '@graphql-eslint/eslint-plugin';
+import js from '@eslint/js';
 
 export default [
-  'eslint:recommended',
+  {
+    files: ['**/*.js'],
+    rules: js.configs.recommended.rules,
+  },
   {
     files: ['**/*.graphql'],
     plugins: {

diff --git a/examples/basic/package.json b/examples/basic/package.json
@@ -12,6 +12,7 @@
     "graphql": "16.6.0"
   },
   "devDependencies": {
+    "@eslint/js": "8.35.0",
     "@graphql-eslint/eslint-plugin": "workspace:*",
     "eslint": "8.33.0"
   }

diff --git a/examples/code-file/eslint.config.js b/examples/code-file/eslint.config.js
@@ -1,11 +1,12 @@
 import * as graphqlESLint from '@graphql-eslint/eslint-plugin';
+import js from '@eslint/js';
 
 export default [
-  'eslint:recommended',
   {
     files: ['**/*.js'],
     processor: graphqlESLint.processors.graphql,
     rules: {
+      ...js.configs.recommended.rules,
       'no-console': 'error',
     },
   },

diff --git a/examples/code-file/package.json b/examples/code-file/package.json
@@ -12,6 +12,7 @@
     "graphql": "16.6.0"
   },
   "devDependencies": {
+    "@eslint/js": "8.35.0",
     "@graphql-eslint/eslint-plugin": "workspace:*",
     "eslint": "8.33.0"
   }

diff --git a/examples/graphql-config-code-file/eslint.config.js b/examples/graphql-config-code-file/eslint.config.js
@@ -1,10 +1,11 @@
 import * as graphqlESLint from '@graphql-eslint/eslint-plugin';
+import js from '@eslint/js';
 
 export default [
-  'eslint:recommended',
   {
     files: ['**/*.js'],
     processor: graphqlESLint.processors.graphql,
+    rules: js.configs.recommended.rules,
   },
   {
     files: ['**/*.graphql'],

diff --git a/examples/graphql-config-code-file/package.json b/examples/graphql-config-code-file/package.json
@@ -13,6 +13,7 @@
     "graphql-tag": "2.12.6"
   },
   "devDependencies": {
+    "@eslint/js": "8.35.0",
     "@graphql-eslint/eslint-plugin": "workspace:*",
     "eslint": "8.33.0"
   }

diff --git a/examples/graphql-config/eslint.config.js b/examples/graphql-config/eslint.config.js
@@ -1,7 +1,11 @@
 import * as graphqlESLint from '@graphql-eslint/eslint-plugin';
+import js from '@eslint/js';
 
 export default [
-  'eslint:recommended',
+  {
+    files: ['**/*.js'],
+    rules: js.configs.recommended.rules,
+  },
   {
     files: ['**/*.graphql'],
     plugins: {

diff --git a/examples/graphql-config/package.json b/examples/graphql-config/package.json
@@ -12,6 +12,7 @@
     "graphql": "16.6.0"
   },
   "devDependencies": {
+    "@eslint/js": "8.35.0",
     "@graphql-eslint/eslint-plugin": "workspace:*",
     "eslint": "8.33.0"
   }

diff --git a/examples/monorepo/eslint.config.js b/examples/monorepo/eslint.config.js
@@ -1,10 +1,14 @@
 import * as graphqlESLint from '@graphql-eslint/eslint-plugin';
+import js from '@eslint/js';
 
 const SCHEMA_PATH = 'server/**/*.gql';
 const OPERATIONS_PATH = 'client/**/*.{tsx,gql}';
 
 export default [
-  'eslint:recommended',
+  {
+    files: ['**/*.{js,tsx}'],
+    rules: js.configs.recommended.rules,
+  },
   {
     files: ['client/**/*.tsx'],
     // Setup processor for operations/fragments definitions on code-files

diff --git a/examples/monorepo/package.json b/examples/monorepo/package.json
@@ -11,6 +11,7 @@
     "graphql": "16.6.0"
   },
   "devDependencies": {
+    "@eslint/js": "8.35.0",
     "@graphql-eslint/eslint-plugin": "workspace:*",
     "eslint": "8.33.0"
   }

diff --git a/examples/prettier/eslint.config.js b/examples/prettier/eslint.config.js
@@ -1,9 +1,9 @@
 import * as graphqlESLint from '@graphql-eslint/eslint-plugin';
 import prettierPlugin from 'eslint-plugin-prettier';
 import prettierConfig from 'eslint-config-prettier';
+import js from '@eslint/js';
 
 export default [
-  'eslint:recommended',
   {
     plugins: {
       prettier: prettierPlugin,
@@ -13,6 +13,7 @@ export default [
     files: ['**/*.js'],
     processor: graphqlESLint.processors.graphql,
     rules: {
+      ...js.configs.recommended.rules,
       ...prettierConfig.rules,
       ...prettierPlugin.configs.recommended.rules,
     },

diff --git a/examples/prettier/package.json b/examples/prettier/package.json
@@ -12,6 +12,7 @@
     "graphql": "16.6.0"
   },
   "devDependencies": {
+    "@eslint/js": "8.35.0",
     "@graphql-eslint/eslint-plugin": "workspace:*",
     "eslint": "8.33.0",
     "eslint-config-prettier": "8.6.0",

diff --git a/package.json b/package.json
@@ -31,7 +31,7 @@
     "chalk": "4.1.2",
     "dedent": "0.7.0",
     "enquirer": "2.3.6",
-    "eslint": "8.33.0",
+    "eslint": "8.35.0",
     "eslint-plugin-eslint-plugin": "5.0.7",
     "eslint-plugin-tailwindcss": "3.9.0",
     "husky": "8.0.3",
@@ -42,17 +42,17 @@
     "rimraf": "4.1.2",
     "tsx": "3.12.3",
     "typescript": "4.9.5",
-    "vitest": "0.28.4"
+    "vitest": "0.29.1"
   },
   "resolutions": {
     "graphql": "16.6.0"
   },
   "pnpm": {
     "patchedDependencies": {
-      "eslint@8.33.0": "patches/eslint@8.31.0.patch",
+      "eslint@8.35.0": "patches/eslint@8.31.0.patch",
       "eslint-plugin-eslint-plugin@5.0.7": "patches/eslint-plugin-eslint-plugin@5.0.6.patch",
       "json-schema-to-markdown@1.1.1": "patches/json-schema-to-markdown@1.1.1.patch",
-      "@vitest/runner@0.28.4": "patches/@vitest__runner@0.28.4.patch"
+      "@vitest/runner@0.29.1": "patches/@vitest__runner@0.28.4.patch"
     }
   }
 }
diff --git a/packages/plugin/src/rules/require-id-when-available.ts b/packages/plugin/src/rules/require-id-when-available.ts
@@ -5,6 +5,7 @@ import {
   GraphQLInterfaceType,
   GraphQLObjectType,
   GraphQLOutputType,
+  GraphQLUnionType,
   Kind,
   SelectionSetNode,
   TypeInfo,
@@ -165,83 +166,101 @@ export const rule: GraphQLESLintRule<RuleOptions, true> = {
       checkedFragmentSpreads = new Set<string>(),
     ): void {
       const rawType = getBaseType(type);
-      const isObjectType = rawType instanceof GraphQLObjectType;
-      const isInterfaceType = rawType instanceof GraphQLInterfaceType;
 
-      if (!isObjectType && !isInterfaceType) {
-        return;
+      if (rawType instanceof GraphQLObjectType || rawType instanceof GraphQLInterfaceType) {
+        checkFields(rawType);
+      } else if (rawType instanceof GraphQLUnionType) {
+        for (const selection of node.selections) {
+          if (selection.kind === Kind.INLINE_FRAGMENT) {
+            const types = rawType.getTypes();
+            const t = types.find(t => t.name === selection.typeCondition!.name.value);
+            if (t) {
+              checkFields(t);
+            }
+          }
+        }
       }
-      const fields = rawType.getFields();
-      const hasIdFieldInType = idNames.some(name => fields[name]);
 
-      if (!hasIdFieldInType) {
-        return;
-      }
+      function checkFields(rawType: GraphQLInterfaceType | GraphQLObjectType) {
+        const fields = rawType.getFields();
+        const hasIdFieldInType = idNames.some(name => fields[name]);
 
-      function hasIdField({ selections }: typeof node): boolean {
-        return selections.some(selection => {
-          if (selection.kind === Kind.FIELD) {
-            if (selection.alias && idNames.includes(selection.alias.value)) {
-              return true;
-            }
+        if (!hasIdFieldInType) {
+          return;
+        }
 
-            return idNames.includes(selection.name.value);
-          }
+        function hasIdField({ selections }: typeof node): boolean {
+          return selections.some(selection => {
+            if (selection.kind === Kind.FIELD) {
+              if (selection.alias && idNames.includes(selection.alias.value)) {
+                return true;
+              }
 
-          if (selection.kind === Kind.INLINE_FRAGMENT) {
-            return hasIdField(selection.selectionSet);
-          }
+              return idNames.includes(selection.name.value);
+            }
 
-          if (selection.kind === Kind.FRAGMENT_SPREAD) {
-            const [foundSpread] = siblings.getFragment(selection.name.value);
-            if (foundSpread) {
-              const fragmentSpread = foundSpread.document;
-              checkedFragmentSpreads.add(fragmentSpread.name.value);
-              return hasIdField(fragmentSpread.selectionSet);
+            if (selection.kind === Kind.INLINE_FRAGMENT) {
+              return hasIdField(selection.selectionSet);
             }
-          }
-          return false;
-        });
-      }
 
-      const hasId = hasIdField(node);
+            if (selection.kind === Kind.FRAGMENT_SPREAD) {
+              const [foundSpread] = siblings.getFragment(selection.name.value);
+              if (foundSpread) {
+                const fragmentSpread = foundSpread.document;
+                checkedFragmentSpreads.add(fragmentSpread.name.value);
+                return hasIdField(fragmentSpread.selectionSet);
+              }
+            }
+            return false;
+          });
+        }
+
+        const hasId = hasIdField(node);
 
-      checkFragments(node as GraphQLESTreeNode<SelectionSetNode>);
+        checkFragments(node as GraphQLESTreeNode<SelectionSetNode>);
 
-      if (hasId) {
-        return;
-      }
+        if (hasId) {
+          return;
+        }
 
-      const pluralSuffix = idNames.length > 1 ? 's' : '';
-      const fieldName = englishJoinWords(
-        idNames.map(name => `\`${(parent.alias || parent.name).value}.${name}\``),
-      );
+        const pluralSuffix = idNames.length > 1 ? 's' : '';
+        const fieldName = englishJoinWords(
+          idNames.map(name => `\`${(parent.alias || parent.name).value}.${name}\``),
+        );
 
-      const addition =
-        checkedFragmentSpreads.size === 0
-          ? ''
-          : ` or add to used fragment${
-              checkedFragmentSpreads.size > 1 ? 's' : ''
-            } ${englishJoinWords([...checkedFragmentSpreads].map(name => `\`${name}\``))}`;
+        const addition =
+          checkedFragmentSpreads.size === 0
+            ? ''
+            : ` or add to used fragment${
+                checkedFragmentSpreads.size > 1 ? 's' : ''
+              } ${englishJoinWords([...checkedFragmentSpreads].map(name => `\`${name}\``))}`;
 
-      const problem: ReportDescriptor = {
-        loc,
-        messageId: RULE_ID,
-        data: {
-          pluralSuffix,
-          fieldName,
-          addition,
-        },
-      };
+        const problem: ReportDescriptor = {
+          loc,
+          messageId: RULE_ID,
+          data: {
+            pluralSuffix,
+            fieldName,
+            addition,
+          },
+        };
 
-      // Don't provide suggestions for selections in fragments as fragment can be in a separate file
-      if ('type' in node) {
-        problem.suggest = idNames.map(idName => ({
-          desc: `Add \`${idName}\` selection`,
-          fix: fixer => fixer.insertTextBefore((node as any).selections[0], `${idName} `),
-        }));
+        // Don't provide suggestions for selections in fragments as fragment can be in a separate file
+        if ('type' in node) {
+          problem.suggest = idNames.map(idName => ({
+            desc: `Add \`${idName}\` selection`,
+            fix: fixer => {
+              let insertNode = (node as any).selections[0];
+              insertNode =
+                insertNode.kind === Kind.INLINE_FRAGMENT
+                  ? insertNode.selectionSet.selections[0]
+                  : insertNode;
+              return fixer.insertTextBefore(insertNode, `${idName} `);
+            },
+          }));
+        }
+        context.report(problem);
       }
-      context.report(problem);
     }
 
     return {

diff --git a/packages/plugin/tests/__snapshots__/alphabetize.spec.md b/packages/plugin/tests/__snapshots__/alphabetize.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`Invalid #1 1`] = `
 #### ⌨️ Code

diff --git a/packages/plugin/tests/__snapshots__/description-style.spec.md b/packages/plugin/tests/__snapshots__/description-style.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`Invalid #1 1`] = `
 #### ⌨️ Code

diff --git a/packages/plugin/tests/__snapshots__/eslint-directives.spec.md b/packages/plugin/tests/__snapshots__/eslint-directives.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`Invalid #1 1`] = `
 #### ⌨️ Code

diff --git a/packages/plugin/tests/__snapshots__/examples.spec.md b/packages/plugin/tests/__snapshots__/examples.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`Examples > should work in monorepo 1`] = `
 [

diff --git a/packages/plugin/tests/__snapshots__/executable-definitions.spec.md b/packages/plugin/tests/__snapshots__/executable-definitions.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`Invalid #1 1`] = `
 #### ⌨️ Code

diff --git a/packages/plugin/tests/__snapshots__/fields-on-correct-type.spec.md b/packages/plugin/tests/__snapshots__/fields-on-correct-type.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`should highlight selection on multi line 1`] = `
 #### ⌨️ Code

diff --git a/packages/plugin/tests/__snapshots__/input-name.spec.md b/packages/plugin/tests/__snapshots__/input-name.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`Invalid #1 1`] = `
 #### ⌨️ Code

diff --git a/packages/plugin/tests/__snapshots__/known-directives.spec.md b/packages/plugin/tests/__snapshots__/known-directives.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`should work only with Kind.FIELD 1`] = `
 #### ⌨️ Code

diff --git a/packages/plugin/tests/__snapshots__/known-fragment-names.spec.md b/packages/plugin/tests/__snapshots__/known-fragment-names.spec.md
@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`should not throw an error on undefined fragment 1`] = `
 #### ⌨️ Code