feat: no-throw-default-error (#48)

A new rule to enforce using custom error types instead of the default error object. <img width="802" alt="image" src="https://github.com/user-attachments/assets/b5e09529-7123-491a-af4c-942c5394e5d3"> Related to aws/aws-cdk#32324
cdklabs · Nov 29, 2024 · c0faf5d · c0faf5d
1 parent 56098b2
commit c0faf5d
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 3 deletions.
diff --git a/src/index.ts b/src/index.ts
@@ -10,5 +10,6 @@ export const rules = {
   'invalid-cfn-imports': require('./rules/invalid-cfn-imports'),
   'no-literal-partition': require('./rules/no-literal-partition'),
   'no-invalid-path': require('./rules/no-invalid-path'),
+  'no-throw-default-error': require('./rules/no-throw-default-error'),
   'promiseall-no-unbounded-parallelism': require('./rules/promiseall-no-unbounded-parallelism'),
 };
diff --git a/src/rules/no-throw-default-error.ts b/src/rules/no-throw-default-error.ts
@@ -0,0 +1,52 @@
+import { Rule } from 'eslint';
+import type { NewExpression, ThrowStatement } from 'estree';
+
+export const meta = {
+  hasSuggestions: true,
+};
+
+export function create(context: Rule.RuleContext): Rule.NodeListener {
+  return {
+    ThrowStatement(node: ThrowStatement) {
+      if (node.argument.type !== 'NewExpression') {
+        return;
+      }
+
+      const newExpr = node.argument as NewExpression;
+      if (
+        newExpr.callee &&
+        newExpr.callee.type === 'Identifier' &&
+        newExpr.callee.name === 'Error'
+      ) {
+        context.report({
+          node: newExpr,
+          message: 'Expected a non-default error object to be thrown.',
+          suggest: [
+            {
+              desc: 'Replace with `ValidationError`',
+              fix: (fixer: Rule.RuleFixer) => {
+                // no existing args
+                if (newExpr.arguments.length === 0) {
+                  return fixer.replaceText(newExpr, "new ValidationError('<insert error message>', this)");
+                }
+
+                const fixes = [
+                  fixer.replaceText(newExpr.callee, 'ValidationError'),
+                ];
+
+                const last = newExpr.arguments.at(-1)?.range;
+                if (last) {
+                  fixes.push(
+                    fixer.insertTextAfterRange(last, ', this'),
+                  );
+                }
+
+                return fixes;
+              },
+            },
+          ],
+        });
+      }
+    },
+  };
+}
diff --git a/test/rules/fixtures.test.ts b/test/rules/fixtures.test.ts
@@ -15,7 +15,12 @@ const fixturesRoot = path.join(__dirname, 'fixtures');
 fs.readdirSync(fixturesRoot).filter(f => f !== 'lib').filter(f => fs.lstatSync(path.join(fixturesRoot, f)).isDirectory()).forEach(d => {
   describe(d, () => {
     const fixturesDir = path.join(fixturesRoot, d);
-    const fixtureFiles = fs.readdirSync(fixturesDir).filter(f => f.endsWith('.ts') && !f.endsWith('.expected.ts') && f !== 'index.ts');
+    const fixtureFiles = fs.readdirSync(fixturesDir).filter(f =>
+      f.endsWith('.ts')
+      && !f.endsWith('.expected.ts')
+      && !f.endsWith('.suggested.ts')
+      && f !== 'index.ts',
+    );
 
     if (!fixtureFiles.length) {
       return;
@@ -43,11 +48,13 @@ fs.readdirSync(fixturesRoot).filter(f => f !== 'lib').filter(f => fs.lstatSync(p
       it(f, async () => {
         const originalFilePath = path.join(fixturesDir, f);
         const expectedFixedFilePath = path.join(fixturesDir, `${path.basename(f, '.ts')}.expected.ts`);
+        const suggestedFixFilePath = path.join(fixturesDir, `${path.basename(f, '.ts')}.suggested.ts`);
         const expectedErrorFilepath = path.join(fixturesDir, `${path.basename(f, '.ts')}.error.txt`);
         const fix = fs.existsSync(expectedFixedFilePath);
+        const suggestion = fs.existsSync(suggestedFixFilePath);
         const checkErrors = fs.existsSync(expectedErrorFilepath);
-        if (fix && checkErrors) {
-          fail(`Expected only a fixed file or an expected error message file. Both ${expectedFixedFilePath} and ${expectedErrorFilepath} are present.`);
+        if (fix && checkErrors && suggestion) {
+          fail(`Expected only a fixed file or an expected error message file. Multiple of ${expectedFixedFilePath}, ${suggestedFixFilePath} and ${expectedErrorFilepath} are present.`);
         } else if (fix) {
           const actualFile = await lintAndFix(originalFilePath, outputDir);
           const actual = await fs.readFile(actualFile, { encoding: 'utf8' });
@@ -56,6 +63,14 @@ fs.readdirSync(fixturesRoot).filter(f => f !== 'lib').filter(f => fs.lstatSync(p
             fail(`Linted file did not match expectations.\n--------- Expected ----------\n${expected}\n---------- Actual ----------\n${actual}`);
           }
           return;
+        } else if (suggestion) {
+          const actualFile = await lintAndApplySuggestion(originalFilePath, outputDir);
+          const actual = await fs.readFile(actualFile, { encoding: 'utf8' });
+          const expected = await fs.readFile(suggestedFixFilePath, { encoding: 'utf8' });
+          if (actual !== expected) {
+            fail(`Linted file did not match expectations.\n--------- Expected ----------\n${expected}\n---------- Actual ----------\n${actual}`);
+          }
+          return;
         } else if (checkErrors) {
           const actualErrorMessages = await lint(originalFilePath);
           const expectedErrorMessages = (await fs.readFile(expectedErrorFilepath, { encoding: 'utf8' })).split('\n');
@@ -92,6 +107,25 @@ async function lintAndFix(file: string, outputDir: string) {
   return newPath;
 }
 
+async function lintAndApplySuggestion(file: string, outputDir: string) {
+  const newPath = path.join(outputDir, path.basename(file));
+  let result = await linter.lintFiles(file);
+  const hasSuggestions = result.find(r => r.messages.find(m => m.suggestions?.length)) !== undefined;
+  if (hasSuggestions) {
+    for (const r of result) {
+      const suggestion = r.messages.find(m => m.suggestions?.length)?.suggestions?.[0];
+      if (suggestion && r.source) {
+        const output = r.source.slice(0, suggestion.fix.range[0]) + suggestion.fix.text + r.source.slice(suggestion.fix.range[1]);
+        await fs.writeFile(newPath, output, { encoding: 'utf8' });
+      }
+    }
+  } else {
+    // If there are no fixes, copy the input file as output
+    await fs.copyFile(file, newPath);
+  }
+  return newPath;
+}
+
 async function lint(file: string) {
   const result = await linter.lintFiles(file);
   // If you only lint one file, then result.length will always be one.

diff --git a/test/rules/fixtures/no-throw-default-error/error-no-args.suggested.ts b/test/rules/fixtures/no-throw-default-error/error-no-args.suggested.ts
@@ -0,0 +1 @@
+throw new ValidationError('<insert error message>', this);
diff --git a/test/rules/fixtures/no-throw-default-error/error-no-args.ts b/test/rules/fixtures/no-throw-default-error/error-no-args.ts
@@ -0,0 +1 @@
+throw new Error();
diff --git a/test/rules/fixtures/no-throw-default-error/error-one-arg.suggested.ts b/test/rules/fixtures/no-throw-default-error/error-one-arg.suggested.ts
@@ -0,0 +1 @@
+throw new ValidationError('My error', this);
diff --git a/test/rules/fixtures/no-throw-default-error/error-one-arg.ts b/test/rules/fixtures/no-throw-default-error/error-one-arg.ts
@@ -0,0 +1 @@
+throw new Error('My error');
diff --git a/test/rules/fixtures/no-throw-default-error/eslintrc.js b/test/rules/fixtures/no-throw-default-error/eslintrc.js
@@ -0,0 +1,8 @@
+module.exports = {
+  plugins: ['local'],
+  rules: {
+    'local/no-throw-default-error': [ 'error' ],
+  }
+}
+
+
diff --git a/test/rules/fixtures/no-throw-default-error/throw-error.error.txt b/test/rules/fixtures/no-throw-default-error/throw-error.error.txt
@@ -0,0 +1 @@
+Expected a non-default error object to be thrown.
diff --git a/test/rules/fixtures/no-throw-default-error/throw-error.ts b/test/rules/fixtures/no-throw-default-error/throw-error.ts
@@ -0,0 +1 @@
+throw new Error('Some error');