Add new rule no-decorators-before-export

README.md

@@ -148,14 +148,15 @@ module.exports = {
 
 ### Miscellaneous
 
-| Name                                                                                                                   | Description                                                                                                                   | 💼 | 🔧 | 💡 |
-| :--------------------------------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------- | :- | :- | :- |
-| [named-functions-in-promises](docs/rules/named-functions-in-promises.md)                                               | enforce usage of named functions in promises                                                                                  |    |    |    |
-| [no-html-safe](docs/rules/no-html-safe.md)                                                                             | disallow the use of `htmlSafe`                                                                                                |    |    |    |
-| [no-incorrect-calls-with-inline-anonymous-functions](docs/rules/no-incorrect-calls-with-inline-anonymous-functions.md) | disallow inline anonymous functions as arguments to `debounce`, `once`, and `scheduleOnce`                                    | ✅  |    |    |
-| [no-invalid-debug-function-arguments](docs/rules/no-invalid-debug-function-arguments.md)                               | disallow usages of Ember's `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order. | ✅  |    |    |
-| [no-restricted-property-modifications](docs/rules/no-restricted-property-modifications.md)                             | disallow modifying the specified properties                                                                                   |    | 🔧 |    |
-| [require-fetch-import](docs/rules/require-fetch-import.md)                                                             | enforce explicit import for `fetch()`                                                                                         |    |    |    |
+| Name                                                                                                                   | Description                                                                                                                                      | 💼 | 🔧 | 💡 |
+| :--------------------------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------- | :- | :- | :- |
+| [named-functions-in-promises](docs/rules/named-functions-in-promises.md)                                               | enforce usage of named functions in promises                                                                                                     |    |    |    |
+| [no-decorators-before-export](docs/rules/no-decorators-before-export.md)                                               | disallow the use of a class decorator on an export declaration. Enforces use of the decorator on the class directly before exporting the result. | ✅  | 🔧 |    |
+| [no-html-safe](docs/rules/no-html-safe.md)                                                                             | disallow the use of `htmlSafe`                                                                                                                   |    |    |    |
+| [no-incorrect-calls-with-inline-anonymous-functions](docs/rules/no-incorrect-calls-with-inline-anonymous-functions.md) | disallow inline anonymous functions as arguments to `debounce`, `once`, and `scheduleOnce`                                                       | ✅  |    |    |
+| [no-invalid-debug-function-arguments](docs/rules/no-invalid-debug-function-arguments.md)                               | disallow usages of Ember's `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order.                    | ✅  |    |    |
+| [no-restricted-property-modifications](docs/rules/no-restricted-property-modifications.md)                             | disallow modifying the specified properties                                                                                                      |    | 🔧 |    |
+| [require-fetch-import](docs/rules/require-fetch-import.md)                                                             | enforce explicit import for `fetch()`                                                                                                            |    |    |    |
 
 ### Routes
 

docs/rules/no-decorators-before-export.md

@@ -0,0 +1,101 @@
+# ember/no-decorators-before-export
+
+💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/ember-cli/eslint-plugin-ember#-configurations).
+
+🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
+
+<!-- end auto-generated rule header -->
+
+Disallow the use of a class decorator on an export declaration. Enforces use of the decorator on the class directly before exporting the result.
+
+## Background
+
+In the very first  TC39 proposal for decorators, a syntax option was considered to allow the following:
+
+```ts
+import classic from 'ember-classic-decorators';
+import Component from '@ember/component';
+
+@classic
+export default class MyComponent extends Component {}
+```
+
+However, in the various specs that followed, including in the current proposal that is stage-3, this syntax was explicitly disallowed. The problem it causes is a little more apparent when considering it as a function and placing everything on one line:
+
+```ts
+@classic export default class MyComponent extends Component {}
+```
+
+The correct way to specify decorators is onto the class itself prior to exporting the class. This mechanism works with the version of decorators which Ember adopted, and is the positioning the stage-3 spec adopted as well.
+
+```ts
+import classic from 'ember-classic-decorators';
+import Component from '@ember/component';
+
+@classic
+class MyComponent extends Component {}
+
+export default MyComponent;
+```
+
+Unfortunately, in order to maximize compatibility with existing decorators, the second rendition of the spec - the rendition of the spec which Ember adopted (there have been four) - allowed this syntax to still be used if using the babel plugin AND also explicitly allowing it in the plugin config. Ember chose to be maximally capatible and enabled this option.
+
+This can create problems:
+
+1) being non-spec, non-babel parsers do not support it. This means codemods using jscodeshift, rollup using acorn, and any number of other tools or code compilers error when they encounter these files.
+
+2) its easy for transpilation to go very wrong if custom babel transforms do not account for this case.
+
+3) The babel transform for decorators never included support for applying them to unnamed default exports.
+
+Considering that this syntax was non-spec even at the point that ember adopted decorators, and considering that the correct approach works as expected, we lint against the non-standard approach.
+
+## Examples
+
+### Good
+
+```ts
+import classic from 'ember-classic-decorators';
+import Component from '@ember/component';
+
+@classic
+class MyComponent extends Component {}
+
+export default MyComponent;
+```
+
+```ts
+import classic from 'ember-classic-decorators';
+import EmberObject from '@ember/object';
+
+@classic
+class MyKlass extends EmberObject {}
+
+export { MyKlass };
+```
+
+### Bad
+
+```ts
+import classic from 'ember-classic-decorators';
+import Component from '@ember/component';
+
+@classic
+export default class extends Component {}
+```
+
+```ts
+import classic from 'ember-classic-decorators';
+import Component from '@ember/component';
+
+@classic
+export default class MyComponent extends Component {}
+```
+
+```ts
+import classic from 'ember-classic-decorators';
+import EmberObject from '@ember/object';
+
+@classic
+export class MyKlass extends EmberObject {}
+```

lib/recommended-rules.js

@@ -23,6 +23,7 @@ module.exports = {
   "ember/no-component-lifecycle-hooks": "error",
   "ember/no-computed-properties-in-native-classes": "error",
   "ember/no-controller-access-in-routes": "error",
+  "ember/no-decorators-before-export": "error",
   "ember/no-deeply-nested-dependent-keys-with-each": "error",
   "ember/no-duplicate-dependent-keys": "error",
   "ember/no-ember-super-in-es-classes": "error",
@@ -70,4 +71,4 @@ module.exports = {
   "ember/routes-segments-snake-case": "error",
   "ember/use-brace-expansion": "error",
   "ember/use-ember-data-rfc-395-imports": "error"
-}
+}

lib/rules/no-decorators-before-export.js

@@ -0,0 +1,73 @@
+'use strict';
+
+/** @type {import('eslint').Rule.RuleModule} */
+
+function reportInvalidSyntax(context, node) {
+  if (isInvalidSyntax(node)) {
+    const isDefaultExport = node.type === 'ExportDefaultDeclaration';
+    const isNamed = node.declaration.id !== null;
+    const name = isNamed ? node.declaration.id.name : '';
+    const numDecorators = node.declaration.decorators.length;
+
+    const message = `Usage of class decorator${numDecorators > 1 ? 's' : ''} on the ${
+      isNamed ? '' : 'un-named '
+    }${isDefaultExport ? 'default ' : ''}export${
+      isNamed ? ` ${name}` : ''
+    } must occur prior to exporting the class.`;
+
+    if (isNamed) {
+      const sourceCode = context.getSourceCode();
+      const src = sourceCode.getText(node);
+      context.report({
+        node,
+        message,
+        fix(fixer) {
+          if (isDefaultExport) {
+            let newSrc = src.replace(`export default class ${name}`, `class ${name}`);
+            newSrc = `${newSrc}\nexport default ${name};\n`;
+            return fixer.replaceText(node, newSrc);
+          }
+          let newSrc = src.replace(`export class ${name}`, `class ${name}`);
+          newSrc = `${newSrc}\nexport { ${name} };\n`;
+          return fixer.replaceText(node, newSrc);
+        },
+      });
+    } else {
+      context.report({ node, message });
+    }
+  }
+}
+
+function isInvalidSyntax(node) {
+  return (
+    node.declaration &&
+    node.declaration.type === 'ClassDeclaration' &&
+    node.declaration.decorators?.length
+  );
+}
+
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        'disallow the use of a class decorator on an export declaration. Enforces use of the decorator on the class directly before exporting the result.',
+      category: 'Miscellaneous',
+      recommended: true,
+      url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-decorators-before-export.md',
+    },
+    fixable: 'code',
+    schema: [],
+  },
+
+  create(context) {
+    return {
+      ExportDefaultDeclaration(node) {
+        return reportInvalidSyntax(context, node);
+      },
+      ExportNamedDeclaration(node) {
+        return reportInvalidSyntax(context, node);
+      },
+    };
+  },
+};

tests/__snapshots__/recommended.js.snap

@@ -20,6 +20,7 @@ exports[`recommended rules has the right list 1`] = `
   "no-component-lifecycle-hooks",
   "no-computed-properties-in-native-classes",
   "no-controller-access-in-routes",
+  "no-decorators-before-export",
   "no-deeply-nested-dependent-keys-with-each",
   "no-duplicate-dependent-keys",
   "no-ember-super-in-es-classes",

tests/lib/rules/no-decorators-before-export.js

@@ -0,0 +1,70 @@
+const rule = require('../../../lib/rules/no-decorators-before-export');
+const RuleTester = require('eslint').RuleTester;
+
+const eslintTester = new RuleTester({
+  parser: require.resolve('@babel/eslint-parser'),
+  parserOptions: {
+    ecmaVersion: 2022,
+    sourceType: 'module',
+    requireConfigFile: false,
+    babelOptions: {
+      babelrc: false,
+      plugins: [['@babel/plugin-proposal-decorators', { decoratorsBeforeExport: true }]],
+    },
+  },
+});
+
+eslintTester.run('no-decorators-before-export', rule, {
+  valid: [
+    'export default class {}',
+    'export default class Foo {}',
+    'class Foo {}\nexport default Foo;',
+    'export class Foo {}',
+    'class Foo {}\nexport { Foo };',
+    "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {}\nexport { Foo };",
+    "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {}\nexport default Foo;",
+  ],
+  invalid: [
+    // basic
+    {
+      code: "import classic from 'ember-classic-decorators';\n@classic\nexport class Foo {}",
+      output:
+        "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {}\nexport { Foo };\n",
+      errors: [
+        'Usage of class decorator on the export Foo must occur prior to exporting the class.',
+      ],
+    },
+    {
+      code: "import classic from 'ember-classic-decorators';\n@classic\nexport default class Foo {}",
+      output:
+        "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {}\nexport default Foo;\n",
+      errors: [
+        'Usage of class decorator on the default export Foo must occur prior to exporting the class.',
+      ],
+    },
+    {
+      code: "import classic from 'ember-classic-decorators';\n@classic\nexport default class {}",
+      output: null,
+      errors: [
+        'Usage of class decorator on the un-named default export must occur prior to exporting the class.',
+      ],
+    },
+    // ensure interop
+    {
+      code: "import classic from 'ember-classic-decorators';\n@classic\nexport class Foo {\n  someProp = false;\n}\nexport class Bar {};\n",
+      output:
+        "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {\n  someProp = false;\n}\nexport { Foo };\n\nexport class Bar {};\n",
+      errors: [
+        'Usage of class decorator on the export Foo must occur prior to exporting the class.',
+      ],
+    },
+    {
+      code: "import classic from 'ember-classic-decorators';\n@classic\nexport default class Foo {\n  someProp = false;\n}\nexport class Bar {};\n",
+      output:
+        "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {\n  someProp = false;\n}\nexport default Foo;\n\nexport class Bar {};\n",
+      errors: [
+        'Usage of class decorator on the default export Foo must occur prior to exporting the class.',
+      ],
+    },
+  ],
+});