Skip to content

Commit 9916d8d

Browse files
FloEdelmannDimitri POSTOLOVdimaMachina
authored
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]>
1 parent e21d142 commit 9916d8d

18 files changed

+418
-3
lines changed

‎.changeset/spotty-meals-think.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@graphql-eslint/eslint-plugin': minor
3+
---
4+
5+
Add new `require-import-fragment` rule that reports fragments which were not imported via an import
6+
expression.

‎packages/plugin/src/rules/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { rule as requireDeprecationReason } from './require-deprecation-reason.j
2929
import { rule as requireDescription } from './require-description.js';
3030
import { rule as requireFieldOfTypeQueryInMutationResult } from './require-field-of-type-query-in-mutation-result.js';
3131
import { rule as requireIdWhenAvailable } from './require-id-when-available.js';
32+
import { rule as requireImportFragment } from './require-import-fragment.js';
3233
import { rule as requireNullableFieldsWithOneof } from './require-nullable-fields-with-oneof.js';
3334
import { rule as requireTypePatternWithOneof } from './require-type-pattern-with-oneof.js';
3435
import { rule as selectionSetDepth } from './selection-set-depth.js';
@@ -64,6 +65,7 @@ export const rules = {
6465
'require-description': requireDescription,
6566
'require-field-of-type-query-in-mutation-result': requireFieldOfTypeQueryInMutationResult,
6667
'require-id-when-available': requireIdWhenAvailable,
68+
'require-import-fragment': requireImportFragment,
6769
'require-nullable-fields-with-oneof': requireNullableFieldsWithOneof,
6870
'require-type-pattern-with-oneof': requireTypePatternWithOneof,
6971
'selection-set-depth': selectionSetDepth,

‎packages/plugin/src/rules/lone-executable-definition.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
7878
definitions.push({ type, node });
7979
}
8080
},
81-
'Program:exit'() {
81+
'Document:exit'() {
8282
for (const { node, type } of definitions.slice(1) /* ignore first definition */) {
8383
let name = pascalCase(type);
8484
const definitionName = node.name?.value;

‎packages/plugin/src/rules/require-import-fragment.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import path from 'path';
2+
import { NameNode } from 'graphql';
3+
import { requireSiblingsOperations } from '../utils.js';
4+
import { GraphQLESTreeNode } from '../estree-converter/index.js';
5+
import { GraphQLESLintRule } from '../types.js';
6+
7+
const RULE_ID = 'require-import-fragment';
8+
const SUGGESTION_ID = 'add-import-expression';
9+
10+
export const rule: GraphQLESLintRule = {
11+
meta: {
12+
type: 'suggestion',
13+
docs: {
14+
category: 'Operations',
15+
description: 'Require fragments to be imported via an import expression.',
16+
url: `https://github.com/B2o5T/graphql-eslint/blob/master/docs/rules/${RULE_ID}.md`,
17+
examples: [
18+
{
19+
title: 'Incorrect',
20+
code: /* GraphQL */ `
21+
query {
22+
user {
23+
...UserFields
24+
}
25+
}
26+
`,
27+
},
28+
{
29+
title: 'Incorrect',
30+
code: /* GraphQL */ `
31+
# import 'post-fields.fragment.graphql'
32+
query {
33+
user {
34+
...UserFields
35+
}
36+
}
37+
`,
38+
},
39+
{
40+
title: 'Incorrect',
41+
code: /* GraphQL */ `
42+
# import UserFields from 'post-fields.fragment.graphql'
43+
query {
44+
user {
45+
...UserFields
46+
}
47+
}
48+
`,
49+
},
50+
{
51+
title: 'Correct',
52+
code: /* GraphQL */ `
53+
# import UserFields from 'user-fields.fragment.graphql'
54+
query {
55+
user {
56+
...UserFields
57+
}
58+
}
59+
`,
60+
},
61+
],
62+
requiresSiblings: true,
63+
isDisabledForAllConfig: true,
64+
},
65+
hasSuggestions: true,
66+
messages: {
67+
[RULE_ID]: 'Expected "{{fragmentName}}" fragment to be imported.',
68+
[SUGGESTION_ID]: 'Add import expression for "{{fragmentName}}".',
69+
},
70+
schema: [],
71+
},
72+
create(context) {
73+
const comments = context.getSourceCode().getAllComments();
74+
const siblings = requireSiblingsOperations(RULE_ID, context);
75+
const filePath = context.getFilename();
76+
77+
return {
78+
'FragmentSpread > .name'(node: GraphQLESTreeNode<NameNode>) {
79+
const fragmentName = node.value;
80+
const fragmentsFromSiblings = siblings.getFragment(fragmentName);
81+
82+
for (const comment of comments) {
83+
if (comment.type !== 'Line') continue;
84+
85+
// 1. could start with extra whitespace
86+
// 2. match both named/default import
87+
const isPossibleImported = new RegExp(
88+
`^\\s*import\\s+(${fragmentName}\\s+from\\s+)?['"]`,
89+
).test(comment.value);
90+
if (!isPossibleImported) continue;
91+
92+
const extractedImportPath = comment.value.match(/(["'])((?:\1|.)*?)\1/)?.[2];
93+
if (!extractedImportPath) continue;
94+
95+
const importPath = path.join(path.dirname(filePath), extractedImportPath);
96+
const hasInSiblings = fragmentsFromSiblings.some(
97+
source => source.filePath === importPath,
98+
);
99+
if (hasInSiblings) return;
100+
}
101+
102+
const fragmentInSameFile = fragmentsFromSiblings.some(
103+
source => source.filePath === filePath,
104+
);
105+
if (fragmentInSameFile) return;
106+
107+
const suggestedFilePaths = fragmentsFromSiblings.length
108+
? fragmentsFromSiblings.map(o => path.relative(path.dirname(filePath), o.filePath))
109+
: ['CHANGE_ME.graphql'];
110+
111+
context.report({
112+
node,
113+
messageId: RULE_ID,
114+
data: { fragmentName },
115+
suggest: suggestedFilePaths.map(suggestedPath => ({
116+
messageId: SUGGESTION_ID,
117+
data: { fragmentName },
118+
fix: fixer =>
119+
fixer.insertTextBeforeRange(
120+
[0, 0],
121+
`# import ${fragmentName} from '${suggestedPath}'\n`,
122+
),
123+
})),
124+
});
125+
},
126+
};
127+
},
128+
};

‎packages/plugin/src/testkit.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@ export type GraphQLESLintRuleListener<WithTypeInfo extends boolean = false> = Re
1313
[K in keyof ASTKindToNode]?: (node: GraphQLESTreeNode<ASTKindToNode[K], WithTypeInfo>) => void;
1414
};
1515

16-
export type GraphQLValidTestCase<Options> = Omit<
16+
export type GraphQLValidTestCase<Options = []> = Omit<
1717
RuleTester.ValidTestCase,
1818
'options' | 'parserOptions'
1919
> & {
2020
options?: Options;
2121
parserOptions?: Omit<ParserOptions, 'filePath'>;
2222
};
2323

24-
export type GraphQLInvalidTestCase<T> = GraphQLValidTestCase<T> & {
24+
export type GraphQLInvalidTestCase<T = []> = GraphQLValidTestCase<T> & {
2525
errors: (RuleTester.TestCaseError | string)[] | number;
2626
output?: string | null;
2727
};

‎packages/plugin/tests/__snapshots__/require-import-fragment.spec.md

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Vitest Snapshot v1
2+
3+
exports[`should report fragments when there are no import expressions 1`] = `
4+
#### ⌨️ Code
5+
6+
1 | {
7+
2 | foo {
8+
3 | ...FooFields
9+
4 | }
10+
5 | }
11+
12+
#### ❌ Error
13+
14+
2 | foo {
15+
> 3 | ...FooFields
16+
| ^^^^^^^^^ Expected "FooFields" fragment to be imported.
17+
4 | }
18+
19+
#### 💡 Suggestion: Add import expression for "FooFields".
20+
21+
1 | # import FooFields from 'foo-fragment.gql'
22+
2 | {
23+
3 | foo {
24+
4 | ...FooFields
25+
5 | }
26+
6 | }
27+
`;
28+
29+
exports[`should report with default import 1`] = `
30+
#### ⌨️ Code
31+
32+
1 | #import 'bar-fragment.gql'
33+
2 | query {
34+
3 | foo {
35+
4 | ...FooFields
36+
5 | }
37+
6 | }
38+
39+
#### ❌ Error
40+
41+
3 | foo {
42+
> 4 | ...FooFields
43+
| ^^^^^^^^^ Expected "FooFields" fragment to be imported.
44+
5 | }
45+
46+
#### 💡 Suggestion: Add import expression for "FooFields".
47+
48+
1 | # import FooFields from 'foo-fragment.gql'
49+
2 | #import 'bar-fragment.gql'
50+
3 | query {
51+
4 | foo {
52+
5 | ...FooFields
53+
6 | }
54+
7 | }
55+
`;
56+
57+
exports[`should report with named import 1`] = `
58+
#### ⌨️ Code
59+
60+
1 | #import FooFields from "bar-fragment.gql"
61+
2 | query {
62+
3 | foo {
63+
4 | ...FooFields
64+
5 | }
65+
6 | }
66+
67+
#### ❌ Error
68+
69+
3 | foo {
70+
> 4 | ...FooFields
71+
| ^^^^^^^^^ Expected "FooFields" fragment to be imported.
72+
5 | }
73+
74+
#### 💡 Suggestion: Add import expression for "FooFields".
75+
76+
1 | # import FooFields from 'foo-fragment.gql'
77+
2 | #import FooFields from "bar-fragment.gql"
78+
3 | query {
79+
4 | foo {
80+
5 | ...FooFields
81+
6 | }
82+
7 | }
83+
`;

‎packages/plugin/tests/mocks/import-fragments/bar-fragment.gql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
fragment BarFields on Bar {
2+
id
3+
}

‎packages/plugin/tests/mocks/import-fragments/foo-fragment.gql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
fragment FooFields on Foo {
2+
id
3+
}

‎packages/plugin/tests/mocks/import-fragments/invalid-query-default.gql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#import 'bar-fragment.gql'
2+
query {
3+
foo {
4+
...FooFields
5+
}
6+
}

‎packages/plugin/tests/mocks/import-fragments/invalid-query.gql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#import FooFields from "bar-fragment.gql"
2+
query {
3+
foo {
4+
...FooFields
5+
}
6+
}

0 commit comments

Comments
 (0)