Skip to content

Commit 7f88174

Browse files
authored
improve error message for require-deprecation-reason (#1531)
* improve errors * better * improve error message * better * better * better * better * better * better * update vitest * prettier * add changeset * update snapshots
1 parent 3399b8c commit 7f88174

11 files changed

+178
-135
lines changed

.changeset/short-goats-provide.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-eslint/eslint-plugin': patch
3+
---
4+
5+
improve error message for require-deprecation-reason

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
"rimraf": "4.4.0",
4343
"tsx": "3.12.6",
4444
"typescript": "4.9.5",
45-
"vitest": "0.29.2"
45+
"vitest": "0.29.7"
4646
},
4747
"resolutions": {
4848
"graphql": "16.6.0"
@@ -52,7 +52,7 @@
5252
5353
5454
55-
"@vitest/[email protected].2": "patches/@[email protected]"
55+
"@vitest/[email protected].7": "patches/@[email protected]"
5656
}
5757
}
5858
}

packages/plugin/src/rules/require-deprecation-reason.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ArgumentNode, DirectiveNode } from 'graphql';
22
import { GraphQLESTreeNode, valueFromNode } from '../estree-converter/index.js';
33
import { GraphQLESLintRule } from '../types.js';
4+
import { getNodeName } from '../utils.js';
45

56
export const rule: GraphQLESLintRule = {
67
meta: {
@@ -50,7 +51,7 @@ export const rule: GraphQLESLintRule = {
5051
if (!value) {
5152
context.report({
5253
node: node.name,
53-
message: 'Directive "@deprecated" must have a reason!',
54+
message: `Deprecation reason is required for ${getNodeName(node.parent)}.`,
5455
});
5556
}
5657
},

packages/plugin/src/rules/require-description.ts

Lines changed: 8 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,14 @@ import { getRootTypeNames } from '@graphql-tools/utils';
22
import { ASTKindToNode, Kind, TokenKind } from 'graphql';
33
import { GraphQLESTreeNode } from '../estree-converter/index.js';
44
import { GraphQLESLintRule, ValueOf } from '../types.js';
5-
import { getLocation, requireGraphQLSchemaFromContext, TYPES_KINDS } from '../utils.js';
5+
import {
6+
getLocation,
7+
getNodeName,
8+
requireGraphQLSchemaFromContext,
9+
TYPES_KINDS,
10+
} from '../utils.js';
611

7-
const RULE_ID = 'require-description';
12+
export const RULE_ID = 'require-description';
813

914
const ALLOWED_KINDS = [
1015
...TYPES_KINDS,
@@ -19,36 +24,6 @@ type AllowedKind = (typeof ALLOWED_KINDS)[number];
1924
type AllowedKindToNode = Pick<ASTKindToNode, AllowedKind>;
2025
type SelectorNode = GraphQLESTreeNode<ValueOf<AllowedKindToNode>>;
2126

22-
function getNodeName(node: SelectorNode) {
23-
const DisplayNodeNameMap = {
24-
[Kind.OBJECT_TYPE_DEFINITION]: 'type',
25-
[Kind.INTERFACE_TYPE_DEFINITION]: 'interface',
26-
[Kind.ENUM_TYPE_DEFINITION]: 'enum',
27-
[Kind.SCALAR_TYPE_DEFINITION]: 'scalar',
28-
[Kind.INPUT_OBJECT_TYPE_DEFINITION]: 'input',
29-
[Kind.UNION_TYPE_DEFINITION]: 'union',
30-
[Kind.DIRECTIVE_DEFINITION]: 'directive',
31-
} as const;
32-
33-
switch (node.kind) {
34-
case Kind.OBJECT_TYPE_DEFINITION:
35-
case Kind.INTERFACE_TYPE_DEFINITION:
36-
case Kind.ENUM_TYPE_DEFINITION:
37-
case Kind.SCALAR_TYPE_DEFINITION:
38-
case Kind.INPUT_OBJECT_TYPE_DEFINITION:
39-
case Kind.UNION_TYPE_DEFINITION:
40-
return `${DisplayNodeNameMap[node.kind]} ${node.name.value}`;
41-
case Kind.DIRECTIVE_DEFINITION:
42-
return `${DisplayNodeNameMap[node.kind]} @${node.name.value}`;
43-
case Kind.FIELD_DEFINITION:
44-
case Kind.INPUT_VALUE_DEFINITION:
45-
case Kind.ENUM_VALUE_DEFINITION:
46-
return `${node.parent.name.value}.${node.name.value}`;
47-
case Kind.OPERATION_DEFINITION:
48-
return node.name ? `${node.operation} ${node.name.value}` : node.operation;
49-
}
50-
}
51-
5227
const schema = {
5328
type: 'array',
5429
minItems: 1,
@@ -157,7 +132,7 @@ export const rule: GraphQLESLintRule<RuleOptions> = {
157132
},
158133
type: 'suggestion',
159134
messages: {
160-
[RULE_ID]: 'Description is required for `{{ nodeName }}`.',
135+
[RULE_ID]: 'Description is required for {{ nodeName }}',
161136
},
162137
schema,
163138
},

packages/plugin/src/utils.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,37 @@ type Truthy<T> = T extends '' | 0 | false | null | undefined ? never : T; // fro
122122
export function truthy<T>(value: T): value is Truthy<T> {
123123
return !!value;
124124
}
125+
126+
export function getNodeName(node: any) {
127+
const DisplayNodeNameMap = {
128+
[Kind.OBJECT_TYPE_DEFINITION]: 'type',
129+
[Kind.INTERFACE_TYPE_DEFINITION]: 'interface',
130+
[Kind.ENUM_TYPE_DEFINITION]: 'enum',
131+
[Kind.SCALAR_TYPE_DEFINITION]: 'scalar',
132+
[Kind.INPUT_OBJECT_TYPE_DEFINITION]: 'input',
133+
[Kind.UNION_TYPE_DEFINITION]: 'union',
134+
[Kind.DIRECTIVE_DEFINITION]: 'directive',
135+
[Kind.FIELD_DEFINITION]: 'field',
136+
[Kind.ENUM_VALUE_DEFINITION]: 'value',
137+
[Kind.INPUT_VALUE_DEFINITION]: 'value',
138+
} as const;
139+
140+
switch (node.kind) {
141+
case Kind.OBJECT_TYPE_DEFINITION:
142+
case Kind.INTERFACE_TYPE_DEFINITION:
143+
case Kind.ENUM_TYPE_DEFINITION:
144+
case Kind.SCALAR_TYPE_DEFINITION:
145+
case Kind.INPUT_OBJECT_TYPE_DEFINITION:
146+
case Kind.UNION_TYPE_DEFINITION:
147+
case Kind.DIRECTIVE_DEFINITION:
148+
return `${DisplayNodeNameMap[node.kind]} "${node.name.value}"`;
149+
case Kind.FIELD_DEFINITION:
150+
case Kind.INPUT_VALUE_DEFINITION:
151+
case Kind.ENUM_VALUE_DEFINITION:
152+
return `${DisplayNodeNameMap[node.kind]} "${node.name.value}" in ${
153+
DisplayNodeNameMap[node.parent.kind]
154+
} "${node.parent.name.value}"`;
155+
case Kind.OPERATION_DEFINITION:
156+
return node.name ? `${node.operation} "${node.name.value}"` : node.operation;
157+
}
158+
}

packages/plugin/tests/__snapshots__/examples.spec.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ exports[`Examples > should work in monorepo 1`] = `
8585
endColumn: 10,
8686
endLine: 1,
8787
line: 1,
88-
message: Description is required for \`type Post\`.,
88+
message: Description is required for type "Post",
8989
messageId: require-description,
9090
nodeType: null,
9191
ruleId: @graphql-eslint/require-description,
@@ -101,7 +101,7 @@ exports[`Examples > should work in monorepo 1`] = `
101101
endColumn: 11,
102102
endLine: 1,
103103
line: 1,
104-
message: Description is required for \`type Query\`.,
104+
message: Description is required for type "Query",
105105
messageId: require-description,
106106
nodeType: null,
107107
ruleId: @graphql-eslint/require-description,
@@ -117,7 +117,7 @@ exports[`Examples > should work in monorepo 1`] = `
117117
endColumn: 16,
118118
endLine: 1,
119119
line: 1,
120-
message: Description is required for \`scalar DateTime\`.,
120+
message: Description is required for scalar "DateTime",
121121
messageId: require-description,
122122
nodeType: null,
123123
ruleId: @graphql-eslint/require-description,
@@ -133,7 +133,7 @@ exports[`Examples > should work in monorepo 1`] = `
133133
endColumn: 10,
134134
endLine: 1,
135135
line: 1,
136-
message: Description is required for \`type User\`.,
136+
message: Description is required for type "User",
137137
messageId: require-description,
138138
nodeType: null,
139139
ruleId: @graphql-eslint/require-description,
@@ -361,7 +361,7 @@ exports[`Examples > should work on \`.graphql\` files 1`] = `
361361
endColumn: 7,
362362
endLine: 2,
363363
line: 2,
364-
message: Description is required for \`Query.user\`.,
364+
message: Description is required for field "user" in type "Query",
365365
messageId: require-description,
366366
nodeType: null,
367367
ruleId: @graphql-eslint/require-description,
@@ -372,7 +372,7 @@ exports[`Examples > should work on \`.graphql\` files 1`] = `
372372
endColumn: 5,
373373
endLine: 6,
374374
line: 6,
375-
message: Description is required for \`User.id\`.,
375+
message: Description is required for field "id" in type "User",
376376
messageId: require-description,
377377
nodeType: null,
378378
ruleId: @graphql-eslint/require-description,
@@ -383,7 +383,7 @@ exports[`Examples > should work on \`.graphql\` files 1`] = `
383383
endColumn: 7,
384384
endLine: 7,
385385
line: 7,
386-
message: Description is required for \`User.name\`.,
386+
message: Description is required for field "name" in type "User",
387387
messageId: require-description,
388388
nodeType: null,
389389
ruleId: @graphql-eslint/require-description,

packages/plugin/tests/__snapshots__/require-deprecation-reason.spec.md

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,51 +9,71 @@ exports[`Invalid #1 1`] = `
99
4 | notDeprecated: String
1010
5 | }
1111
6 |
12-
7 | enum testEnum {
12+
7 | enum TestEnum {
1313
8 | item1 @deprecated
1414
9 | item2 @deprecated(reason: "Reason")
1515
10 | }
1616
11 |
17-
12 | interface testInterface {
17+
12 | interface TestInterface {
1818
13 | item1: String @deprecated
1919
14 | item2: Number @deprecated(reason: "Reason")
2020
15 | item3: String
2121
16 | item4: String @deprecated(reason: "")
2222
17 | item5: String @deprecated(reason: " ")
2323
18 | }
24+
19 |
25+
20 | type MyQuery @deprecated
26+
21 |
27+
22 | input MyInput {
28+
23 | foo: String! @deprecated
29+
24 | }
2430

25-
#### ❌ Error 1/5
31+
#### ❌ Error 1/7
2632

2733
1 | type A {
2834
> 2 | deprecatedWithoutReason: String @deprecated
29-
| ^^^^^^^^^^ Directive "@deprecated" must have a reason!
35+
| ^^^^^^^^^^ Deprecation reason is required for field "deprecatedWithoutReason" in type "A".
3036
3 | deprecatedWithReason: String @deprecated(reason: "Reason")
3137

32-
#### ❌ Error 2/5
38+
#### ❌ Error 2/7
3339

34-
7 | enum testEnum {
40+
7 | enum TestEnum {
3541
> 8 | item1 @deprecated
36-
| ^^^^^^^^^^ Directive "@deprecated" must have a reason!
42+
| ^^^^^^^^^^ Deprecation reason is required for value "item1" in enum "TestEnum".
3743
9 | item2 @deprecated(reason: "Reason")
3844

39-
#### ❌ Error 3/5
45+
#### ❌ Error 3/7
4046

41-
12 | interface testInterface {
47+
12 | interface TestInterface {
4248
> 13 | item1: String @deprecated
43-
| ^^^^^^^^^^ Directive "@deprecated" must have a reason!
49+
| ^^^^^^^^^^ Deprecation reason is required for field "item1" in interface "TestInterface".
4450
14 | item2: Number @deprecated(reason: "Reason")
4551

46-
#### ❌ Error 4/5
52+
#### ❌ Error 4/7
4753

4854
15 | item3: String
4955
> 16 | item4: String @deprecated(reason: "")
50-
| ^^^^^^^^^^ Directive "@deprecated" must have a reason!
56+
| ^^^^^^^^^^ Deprecation reason is required for field "item4" in interface "TestInterface".
5157
17 | item5: String @deprecated(reason: " ")
5258

53-
#### ❌ Error 5/5
59+
#### ❌ Error 5/7
5460

5561
16 | item4: String @deprecated(reason: "")
5662
> 17 | item5: String @deprecated(reason: " ")
57-
| ^^^^^^^^^^ Directive "@deprecated" must have a reason!
63+
| ^^^^^^^^^^ Deprecation reason is required for field "item5" in interface "TestInterface".
5864
18 | }
65+
66+
#### ❌ Error 6/7
67+
68+
19 |
69+
> 20 | type MyQuery @deprecated
70+
| ^^^^^^^^^^ Deprecation reason is required for type "MyQuery".
71+
21 |
72+
73+
#### ❌ Error 7/7
74+
75+
22 | input MyInput {
76+
> 23 | foo: String! @deprecated
77+
| ^^^^^^^^^^ Deprecation reason is required for value "foo" in input "MyInput".
78+
24 | }
5979
`;

0 commit comments

Comments
 (0)