Added new lint rules for AmplifyUserError and AmplifyFault to restric…

…t where they should be used (#1960)

* chore: added new lint rules, but no specification for what they should act on

* chore: new rules disabled globally

* chore: added controls to turn on new rules in auth-construct and cli packages

* chore: changeset update

* chore: changeset added for cli and auth-construct

* chore: added tests for new lint rules

* chore: working on updating how rule is applied

* chore: moved selective rule activations

* chore: added comment about updating off to error in prefer-amplify-errors

* chore: change set update

* removed regex, ignore now happens with overrides

* changeset is now empty

* removed checkNode - unnecessary

---------

Co-authored-by: Vieltojarvi <[email protected]>

.changeset/famous-files-laugh.md

@@ -0,0 +1,2 @@
+---
+---

packages/eslint-rules/src/index.ts

@@ -1,9 +1,13 @@
 import { noEmptyCatchRule } from './rules/no_empty_catch.js';
 import { amplifyErrorNameRule } from './rules/amplify_error_name.js';
+import { preferAmplifyErrorsRule } from './rules/prefer_amplify_errors.js';
+import { noAmplifyErrors } from './rules/no_amplify_errors.js';
 
 export const rules: Record<string, unknown> = {
   'amplify-error-name': amplifyErrorNameRule,
   'no-empty-catch': noEmptyCatchRule,
+  'prefer-amplify-errors': preferAmplifyErrorsRule,
+  'no-amplify-errors': noAmplifyErrors,
 };
 
 export const configs = {
@@ -12,6 +16,23 @@ export const configs = {
     rules: {
       'amplify-backend-rules/amplify-error-name': 'error',
       'amplify-backend-rules/no-empty-catch': 'error',
+      'amplify-backend-rules/prefer-amplify-errors': 'off',
+      'amplify-backend-rules/no-amplify-errors': 'off',
     },
+    overrides: [
+      {
+        files: ['packages/cli/src/**'],
+        excludedFiles: ['**/*.test.ts'],
+        rules: {
+          'amplify-backend-rules/prefer-amplify-errors': 'off', //will be changed to 'error' in the future
+        },
+      },
+      {
+        files: ['packages/auth-construct/src/**'],
+        rules: {
+          'amplify-backend-rules/no-amplify-errors': 'error',
+        },
+      },
+    ],
   },
 };

packages/eslint-rules/src/rules/no_amplify_errors.test.ts

@@ -0,0 +1,36 @@
+import * as nodeTest from 'node:test';
+import { RuleTester } from '@typescript-eslint/rule-tester';
+import { noAmplifyErrors } from './no_amplify_errors';
+
+RuleTester.afterAll = nodeTest.after;
+// See https://typescript-eslint.io/packages/rule-tester/#with-specific-frameworks
+// Node test runner methods return promises which are not relevant in the context of testing.
+// We do ignore them in other places with void keyword.
+// eslint-disable-next-line @typescript-eslint/no-misused-promises
+RuleTester.it = nodeTest.it;
+// eslint-disable-next-line @typescript-eslint/no-misused-promises
+RuleTester.describe = nodeTest.describe;
+
+const ruleTester = new RuleTester();
+
+ruleTester.run('no-amplify-errors', noAmplifyErrors, {
+  valid: ['throw new Error()'],
+  invalid: [
+    {
+      code: 'throw new AmplifyUserError()',
+      errors: [
+        {
+          messageId: 'useOfAmplifyErrorDetected',
+        },
+      ],
+    },
+    {
+      code: 'throw new AmplifyFault()',
+      errors: [
+        {
+          messageId: 'useOfFaultDetected',
+        },
+      ],
+    },
+  ],
+});

packages/eslint-rules/src/rules/no_amplify_errors.ts

@@ -0,0 +1,43 @@
+import { ESLintUtils } from '@typescript-eslint/utils';
+
+export const noAmplifyErrors = ESLintUtils.RuleCreator.withoutDocs({
+  create(context) {
+    return {
+      // This naming comes from @typescript-eslint/utils types.
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      NewExpression(node) {
+        const checkNode = (
+          errorType: string,
+          messageId: 'useOfAmplifyErrorDetected' | 'useOfFaultDetected'
+        ) => {
+          if (
+            node.callee.type === 'Identifier' &&
+            node.callee.name === errorType
+          ) {
+            context.report({
+              node,
+              messageId,
+            });
+          }
+        };
+        checkNode('AmplifyFault', 'useOfFaultDetected');
+        checkNode('AmplifyUserError', 'useOfAmplifyErrorDetected');
+      },
+    };
+  },
+  meta: {
+    docs: {
+      description:
+        'AmplifyUserError and AmplifyFault should not be used in certain packages',
+    },
+    messages: {
+      useOfAmplifyErrorDetected:
+        'Error base class should be used instead of AmplifyUserError',
+      useOfFaultDetected:
+        'Error base class should be used instead of AmplifyFault',
+    },
+    type: 'problem',
+    schema: [],
+  },
+  defaultOptions: [],
+});

packages/eslint-rules/src/rules/prefer_amplify_errors.test.ts

@@ -0,0 +1,28 @@
+import * as nodeTest from 'node:test';
+import { RuleTester } from '@typescript-eslint/rule-tester';
+import { preferAmplifyErrorsRule } from './prefer_amplify_errors';
+
+RuleTester.afterAll = nodeTest.after;
+// See https://typescript-eslint.io/packages/rule-tester/#with-specific-frameworks
+// Node test runner methods return promises which are not relevant in the context of testing.
+// We do ignore them in other places with void keyword.
+// eslint-disable-next-line @typescript-eslint/no-misused-promises
+RuleTester.it = nodeTest.it;
+// eslint-disable-next-line @typescript-eslint/no-misused-promises
+RuleTester.describe = nodeTest.describe;
+
+const ruleTester = new RuleTester();
+
+ruleTester.run('prefer-amplify-errors', preferAmplifyErrorsRule, {
+  valid: ['throw new AmplifyUserError()', 'throw new AmplifyFault()'],
+  invalid: [
+    {
+      code: 'throw new Error()',
+      errors: [
+        {
+          messageId: 'useOfErrorDetected',
+        },
+      ],
+    },
+  ],
+});

packages/eslint-rules/src/rules/prefer_amplify_errors.ts

@@ -0,0 +1,30 @@
+import { ESLintUtils } from '@typescript-eslint/utils';
+
+export const preferAmplifyErrorsRule = ESLintUtils.RuleCreator.withoutDocs({
+  create(context) {
+    return {
+      // This naming comes from @typescript-eslint/utils types.
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      NewExpression(node) {
+        if (node.callee.type === 'Identifier' && node.callee.name === 'Error') {
+          context.report({
+            messageId: 'useOfErrorDetected',
+            node,
+          });
+        }
+      },
+    };
+  },
+  meta: {
+    docs: {
+      description: 'Error base class should not be used in certain packages',
+    },
+    messages: {
+      useOfErrorDetected:
+        'AmplifyUserError or AmplifyFault should be used instead of Error base class',
+    },
+    type: 'problem',
+    schema: [],
+  },
+  defaultOptions: [],
+});