Add require-import-fragment rule (#1443)

* Add `require-import-fragment` rule * Generate configs * Add changeset * Also allow locally defined fragments * Add suggestion * Pass only name node * Use `trimStart` instead of `trim` Co-authored-by: Dimitri POSTOLOV <[email protected]> * Adjust test case name * apply fixes * this is no needed * fix tests * some fixes * prettify * simplify * last refactor --------- Co-authored-by: Dimitri POSTOLOV <[email protected]> Co-authored-by: Dimitri POSTOLOV <[email protected]>
graphql-hive · Feb 9, 2023 · 9916d8d · 9916d8d
1 parent e21d142
commit 9916d8d
Show file tree

Hide file tree

Showing 18 changed files with 418 additions and 3 deletions.
diff --git a/.changeset/spotty-meals-think.md b/.changeset/spotty-meals-think.md
@@ -0,0 +1,6 @@
+---
+'@graphql-eslint/eslint-plugin': minor
+---
+
+Add new `require-import-fragment` rule that reports fragments which were not imported via an import
+expression.
diff --git a/packages/plugin/src/rules/index.ts b/packages/plugin/src/rules/index.ts
@@ -29,6 +29,7 @@ import { rule as requireDeprecationReason } from './require-deprecation-reason.j
 import { rule as requireDescription } from './require-description.js';
 import { rule as requireFieldOfTypeQueryInMutationResult } from './require-field-of-type-query-in-mutation-result.js';
 import { rule as requireIdWhenAvailable } from './require-id-when-available.js';
+import { rule as requireImportFragment } from './require-import-fragment.js';
 import { rule as requireNullableFieldsWithOneof } from './require-nullable-fields-with-oneof.js';
 import { rule as requireTypePatternWithOneof } from './require-type-pattern-with-oneof.js';
 import { rule as selectionSetDepth } from './selection-set-depth.js';
@@ -64,6 +65,7 @@ export const rules = {
   'require-description': requireDescription,
   'require-field-of-type-query-in-mutation-result': requireFieldOfTypeQueryInMutationResult,
   'require-id-when-available': requireIdWhenAvailable,
+  'require-import-fragment': requireImportFragment,
   'require-nullable-fields-with-oneof': requireNullableFieldsWithOneof,
   'require-type-pattern-with-oneof': requireTypePatternWithOneof,
   'selection-set-depth': selectionSetDepth,

diff --git a/packages/plugin/src/rules/lone-executable-definition.ts b/packages/plugin/src/rules/lone-executable-definition.ts
@@ -78,7 +78,7 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
           definitions.push({ type, node });
         }
       },
-      'Program:exit'() {
+      'Document:exit'() {
         for (const { node, type } of definitions.slice(1) /* ignore first definition */) {
           let name = pascalCase(type);
           const definitionName = node.name?.value;

diff --git a/packages/plugin/src/rules/require-import-fragment.ts b/packages/plugin/src/rules/require-import-fragment.ts
@@ -0,0 +1,128 @@
+import path from 'path';
+import { NameNode } from 'graphql';
+import { requireSiblingsOperations } from '../utils.js';
+import { GraphQLESTreeNode } from '../estree-converter/index.js';
+import { GraphQLESLintRule } from '../types.js';
+
+const RULE_ID = 'require-import-fragment';
+const SUGGESTION_ID = 'add-import-expression';
+
+export const rule: GraphQLESLintRule = {
+  meta: {
+    type: 'suggestion',
+    docs: {
+      category: 'Operations',
+      description: 'Require fragments to be imported via an import expression.',
+      url: `https://github.com/B2o5T/graphql-eslint/blob/master/docs/rules/${RULE_ID}.md`,
+      examples: [
+        {
+          title: 'Incorrect',
+          code: /* GraphQL */ `
+            query {
+              user {
+                ...UserFields
+              }
+            }
+          `,
+        },
+        {
+          title: 'Incorrect',
+          code: /* GraphQL */ `
+            # import 'post-fields.fragment.graphql'
+            query {
+              user {
+                ...UserFields
+              }
+            }
+          `,
+        },
+        {
+          title: 'Incorrect',
+          code: /* GraphQL */ `
+            # import UserFields from 'post-fields.fragment.graphql'
+            query {
+              user {
+                ...UserFields
+              }
+            }
+          `,
+        },
+        {
+          title: 'Correct',
+          code: /* GraphQL */ `
+            # import UserFields from 'user-fields.fragment.graphql'
+            query {
+              user {
+                ...UserFields
+              }
+            }
+          `,
+        },
+      ],
+      requiresSiblings: true,
+      isDisabledForAllConfig: true,
+    },
+    hasSuggestions: true,
+    messages: {
+      [RULE_ID]: 'Expected "{{fragmentName}}" fragment to be imported.',
+      [SUGGESTION_ID]: 'Add import expression for "{{fragmentName}}".',
+    },
+    schema: [],
+  },
+  create(context) {
+    const comments = context.getSourceCode().getAllComments();
+    const siblings = requireSiblingsOperations(RULE_ID, context);
+    const filePath = context.getFilename();
+
+    return {
+      'FragmentSpread > .name'(node: GraphQLESTreeNode<NameNode>) {
+        const fragmentName = node.value;
+        const fragmentsFromSiblings = siblings.getFragment(fragmentName);
+
+        for (const comment of comments) {
+          if (comment.type !== 'Line') continue;
+
+          // 1. could start with extra whitespace
+          // 2. match both named/default import
+          const isPossibleImported = new RegExp(
+            `^\\s*import\\s+(${fragmentName}\\s+from\\s+)?['"]`,
+          ).test(comment.value);
+          if (!isPossibleImported) continue;
+
+          const extractedImportPath = comment.value.match(/(["'])((?:\1|.)*?)\1/)?.[2];
+          if (!extractedImportPath) continue;
+
+          const importPath = path.join(path.dirname(filePath), extractedImportPath);
+          const hasInSiblings = fragmentsFromSiblings.some(
+            source => source.filePath === importPath,
+          );
+          if (hasInSiblings) return;
+        }
+
+        const fragmentInSameFile = fragmentsFromSiblings.some(
+          source => source.filePath === filePath,
+        );
+        if (fragmentInSameFile) return;
+
+        const suggestedFilePaths = fragmentsFromSiblings.length
+          ? fragmentsFromSiblings.map(o => path.relative(path.dirname(filePath), o.filePath))
+          : ['CHANGE_ME.graphql'];
+
+        context.report({
+          node,
+          messageId: RULE_ID,
+          data: { fragmentName },
+          suggest: suggestedFilePaths.map(suggestedPath => ({
+            messageId: SUGGESTION_ID,
+            data: { fragmentName },
+            fix: fixer =>
+              fixer.insertTextBeforeRange(
+                [0, 0],
+                `# import ${fragmentName} from '${suggestedPath}'\n`,
+              ),
+          })),
+        });
+      },
+    };
+  },
+};
diff --git a/packages/plugin/src/testkit.ts b/packages/plugin/src/testkit.ts
@@ -13,15 +13,15 @@ export type GraphQLESLintRuleListener<WithTypeInfo extends boolean = false> = Re
   [K in keyof ASTKindToNode]?: (node: GraphQLESTreeNode<ASTKindToNode[K], WithTypeInfo>) => void;
 };
 
-export type GraphQLValidTestCase<Options> = Omit<
+export type GraphQLValidTestCase<Options = []> = Omit<
   RuleTester.ValidTestCase,
   'options' | 'parserOptions'
 > & {
   options?: Options;
   parserOptions?: Omit<ParserOptions, 'filePath'>;
 };
 
-export type GraphQLInvalidTestCase<T> = GraphQLValidTestCase<T> & {
+export type GraphQLInvalidTestCase<T = []> = GraphQLValidTestCase<T> & {
   errors: (RuleTester.TestCaseError | string)[] | number;
   output?: string | null;
 };

diff --git a/packages/plugin/tests/__snapshots__/require-import-fragment.spec.md b/packages/plugin/tests/__snapshots__/require-import-fragment.spec.md
@@ -0,0 +1,83 @@
+// Vitest Snapshot v1
+
+exports[`should report fragments when there are no import expressions 1`] = `
+#### ⌨️ Code
+
+      1 | {
+      2 |   foo {
+      3 |     ...FooFields
+      4 |   }
+      5 | }
+
+#### ❌ Error
+
+      2 |   foo {
+    > 3 |     ...FooFields
+        |        ^^^^^^^^^ Expected "FooFields" fragment to be imported.
+      4 |   }
+
+#### 💡 Suggestion: Add import expression for "FooFields".
+
+    1 | # import FooFields from 'foo-fragment.gql'
+    2 | {
+    3 |   foo {
+    4 |     ...FooFields
+    5 |   }
+    6 | }
+`;
+
+exports[`should report with default import 1`] = `
+#### ⌨️ Code
+
+      1 | #import 'bar-fragment.gql'
+      2 | query {
+      3 |   foo {
+      4 |     ...FooFields
+      5 |   }
+      6 | }
+
+#### ❌ Error
+
+      3 |   foo {
+    > 4 |     ...FooFields
+        |        ^^^^^^^^^ Expected "FooFields" fragment to be imported.
+      5 |   }
+
+#### 💡 Suggestion: Add import expression for "FooFields".
+
+    1 | # import FooFields from 'foo-fragment.gql'
+    2 | #import 'bar-fragment.gql'
+    3 | query {
+    4 |   foo {
+    5 |     ...FooFields
+    6 |   }
+    7 | }
+`;
+
+exports[`should report with named import 1`] = `
+#### ⌨️ Code
+
+      1 | #import FooFields from "bar-fragment.gql"
+      2 | query {
+      3 |   foo {
+      4 |     ...FooFields
+      5 |   }
+      6 | }
+
+#### ❌ Error
+
+      3 |   foo {
+    > 4 |     ...FooFields
+        |        ^^^^^^^^^ Expected "FooFields" fragment to be imported.
+      5 |   }
+
+#### 💡 Suggestion: Add import expression for "FooFields".
+
+    1 | # import FooFields from 'foo-fragment.gql'
+    2 | #import FooFields from "bar-fragment.gql"
+    3 | query {
+    4 |   foo {
+    5 |     ...FooFields
+    6 |   }
+    7 | }
+`;
diff --git a/packages/plugin/tests/mocks/import-fragments/bar-fragment.gql b/packages/plugin/tests/mocks/import-fragments/bar-fragment.gql
@@ -0,0 +1,3 @@
+fragment BarFields on Bar {
+  id
+}
diff --git a/packages/plugin/tests/mocks/import-fragments/foo-fragment.gql b/packages/plugin/tests/mocks/import-fragments/foo-fragment.gql
@@ -0,0 +1,3 @@
+fragment FooFields on Foo {
+  id
+}
diff --git a/packages/plugin/tests/mocks/import-fragments/invalid-query-default.gql b/packages/plugin/tests/mocks/import-fragments/invalid-query-default.gql
@@ -0,0 +1,6 @@
+#import 'bar-fragment.gql'
+query {
+  foo {
+    ...FooFields
+  }
+}
diff --git a/packages/plugin/tests/mocks/import-fragments/invalid-query.gql b/packages/plugin/tests/mocks/import-fragments/invalid-query.gql
@@ -0,0 +1,6 @@
+#import FooFields from "bar-fragment.gql"
+query {
+  foo {
+    ...FooFields
+  }
+}
diff --git a/packages/plugin/tests/mocks/import-fragments/missing-import.gql b/packages/plugin/tests/mocks/import-fragments/missing-import.gql
@@ -0,0 +1,5 @@
+{
+  foo {
+    ...FooFields
+  }
+}
diff --git a/packages/plugin/tests/mocks/import-fragments/same-file.gql b/packages/plugin/tests/mocks/import-fragments/same-file.gql
@@ -0,0 +1,9 @@
+{
+  foo {
+    ...FooFields
+  }
+}
+
+fragment FooFields on Foo {
+  id
+}
diff --git a/packages/plugin/tests/mocks/import-fragments/valid-query-default.gql b/packages/plugin/tests/mocks/import-fragments/valid-query-default.gql
@@ -0,0 +1,9 @@
+# Imports could have extra whitespace and double/single quotes
+
+#  import  'foo-fragment.gql'
+
+query {
+  foo {
+    ...FooFields
+  }
+}
diff --git a/packages/plugin/tests/mocks/import-fragments/valid-query.gql b/packages/plugin/tests/mocks/import-fragments/valid-query.gql
@@ -0,0 +1,9 @@
+# Imports could have extra whitespace and double/single quotes
+
+#  import  FooFields  from  "foo-fragment.gql"
+
+query {
+  foo {
+    ...FooFields
+  }
+}
diff --git a/packages/plugin/tests/require-import-fragment.spec.ts b/packages/plugin/tests/require-import-fragment.spec.ts
@@ -0,0 +1,73 @@
+import { join } from 'node:path';
+import { GraphQLInvalidTestCase, GraphQLRuleTester } from '../src';
+import { rule } from '../src/rules/require-import-fragment';
+import { Linter } from 'eslint';
+import ParserOptions = Linter.ParserOptions;
+
+const ruleTester = new GraphQLRuleTester();
+
+function withMocks({
+  name,
+  filename,
+  errors,
+}: {
+  name: string;
+  filename: string;
+  errors?: GraphQLInvalidTestCase['errors'];
+}): {
+  name: string;
+  filename: string;
+  code: string;
+  parserOptions: {
+    documents: ParserOptions['documents'];
+  };
+  errors: any;
+} {
+  return {
+    name,
+    filename,
+    code: ruleTester.fromMockFile(filename.split('/mocks')[1]),
+    parserOptions: {
+      documents: [
+        filename,
+        join(__dirname, 'mocks/import-fragments/foo-fragment.gql'),
+        join(__dirname, 'mocks/import-fragments/bar-fragment.gql'),
+      ],
+    },
+    errors,
+  };
+}
+
+ruleTester.runGraphQLTests('require-import-fragment', rule, {
+  valid: [
+    withMocks({
+      name: 'should not report with named import',
+      filename: join(__dirname, 'mocks/import-fragments/valid-query.gql'),
+    }),
+    withMocks({
+      name: 'should not report with default import',
+      filename: join(__dirname, 'mocks/import-fragments/valid-query-default.gql'),
+    }),
+    withMocks({
+      name: 'should not report fragments from the same file',
+      filename: join(__dirname, 'mocks/import-fragments/same-file.gql'),
+    }),
+  ],
+  invalid: [
+    withMocks({
+      name: 'should report with named import',
+      filename: join(__dirname, 'mocks/import-fragments/invalid-query.gql'),
+      errors: [{ message: 'Expected "FooFields" fragment to be imported.' }],
+    }),
+    withMocks({
+      name: 'should report with default import',
+      filename: join(__dirname, 'mocks/import-fragments/invalid-query-default.gql'),
+      errors: [{ message: 'Expected "FooFields" fragment to be imported.' }],
+    }),
+    withMocks({
+      name: 'should report fragments when there are no import expressions',
+      filename: join(__dirname, 'mocks/import-fragments/missing-import.gql'),
+      errors: [{ message: 'Expected "FooFields" fragment to be imported.' }],
+    }),
+  ],
+});