Skip to content

refactor: improve error messages #936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/website/content/docs/rules/overview.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ full: true
| [`component-name`](naming-convention-component-name) | 0️⃣ | `🔍` `⚙️` | Enforces naming conventions for components. |
| [`filename`](naming-convention-filename) | 0️⃣ | `🔍` `⚙️` | Enforces naming convention for JSX files. |
| [`filename-extension`](naming-convention-filename-extension) | 0️⃣ | `🔍` `⚙️` | Enforces consistent use of the JSX file extension. |
| [`use-state`](naming-convention-use-state) | 0️⃣ | `🔍` | Enforces destructuring and symmetric naming of `useState` hook value and setter variables. |
| [`use-state`](naming-convention-use-state) | 0️⃣ | `🔍` | Enforces destructuring and symmetric naming of `useState` hook value and setter. |

## Debug Rules

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@ ruleTester.run(RULE_NAME, rule, {
invalid: [
{
code: /* tsx */ `<Test_component />`,
errors: [{ messageId: "componentName" }],
errors: [{ messageId: "usePascalCase" }],
},
{
code: /* tsx */ `<TestComponent />`,
errors: [{ messageId: "componentName" }],
errors: [{ messageId: "useConstantCase" }],
options: [{ rule: "CONSTANT_CASE" }],
},
{
code: /* tsx */ `<TestComponent />`,
errors: [{ messageId: "componentName" }],
errors: [{ messageId: "useConstantCase" }],
options: ["CONSTANT_CASE"],
},
{
code: /* tsx */ `<FULLUPPERCASE />`,
errors: [{ messageId: "componentName" }],
errors: [{ messageId: "usePascalCase" }],
options: [{ allowAllCaps: false, rule: "PascalCase" }],
},
{
Expand All @@ -28,7 +28,7 @@ ruleTester.run(RULE_NAME, rule, {
return <div>foo</div>
}
`,
errors: [{ messageId: "componentName" }],
errors: [{ messageId: "useConstantCase" }],
options: [{ rule: "CONSTANT_CASE" }],
},
{
Expand All @@ -39,7 +39,7 @@ ruleTester.run(RULE_NAME, rule, {
)
}
`,
errors: [{ messageId: "componentName" }],
errors: [{ messageId: "useConstantCase" }],
options: [{ allowLeadingUnderscore: false, rule: "CONSTANT_CASE" }],
},
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import * as AST from "@eslint-react/ast";
import { useComponentCollector, useComponentCollectorLegacy } from "@eslint-react/core";
import type { _ } from "@eslint-react/eff";
import { constFalse } from "@eslint-react/eff";
import { _ } from "@eslint-react/eff";
import * as JSX from "@eslint-react/jsx";
import type { RuleFeature } from "@eslint-react/shared";
import { RE_CONSTANT_CASE, RE_PASCAL_CASE } from "@eslint-react/shared";
import type { JSONSchema4 } from "@typescript-eslint/utils/json-schema";
import type { CamelCase } from "string-ts";
import { match } from "ts-pattern";

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

export type MessageID = CamelCase<typeof RULE_NAME>;
export type MessageID =
| "usePascalCase"
| "useConstantCase";

type Case = "CONSTANT_CASE" | "PascalCase";

Expand Down Expand Up @@ -88,32 +88,43 @@ function normalizeOptions(options: Options) {
} as const;
}

function validate(name: string | _, options: ReturnType<typeof normalizeOptions>) {
if (name == null) return false;
if (options.excepts.some((regex) => regex.test(name))) {
return true;
function getViolationMessage(name: string | _, options: ReturnType<typeof normalizeOptions>): MessageID | _ {
if (name == null) return _;
const {
allowAllCaps = false,
allowLeadingUnderscore = false,
allowNamespace = false,
excepts,
rule,
} = options;
if (excepts.some((regex) => regex.test(name))) {
return _;
}
let normalized = name
.normalize("NFKD")
.replace(/[\u0300-\u036F]/g, "");
normalized = normalized.split(".").at(-1) ?? normalized;
const { allowLeadingUnderscore = false, allowNamespace = false } = options;
if (allowNamespace) {
normalized = normalized.replace(":", "");
}
if (allowLeadingUnderscore) {
normalized = normalized.replace(/^_/, "");
}
return match(options.rule)
.with("CONSTANT_CASE", () => RE_CONSTANT_CASE.test(normalized))
return match(rule)
.with("CONSTANT_CASE", () =>
RE_CONSTANT_CASE.test(normalized)
? _
: "useConstantCase")
.with("PascalCase", () => {
// Allow all caps if the string is shorter than 4 characters. e.g. UI, CSS, SVG, etc.
if (normalized.length > 3 && /^[A-Z]+$/u.test(normalized)) {
return options.allowAllCaps ?? false;
return allowAllCaps
? _
: "usePascalCase";
}
return RE_PASCAL_CASE.test(normalized);
return RE_PASCAL_CASE.test(normalized) ? _ : "usePascalCase";
})
.otherwise(constFalse);
.otherwise(() => _);
}

export default createRule<Options, MessageID>({
Expand All @@ -124,7 +135,8 @@ export default createRule<Options, MessageID>({
description: "enforce component naming convention to 'PascalCase' or 'CONSTANT_CASE'",
},
messages: {
componentName: "A component name must be in {{case}}.",
useConstantCase: "Component name '{{name}}' must be in CONSTANT_CASE.",
usePascalCase: "Component name '{{name}}' must be in PascalCase.",
},
schema,
},
Expand All @@ -143,14 +155,13 @@ export default createRule<Options, MessageID>({
if (/^[a-z]/u.test(name)) {
return;
}
if (validate(name, options)) {
return;
}
const violation = getViolationMessage(name, options);
if (violation == null) return;
context.report({
messageId: "componentName",
messageId: violation,
node,
data: {
case: options.rule,
name,
},
});
},
Expand All @@ -160,29 +171,30 @@ export default createRule<Options, MessageID>({
for (const { node: component } of functionComponents.values()) {
const id = AST.getFunctionIdentifier(component);
if (id?.name == null) continue;
if (validate(id.name, options)) {
continue;
}
const name = id.name;
const violation = getViolationMessage(name, options);
if (violation == null) continue;
context.report({
messageId: "componentName",
messageId: violation,
node: id,
data: {
case: options.rule,
name,
},
});
}
for (const { node: component } of classComponents.values()) {
const id = AST.getClassIdentifier(component);
if (id?.name == null) continue;
if (!validate(id.name, options)) {
context.report({
messageId: "componentName",
node: id,
data: {
case: options.rule,
},
});
}
const name = id.name;
const violation = getViolationMessage(name, options);
if (violation == null) continue;
context.report({
messageId: violation,
node: id,
data: {
case: options.rule,
},
});
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ ruleTester.run(RULE_NAME, rule, {
code: withoutJSX,
errors: [
{
messageId: "filenameExtensionUnexpected",
messageId: "useJsxFileExtensionAsNeeded",
},
],
filename: "react.tsx",
Expand All @@ -21,7 +21,7 @@ ruleTester.run(RULE_NAME, rule, {
code: withoutJSX,
errors: [
{
messageId: "filenameExtensionUnexpected",
messageId: "useJsxFileExtensionAsNeeded",
},
],
filename: "react.tsx",
Expand All @@ -31,7 +31,7 @@ ruleTester.run(RULE_NAME, rule, {
code: withJSXElement,
errors: [
{
messageId: "filenameExtensionInvalid",
messageId: "useJsxFileExtension",
},
],
filename: "react.tsx",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export const RULE_FEATURES = [
] as const satisfies RuleFeature[];

export type MessageID =
| "filenameExtensionInvalid"
| "filenameExtensionUnexpected";
| "useJsxFileExtension"
| "useJsxFileExtensionAsNeeded";

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

Expand Down Expand Up @@ -74,8 +74,8 @@ export default createRule<Options, MessageID>({
description: "enforce naming convention for JSX file extensions",
},
messages: {
filenameExtensionInvalid: "The JSX file extension is required.",
filenameExtensionUnexpected: "Use JSX file extension as needed.",
useJsxFileExtension: "Use {{extensions}} file extension for JSX files.",
useJsxFileExtensionAsNeeded: "Do not use {{extensions}} file extension for files without JSX.",
},
schema,
},
Expand All @@ -86,6 +86,7 @@ export default createRule<Options, MessageID>({
const extensions = isObject(options) && "extensions" in options
? options.extensions
: defaultOptions[0].extensions;
const extensionsString = extensions.map((ext) => `'${ext}'`).join(", ");
const filename = context.filename;

let hasJSXNode = false;
Expand All @@ -102,8 +103,11 @@ export default createRule<Options, MessageID>({
const isJSXExt = extensions.includes(fileNameExt);
if (hasJSXNode && !isJSXExt) {
context.report({
messageId: "filenameExtensionInvalid",
messageId: "useJsxFileExtension",
node,
data: {
extensions: extensionsString,
},
});
return;
}
Expand All @@ -119,8 +123,11 @@ export default createRule<Options, MessageID>({
&& allow === "as-needed"
) {
context.report({
messageId: "filenameExtensionUnexpected",
messageId: "useJsxFileExtensionAsNeeded",
node,
data: {
extensions: extensionsString,
},
});
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ react-naming-convention/use-state

## What it does

Enforces destructuring and symmetric naming of `useState` hook value and setter variables
Enforces destructuring and symmetric naming of `useState` hook value and setter

## Examples

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ruleTester.run(RULE_NAME, rule, {
}
`,
errors: [{
messageId: "useState",
messageId: "unexpected",
data: {
setterName: "setState",
stateName: "state",
Expand All @@ -28,7 +28,7 @@ ruleTester.run(RULE_NAME, rule, {
}
`,
errors: [{
messageId: "useState",
messageId: "unexpected",
data: {
setterName: "setState",
stateName: "state",
Expand All @@ -51,7 +51,7 @@ ruleTester.run(RULE_NAME, rule, {
}
`,
errors: [{
messageId: "useState",
messageId: "unexpected",
data: {
setterName: "setState",
stateName: "state",
Expand All @@ -69,7 +69,7 @@ ruleTester.run(RULE_NAME, rule, {
}
`,
errors: [{
messageId: "useState",
messageId: "unexpected",
data: {
setterName: "setState",
stateName: "state",
Expand All @@ -87,7 +87,7 @@ ruleTester.run(RULE_NAME, rule, {
}
`,
errors: [{
messageId: "useState",
messageId: "unexpected",
data: {
setterName: "setState",
stateName: "state",
Expand All @@ -105,7 +105,7 @@ ruleTester.run(RULE_NAME, rule, {
}
`,
errors: [{
messageId: "useState",
messageId: "unexpected",
data: {
setterName: "setState",
stateName: "state",
Expand Down
Loading