Skip to content

Commit 1b49833

Browse files
Rel1cxRel1cx
authored
refactor: improve error messages (#936)
1 parent ffbfa60 commit 1b49833

16 files changed

+172
-154
lines changed

‎apps/website/content/docs/rules/overview.mdx

+1-1
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ full: true
135135
| [`component-name`](naming-convention-component-name) | 0️⃣ | `🔍` `⚙️` | Enforces naming conventions for components. |
136136
| [`filename`](naming-convention-filename) | 0️⃣ | `🔍` `⚙️` | Enforces naming convention for JSX files. |
137137
| [`filename-extension`](naming-convention-filename-extension) | 0️⃣ | `🔍` `⚙️` | Enforces consistent use of the JSX file extension. |
138-
| [`use-state`](naming-convention-use-state) | 0️⃣ | `🔍` | Enforces destructuring and symmetric naming of `useState` hook value and setter variables. |
138+
| [`use-state`](naming-convention-use-state) | 0️⃣ | `🔍` | Enforces destructuring and symmetric naming of `useState` hook value and setter. |
139139

140140
## Debug Rules
141141

‎packages/plugins/eslint-plugin-react-naming-convention/src/rules/component-name.spec.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,21 @@ ruleTester.run(RULE_NAME, rule, {
55
invalid: [
66
{
77
code: /* tsx */ `<Test_component />`,
8-
errors: [{ messageId: "componentName" }],
8+
errors: [{ messageId: "usePascalCase" }],
99
},
1010
{
1111
code: /* tsx */ `<TestComponent />`,
12-
errors: [{ messageId: "componentName" }],
12+
errors: [{ messageId: "useConstantCase" }],
1313
options: [{ rule: "CONSTANT_CASE" }],
1414
},
1515
{
1616
code: /* tsx */ `<TestComponent />`,
17-
errors: [{ messageId: "componentName" }],
17+
errors: [{ messageId: "useConstantCase" }],
1818
options: ["CONSTANT_CASE"],
1919
},
2020
{
2121
code: /* tsx */ `<FULLUPPERCASE />`,
22-
errors: [{ messageId: "componentName" }],
22+
errors: [{ messageId: "usePascalCase" }],
2323
options: [{ allowAllCaps: false, rule: "PascalCase" }],
2424
},
2525
{
@@ -28,7 +28,7 @@ ruleTester.run(RULE_NAME, rule, {
2828
return <div>foo</div>
2929
}
3030
`,
31-
errors: [{ messageId: "componentName" }],
31+
errors: [{ messageId: "useConstantCase" }],
3232
options: [{ rule: "CONSTANT_CASE" }],
3333
},
3434
{
@@ -39,7 +39,7 @@ ruleTester.run(RULE_NAME, rule, {
3939
)
4040
}
4141
`,
42-
errors: [{ messageId: "componentName" }],
42+
errors: [{ messageId: "useConstantCase" }],
4343
options: [{ allowLeadingUnderscore: false, rule: "CONSTANT_CASE" }],
4444
},
4545
],

‎packages/plugins/eslint-plugin-react-naming-convention/src/rules/component-name.ts

+46-34
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import * as AST from "@eslint-react/ast";
22
import { useComponentCollector, useComponentCollectorLegacy } from "@eslint-react/core";
3-
import type { _ } from "@eslint-react/eff";
4-
import { constFalse } from "@eslint-react/eff";
3+
import { _ } from "@eslint-react/eff";
54
import * as JSX from "@eslint-react/jsx";
65
import type { RuleFeature } from "@eslint-react/shared";
76
import { RE_CONSTANT_CASE, RE_PASCAL_CASE } from "@eslint-react/shared";
87
import type { JSONSchema4 } from "@typescript-eslint/utils/json-schema";
9-
import type { CamelCase } from "string-ts";
108
import { match } from "ts-pattern";
119

1210
import { createRule } from "../utils";
@@ -18,7 +16,9 @@ export const RULE_FEATURES = [
1816
"CFG",
1917
] as const satisfies RuleFeature[];
2018

21-
export type MessageID = CamelCase<typeof RULE_NAME>;
19+
export type MessageID =
20+
| "usePascalCase"
21+
| "useConstantCase";
2222

2323
type Case = "CONSTANT_CASE" | "PascalCase";
2424

@@ -88,32 +88,43 @@ function normalizeOptions(options: Options) {
8888
} as const;
8989
}
9090

91-
function validate(name: string | _, options: ReturnType<typeof normalizeOptions>) {
92-
if (name == null) return false;
93-
if (options.excepts.some((regex) => regex.test(name))) {
94-
return true;
91+
function getViolationMessage(name: string | _, options: ReturnType<typeof normalizeOptions>): MessageID | _ {
92+
if (name == null) return _;
93+
const {
94+
allowAllCaps = false,
95+
allowLeadingUnderscore = false,
96+
allowNamespace = false,
97+
excepts,
98+
rule,
99+
} = options;
100+
if (excepts.some((regex) => regex.test(name))) {
101+
return _;
95102
}
96103
let normalized = name
97104
.normalize("NFKD")
98105
.replace(/[\u0300-\u036F]/g, "");
99106
normalized = normalized.split(".").at(-1) ?? normalized;
100-
const { allowLeadingUnderscore = false, allowNamespace = false } = options;
101107
if (allowNamespace) {
102108
normalized = normalized.replace(":", "");
103109
}
104110
if (allowLeadingUnderscore) {
105111
normalized = normalized.replace(/^_/, "");
106112
}
107-
return match(options.rule)
108-
.with("CONSTANT_CASE", () => RE_CONSTANT_CASE.test(normalized))
113+
return match(rule)
114+
.with("CONSTANT_CASE", () =>
115+
RE_CONSTANT_CASE.test(normalized)
116+
? _
117+
: "useConstantCase")
109118
.with("PascalCase", () => {
110119
// Allow all caps if the string is shorter than 4 characters. e.g. UI, CSS, SVG, etc.
111120
if (normalized.length > 3 && /^[A-Z]+$/u.test(normalized)) {
112-
return options.allowAllCaps ?? false;
121+
return allowAllCaps
122+
? _
123+
: "usePascalCase";
113124
}
114-
return RE_PASCAL_CASE.test(normalized);
125+
return RE_PASCAL_CASE.test(normalized) ? _ : "usePascalCase";
115126
})
116-
.otherwise(constFalse);
127+
.otherwise(() => _);
117128
}
118129

119130
export default createRule<Options, MessageID>({
@@ -124,7 +135,8 @@ export default createRule<Options, MessageID>({
124135
description: "enforce component naming convention to 'PascalCase' or 'CONSTANT_CASE'",
125136
},
126137
messages: {
127-
componentName: "A component name must be in {{case}}.",
138+
useConstantCase: "Component name '{{name}}' must be in CONSTANT_CASE.",
139+
usePascalCase: "Component name '{{name}}' must be in PascalCase.",
128140
},
129141
schema,
130142
},
@@ -143,14 +155,13 @@ export default createRule<Options, MessageID>({
143155
if (/^[a-z]/u.test(name)) {
144156
return;
145157
}
146-
if (validate(name, options)) {
147-
return;
148-
}
158+
const violation = getViolationMessage(name, options);
159+
if (violation == null) return;
149160
context.report({
150-
messageId: "componentName",
161+
messageId: violation,
151162
node,
152163
data: {
153-
case: options.rule,
164+
name,
154165
},
155166
});
156167
},
@@ -160,29 +171,30 @@ export default createRule<Options, MessageID>({
160171
for (const { node: component } of functionComponents.values()) {
161172
const id = AST.getFunctionIdentifier(component);
162173
if (id?.name == null) continue;
163-
if (validate(id.name, options)) {
164-
continue;
165-
}
174+
const name = id.name;
175+
const violation = getViolationMessage(name, options);
176+
if (violation == null) continue;
166177
context.report({
167-
messageId: "componentName",
178+
messageId: violation,
168179
node: id,
169180
data: {
170-
case: options.rule,
181+
name,
171182
},
172183
});
173184
}
174185
for (const { node: component } of classComponents.values()) {
175186
const id = AST.getClassIdentifier(component);
176187
if (id?.name == null) continue;
177-
if (!validate(id.name, options)) {
178-
context.report({
179-
messageId: "componentName",
180-
node: id,
181-
data: {
182-
case: options.rule,
183-
},
184-
});
185-
}
188+
const name = id.name;
189+
const violation = getViolationMessage(name, options);
190+
if (violation == null) continue;
191+
context.report({
192+
messageId: violation,
193+
node: id,
194+
data: {
195+
case: options.rule,
196+
},
197+
});
186198
}
187199
},
188200
};

‎packages/plugins/eslint-plugin-react-naming-convention/src/rules/filename-extension.spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ ruleTester.run(RULE_NAME, rule, {
1111
code: withoutJSX,
1212
errors: [
1313
{
14-
messageId: "filenameExtensionUnexpected",
14+
messageId: "useJsxFileExtensionAsNeeded",
1515
},
1616
],
1717
filename: "react.tsx",
@@ -21,7 +21,7 @@ ruleTester.run(RULE_NAME, rule, {
2121
code: withoutJSX,
2222
errors: [
2323
{
24-
messageId: "filenameExtensionUnexpected",
24+
messageId: "useJsxFileExtensionAsNeeded",
2525
},
2626
],
2727
filename: "react.tsx",
@@ -31,7 +31,7 @@ ruleTester.run(RULE_NAME, rule, {
3131
code: withJSXElement,
3232
errors: [
3333
{
34-
messageId: "filenameExtensionInvalid",
34+
messageId: "useJsxFileExtension",
3535
},
3636
],
3737
filename: "react.tsx",

‎packages/plugins/eslint-plugin-react-naming-convention/src/rules/filename-extension.ts

+13-6
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ export const RULE_FEATURES = [
1212
] as const satisfies RuleFeature[];
1313

1414
export type MessageID =
15-
| "filenameExtensionInvalid"
16-
| "filenameExtensionUnexpected";
15+
| "useJsxFileExtension"
16+
| "useJsxFileExtensionAsNeeded";
1717

1818
type Allow = "always" | "as-needed";
1919

@@ -74,8 +74,8 @@ export default createRule<Options, MessageID>({
7474
description: "enforce naming convention for JSX file extensions",
7575
},
7676
messages: {
77-
filenameExtensionInvalid: "The JSX file extension is required.",
78-
filenameExtensionUnexpected: "Use JSX file extension as needed.",
77+
useJsxFileExtension: "Use {{extensions}} file extension for JSX files.",
78+
useJsxFileExtensionAsNeeded: "Do not use {{extensions}} file extension for files without JSX.",
7979
},
8080
schema,
8181
},
@@ -86,6 +86,7 @@ export default createRule<Options, MessageID>({
8686
const extensions = isObject(options) && "extensions" in options
8787
? options.extensions
8888
: defaultOptions[0].extensions;
89+
const extensionsString = extensions.map((ext) => `'${ext}'`).join(", ");
8990
const filename = context.filename;
9091

9192
let hasJSXNode = false;
@@ -102,8 +103,11 @@ export default createRule<Options, MessageID>({
102103
const isJSXExt = extensions.includes(fileNameExt);
103104
if (hasJSXNode && !isJSXExt) {
104105
context.report({
105-
messageId: "filenameExtensionInvalid",
106+
messageId: "useJsxFileExtension",
106107
node,
108+
data: {
109+
extensions: extensionsString,
110+
},
107111
});
108112
return;
109113
}
@@ -119,8 +123,11 @@ export default createRule<Options, MessageID>({
119123
&& allow === "as-needed"
120124
) {
121125
context.report({
122-
messageId: "filenameExtensionUnexpected",
126+
messageId: "useJsxFileExtensionAsNeeded",
123127
node,
128+
data: {
129+
extensions: extensionsString,
130+
},
124131
});
125132
}
126133
},

‎packages/plugins/eslint-plugin-react-naming-convention/src/rules/use-state.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ react-naming-convention/use-state
2020

2121
## What it does
2222

23-
Enforces destructuring and symmetric naming of `useState` hook value and setter variables
23+
Enforces destructuring and symmetric naming of `useState` hook value and setter
2424

2525
## Examples
2626

‎packages/plugins/eslint-plugin-react-naming-convention/src/rules/use-state.spec.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ ruleTester.run(RULE_NAME, rule, {
1212
}
1313
`,
1414
errors: [{
15-
messageId: "useState",
15+
messageId: "unexpected",
1616
data: {
1717
setterName: "setState",
1818
stateName: "state",
@@ -28,7 +28,7 @@ ruleTester.run(RULE_NAME, rule, {
2828
}
2929
`,
3030
errors: [{
31-
messageId: "useState",
31+
messageId: "unexpected",
3232
data: {
3333
setterName: "setState",
3434
stateName: "state",
@@ -51,7 +51,7 @@ ruleTester.run(RULE_NAME, rule, {
5151
}
5252
`,
5353
errors: [{
54-
messageId: "useState",
54+
messageId: "unexpected",
5555
data: {
5656
setterName: "setState",
5757
stateName: "state",
@@ -69,7 +69,7 @@ ruleTester.run(RULE_NAME, rule, {
6969
}
7070
`,
7171
errors: [{
72-
messageId: "useState",
72+
messageId: "unexpected",
7373
data: {
7474
setterName: "setState",
7575
stateName: "state",
@@ -87,7 +87,7 @@ ruleTester.run(RULE_NAME, rule, {
8787
}
8888
`,
8989
errors: [{
90-
messageId: "useState",
90+
messageId: "unexpected",
9191
data: {
9292
setterName: "setState",
9393
stateName: "state",
@@ -105,7 +105,7 @@ ruleTester.run(RULE_NAME, rule, {
105105
}
106106
`,
107107
errors: [{
108-
messageId: "useState",
108+
messageId: "unexpected",
109109
data: {
110110
setterName: "setState",
111111
stateName: "state",

0 commit comments

Comments
 (0)