Merge pull request #1088 from pallymore/feat/check-object-properties-…

…for-side-effects Check object properties for top level side effects
ericcornelissen · Jul 29, 2024 · 9e8cdb7 · 9e8cdb7
2 parents c7bb5f4 + 5f8f5fc
commit 9e8cdb7
Show file tree

Hide file tree

Showing 5 changed files with 293 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,10 @@ Versioning].
 
 ## [Unreleased]
 
+- (`f878f23`) Make the `no-top-level-side-effects` report side effects inside
+  top-level objects.
+- (`9f32d1d`) Make the `no-top-level-side-effects` report derived object values
+  when `allowDerived: false` (i.e. `{[foo]:42}` and `{...foo}`).
 - (`f878f23`) Allow literals at the top level.
 - (`edac7cc`) Automatically determine if analysis should set `commonjs: true`
   for the `no-top-level-side-effects` rule based on ESLint hints (if `commonjs`

diff --git a/lib/helpers.ts b/lib/helpers.ts
@@ -13,6 +13,8 @@ const topLevelTypes = new Set([
   'LogicalExpression',
   'NewExpression',
   'ObjectExpression',
+  'Property',
+  'SpreadElement',
   'UnaryExpression',
   'VariableDeclaration',
   'VariableDeclarator'

diff --git a/lib/rules/no-top-level-side-effects.ts b/lib/rules/no-top-level-side-effects.ts
@@ -268,6 +268,30 @@ export const noTopLevelSideEffects: Rule.RuleModule = {
           });
         }
       },
+      Property: (node) => {
+        if (options.allowDerived || !node.computed) {
+          return;
+        }
+
+        if (isTopLevel(node)) {
+          context.report({
+            node: node.key,
+            messageId: disallowedSideEffect.id
+          });
+        }
+      },
+      SpreadElement: (node) => {
+        if (options.allowDerived) {
+          return;
+        }
+
+        if (isTopLevel(node)) {
+          context.report({
+            node,
+            messageId: disallowedSideEffect.id
+          });
+        }
+      },
       SwitchStatement: (node) => {
         if (isTopLevel(node)) {
           context.report({

diff --git a/package.json b/package.json
@@ -67,7 +67,7 @@
   "scripts": {
     "prepublishOnly": "npm run build",
     "_eslint": "eslint --report-unused-disable-directives",
-    "_prettier": "prettier ./**/*.{js,json,jsonc,md,ts,yml} --ignore-path .gitignore",
+    "_prettier": "prettier \"**/*.{js,json,jsonc,md,ts,yml}\" --ignore-path .gitignore",
     "audit": "better-npm-audit audit",
     "audit:runtime": "better-npm-audit audit --production",
     "build": "rollup --config rollup.config.js",

diff --git a/tests/unit/no-top-level-side-effects.test.ts b/tests/unit/no-top-level-side-effects.test.ts
@@ -653,6 +653,79 @@ const valid: RuleTester.ValidTestCase[] = [
       code: `const u04 = ~a;`,
       options: [options.allowDerived]
     }
+  ],
+  ...[
+    {
+      code: `const x = {};`
+    },
+    {
+      code: `const x = { foo: 123 };`
+    },
+    {
+      code: `const x = { foo: 123, bar: 'baz' };`
+    },
+    {
+      code: `const x = { foo: () => 123 };`
+    },
+    {
+      code: `const x = { foo: function() { return true; } };`
+    },
+    {
+      code: `const x = { foo: function*() { yield true; } };`
+    },
+    {
+      code: `const x = { foo() { return 123; } };`
+    },
+    {
+      code: `const x = { foo: ok() };`,
+      options: [{allowedCalls: ['ok']}]
+    },
+    {
+      code: `const x = { foo: new Foo() };`,
+      options: [{allowedNews: ['Foo']}]
+    },
+    {
+      code: `const x = { foo: require('./config') };`,
+      options: [options.commonjs]
+    },
+    {
+      code: `const x = { foo: 40 + 2 };`,
+      options: [options.allowDerived]
+    },
+    {
+      code: `const x = { ...foo };`,
+      options: [options.allowDerived]
+    },
+    {
+      code: `const x = { [foo]: true };`,
+      options: [options.allowDerived]
+    },
+    {
+      code: `const x = { ...ok() };`,
+      options: [{...options.allowDerived, allowedCalls: ['ok']}]
+    },
+    {
+      code: `const x = { [ok()]: true };`,
+      options: [{...options.allowDerived, allowedCalls: ['ok']}]
+    },
+    {
+      code: `const f = () => ({ foo: 3 + 14 });`
+    },
+    {
+      code: `const f = () => ({ foo: bar() });`
+    },
+    {
+      code: `const f = () => ({ ...foo });`
+    },
+    {
+      code: `const f = () => ({ ...bar() });`
+    },
+    {
+      code: `const f = () => ({ [foo]: true });`
+    },
+    {
+      code: `const f = () => ({ [bar()]: true });`
+    }
   ]
 ];
 
@@ -2499,6 +2572,195 @@ const invalid: RuleTester.InvalidTestCase[] = [
         }
       ]
     }
+  ],
+  ...[
+    {
+      code: `const x = { foo: bad() };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 18,
+          endLine: 1,
+          endColumn: 23
+        }
+      ]
+    },
+    {
+      code: `const x = { foo: { nested: { bar: bad() } } };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 35,
+          endLine: 1,
+          endColumn: 40
+        }
+      ]
+    },
+    {
+      code: `const x = { foo: bad(), bar: worse(), baz: worst() };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 18,
+          endLine: 1,
+          endColumn: 23
+        },
+        {
+          messageId: '0',
+          line: 1,
+          column: 30,
+          endLine: 1,
+          endColumn: 37
+        },
+        {
+          messageId: '0',
+          line: 1,
+          column: 44,
+          endLine: 1,
+          endColumn: 51
+        }
+      ]
+    },
+    {
+      code: `const x = { foo: ok(), bar: fine(), baz: notThis() };`,
+      options: [{allowedCalls: ['ok', 'fine']}],
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 42,
+          endLine: 1,
+          endColumn: 51
+        }
+      ]
+    },
+    {
+      code: `const x = { ...bad() };`,
+      options: [options.allowDerived],
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 16,
+          endLine: 1,
+          endColumn: 21
+        }
+      ]
+    },
+    {
+      code: `const x = { foo: new Foo() };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 18,
+          endLine: 1,
+          endColumn: 27
+        }
+      ]
+    },
+    {
+      code: `const x = { foo: (function(){ return 1 })() };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 18,
+          endLine: 1,
+          endColumn: 44
+        }
+      ]
+    },
+    {
+      code: `const x = { foo: 40 + 2 };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 18,
+          endLine: 1,
+          endColumn: 24
+        }
+      ]
+    },
+    {
+      code: `const x = { foo: await bar() };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 18,
+          endLine: 1,
+          endColumn: 29
+        }
+      ]
+    },
+    {
+      code: `module.exports = { foo: { bar: bad() } };`,
+      options: [options.commonjs],
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 32,
+          endLine: 1,
+          endColumn: 37
+        }
+      ]
+    },
+    {
+      code: `const x = { ...foo };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 13,
+          endLine: 1,
+          endColumn: 19
+        }
+      ]
+    },
+    {
+      code: `const x = { [foo]: true };`,
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 14,
+          endLine: 1,
+          endColumn: 17
+        }
+      ]
+    },
+    {
+      code: `const x = { [foo()]: true };`,
+      options: [options.allowDerived],
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 14,
+          endLine: 1,
+          endColumn: 19
+        }
+      ]
+    },
+    {
+      code: `const x = { foo: (function(){ return 1 })() };`,
+      options: [options.allowIIFE],
+      errors: [
+        {
+          messageId: '0',
+          line: 1,
+          column: 18,
+          endLine: 1,
+          endColumn: 44
+        }
+      ]
+    }
   ]
 ];