Skip to content
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
21 changes: 14 additions & 7 deletions eslint-factory/src/rules/prefer-get-error-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,19 @@ describe("prefer-get-error-message", () => {
});
});

it("invalid: basic ternary is flagged with suggestion", () => {
it("valid: ternary is not flagged when getErrorMessage is unavailable", () => {
cjsRuleTester.run("prefer-get-error-message", preferGetErrorMessageRule, {
valid: [`const errorMessage = err instanceof Error ? err.message : String(err);`],
invalid: [],
});
});

it("invalid: ternary is flagged with suggestion when getErrorMessage is previously imported", () => {
cjsRuleTester.run("prefer-get-error-message", preferGetErrorMessageRule, {
valid: [],
invalid: [
{
code: `const errorMessage = err instanceof Error ? err.message : String(err);`,
code: `const { getErrorMessage } = require("./error_helpers.cjs"); const errorMessage = err instanceof Error ? err.message : String(err);`,
errors: [
{
messageId: "preferGetErrorMessage",
Expand All @@ -87,7 +94,7 @@ describe("prefer-get-error-message", () => {
{
messageId: "replaceWithGetErrorMessage",
data: { errorVar: "err" },
output: `const errorMessage = getErrorMessage(err);`,
output: `const { getErrorMessage } = require("./error_helpers.cjs"); const errorMessage = getErrorMessage(err);`,
},
],
},
Expand All @@ -102,7 +109,7 @@ describe("prefer-get-error-message", () => {
valid: [],
invalid: [
{
code: "core.warning(`failed: ${readErr instanceof Error ? readErr.message : String(readErr)}`);",
code: 'const { getErrorMessage } = require("./error_helpers.cjs"); core.warning(`failed: ${readErr instanceof Error ? readErr.message : String(readErr)}`);',
errors: [
{
messageId: "preferGetErrorMessage",
Expand All @@ -111,7 +118,7 @@ describe("prefer-get-error-message", () => {
{
messageId: "replaceWithGetErrorMessage",
data: { errorVar: "readErr" },
output: "core.warning(`failed: ${getErrorMessage(readErr)}`);",
output: 'const { getErrorMessage } = require("./error_helpers.cjs"); core.warning(`failed: ${getErrorMessage(readErr)}`);',
},
],
},
Expand All @@ -126,7 +133,7 @@ describe("prefer-get-error-message", () => {
valid: [],
invalid: [
{
code: `console.error(e instanceof Error ? e.message : String(e));`,
code: `import { getErrorMessage } from "./error_helpers.cjs"; console.error(e instanceof Error ? e.message : String(e));`,
errors: [
{
messageId: "preferGetErrorMessage",
Expand All @@ -135,7 +142,7 @@ describe("prefer-get-error-message", () => {
{
messageId: "replaceWithGetErrorMessage",
data: { errorVar: "e" },
output: `console.error(getErrorMessage(e));`,
output: `import { getErrorMessage } from "./error_helpers.cjs"; console.error(getErrorMessage(e));`,
},
],
},
Expand Down
27 changes: 26 additions & 1 deletion eslint-factory/src/rules/prefer-get-error-message.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AST_NODE_TYPES, ESLintUtils, TSESTree } from "@typescript-eslint/utils";
import { AST_NODE_TYPES, ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils";

const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`);

Expand All @@ -22,6 +22,30 @@ export const preferGetErrorMessageRule = createRule({
},
defaultOptions: [],
create(context) {
const sourceCode = context.sourceCode;
type Scope = ReturnType<typeof sourceCode.getScope>;

function isDefinitionAvailableAtNode(definition: TSESLint.Scope.Definition, node: TSESTree.Node): boolean {
if (definition.type === "ImportBinding" || definition.type === "FunctionName") {
return true;
}
const definitionNode = definition.name ?? definition.node;
if (!definitionNode?.range || !node.range) return false;
return definitionNode.range[0] < node.range[0];
}

function hasResolvableLocalBinding(node: TSESTree.Node, name: string): boolean {
let scope: Scope | null = sourceCode.getScope(node);
while (scope) {
const variable = scope.set.get(name);
if (variable && variable.defs.some(def => isDefinitionAvailableAtNode(def, node))) {
return true;
}
scope = scope.upper;
}
Comment on lines +40 to +45
return false;
}

return {
ConditionalExpression(node) {
const test = node.test;
Expand All @@ -38,6 +62,7 @@ export const preferGetErrorMessageRule = createRule({
if (alternate.type !== AST_NODE_TYPES.CallExpression || !isIdentifierNamed(alternate.callee, "String") || alternate.arguments.length !== 1 || !isIdentifierNamed(alternate.arguments[0], errorVar)) {
return;
}
if (!hasResolvableLocalBinding(node, "getErrorMessage")) return;

context.report({
node,
Expand Down
Loading