Fix S6661 (prefer-object-spread): Disable for projects that don't s…

…upport the object spread syntax and improve rule description (#4552)

Co-authored-by: yassin-kammoun-sonarsource <yassin.kammoun@sonarsource.com>

packages/jsts/src/analysis/projectAnalysis/projectAnalyzer.ts

@@ -30,7 +30,7 @@ import { analyzeWithWatchProgram } from './analyzeWithWatchProgram';
 import { analyzeWithoutProgram } from './analyzeWithoutProgram';
 import { initializeLinter } from '../../linter';
 import { TSCONFIG_JSON, setTSConfigs, getTSConfigsIterator } from '../../program';
-import { PACKAGE_JSON, parsePackageJson, setPackageJsons } from '../../dependencies';
+import { PACKAGE_JSON, parsePackageJson, setPackageJsons } from '../../project-metadata';
 
 /**
  * Analyzes a JavaScript / TypeScript project in a single run

packages/jsts/src/index.ts

@@ -19,7 +19,7 @@
  */
 export * from './analysis';
 export * from './builders';
-export * from './dependencies';
+export * from './project-metadata';
 export * from './linter';
 export * from './parsers';
 export * from './program';

packages/jsts/src/project-metadata/index.ts

@@ -0,0 +1,73 @@
+/*
+ * SonarQube JavaScript Plugin
+ * Copyright (C) 2011-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+export * from './package-json';
+
+import { getNearestPackageJsons } from '.';
+import * as semver from 'semver';
+
+/**
+ * Minimum version per reference
+ */
+type MinimumVersions = {
+  node?: string;
+};
+
+/**
+ * Checks if context where the filename is located supports the provided
+ * minimum versions.
+ */
+export function isSupported(filename: string, minVersions: MinimumVersions): boolean {
+  validateVersions(minVersions);
+  return isSupportedNodeVersion(filename, minVersions.node);
+}
+
+/**
+ * Check if the versions are valid semver
+ */
+function validateVersions(versions: MinimumVersions) {
+  for (const [ref, version] of Object.entries(versions)) {
+    if (semver.valid(version) === null) {
+      throw new Error(`Invalid semver version: "${version}" for "${ref}"`);
+    }
+  }
+}
+
+/**
+ * Check if the feature is supported by the minimum Node.js version of the project.
+ *
+ * @param filename
+ * @param requiredVersion
+ * @returns
+ */
+function isSupportedNodeVersion(filename: string, requiredVersion?: string): boolean {
+  if (!requiredVersion) {
+    return true;
+  }
+  const packageJsons = getNearestPackageJsons(filename);
+  const versionRange = packageJsons.find(pj => pj.contents.engines?.node)?.contents.engines?.node;
+  if (!versionRange) {
+    return true;
+  }
+  const projectMinVersion = semver.minVersion(versionRange);
+  if (!projectMinVersion) {
+    return true;
+  }
+  return semver.gte(projectMinVersion, requiredVersion);
+}

packages/jsts/src/rules/S4123/rule.ts

@@ -36,10 +36,7 @@ export const rule: Rule.RuleModule = {
     if (isRequiredParserServices(services)) {
       return {
         AwaitExpression: (node: estree.AwaitExpression) => {
-          const awaitedType = getTypeFromTreeNode(
-            (node as estree.AwaitExpression).argument,
-            services,
-          );
+          const awaitedType = getTypeFromTreeNode(node.argument, services);
           if (
             !isException(node, services) &&
             !isThenable(awaitedType) &&

packages/jsts/src/rules/S6661/fixtures/unsupported-node/package.json

@@ -0,0 +1,5 @@
+{
+  "engines": {
+    "node": ">= 0.10.0"
+  }
+}

packages/jsts/src/rules/S6661/index.ts

@@ -17,7 +17,21 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
+import { Rule } from 'eslint';
 import { eslintRules } from '../core';
 import { decorate } from './decorator';
+import { isSupported } from '@sonar/jsts';
 
-export const rule = decorate(eslintRules['prefer-object-spread']);
+const decorated = decorate(eslintRules['prefer-object-spread']);
+
+export const rule: Rule.RuleModule = {
+  meta: decorated.meta,
+  create(context) {
+    // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#browser_compatibility
+    if (!isSupported(context.filename, { node: '8.3.0' })) {
+      return {};
+    }
+
+    return decorated.create(context);
+  },
+};

packages/jsts/src/rules/S6661/unit.test.ts

@@ -19,6 +19,10 @@
  */
 import { RuleTester } from 'eslint';
 import { rule } from './';
+import { clearPackageJsons, loadPackageJsons } from '@sonar/jsts';
+import path from 'path';
+
+clearPackageJsons();
 
 const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } });
 
@@ -71,3 +75,22 @@ const b = { ...foo, ...bar};`,
     },
   ],
 });
+
+clearPackageJsons();
+const project = path.join(__dirname, 'fixtures', 'unsupported-node');
+loadPackageJsons(project, []);
+const filename = path.join(project, 'file.js');
+
+ruleTester.run(
+  'When the project does not support the object spread syntax, the rule should be ignored',
+  rule,
+  {
+    valid: [
+      {
+        code: `Object.assign({}, bar);`,
+        filename,
+      },
+    ],
+    invalid: [],
+  },
+);

packages/jsts/tests/project-metadata/fixtures/is-supported-node/no-node/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "project",
+  "version": "1.0.0",
+  "author": "Your Name <email@example.com>"
+}

packages/jsts/tests/project-metadata/fixtures/is-supported-node/with-node-no-minimum/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "project",
+  "version": "1.0.0",
+  "author": "Your Name <email@example.com>",
+  "engines": {
+    "node": ">4 <3"
+  }
+}

packages/jsts/tests/project-metadata/fixtures/is-supported-node/with-node-with-minimum/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "project",
+  "version": "1.0.0",
+  "author": "Your Name <email@example.com>",
+  "engines": {
+    "node": ">=5.0.0"
+  }
+}

packages/jsts/tests/dependencies/index.test.ts → packages/jsts/tests/project-metadata/index.test.ts

@@ -25,15 +25,16 @@ import {
   getNearestPackageJsons,
   clearPackageJsons,
   getPackageJsonsCount,
+  isSupported,
 } from '@sonar/jsts';
 
 describe('initialize package.json files', () => {
+  const baseDir = path.posix.join(toUnixPath(__dirname), 'fixtures', 'package-json');
   beforeEach(() => {
     clearPackageJsons();
   });
 
   it('should find all package.json files', () => {
-    const baseDir = path.posix.join(toUnixPath(__dirname), 'fixtures');
     loadPackageJsons(baseDir, []);
     expect(getPackageJsonsCount()).toEqual(7);
 
@@ -91,8 +92,7 @@ describe('initialize package.json files', () => {
 
     const fakeFilePJList = getNearestPackageJsons(
       path.posix.join(
-        __dirname,
-        'fixtures',
+        baseDir,
         'moduleB',
         '.submoduleB',
         'subfolder1',
@@ -106,8 +106,6 @@ describe('initialize package.json files', () => {
   });
 
   it('should ignore package.json files from ignored patterns', () => {
-    const baseDir = path.posix.join(toUnixPath(__dirname), 'fixtures');
-
     loadPackageJsons(baseDir, ['moduleA']);
     expect(getPackageJsonsCount()).toEqual(4);
     const expected = [
@@ -135,8 +133,6 @@ describe('initialize package.json files', () => {
   });
 
   it('should return empty array when no package.json are in the DB or none exist in the file tree', () => {
-    const baseDir = path.posix.join(toUnixPath(__dirname), 'fixtures');
-
     expect(getPackageJsonsCount()).toEqual(0);
     expect(
       getNearestPackageJsons(path.posix.join(baseDir, '..', 'another-module', 'index.js')),
@@ -149,3 +145,67 @@ describe('initialize package.json files', () => {
     ).toHaveLength(0);
   });
 });
+
+describe('isSupported()', () => {
+  let baseDir;
+  beforeEach(() => {
+    clearPackageJsons();
+    baseDir = path.posix.join(toUnixPath(__dirname), 'fixtures', 'is-supported-node');
+  });
+
+  it('should throw an error when a version is invalid', () => {
+    expect(() => isSupported('index.js', { node: 'invalid' })).toThrowError(
+      'Invalid semver version: "invalid" for "node"',
+    );
+  });
+
+  it('should return true when no minimum version is provided', () => {
+    expect(isSupported('index.js', {})).toBe(true);
+  });
+
+  describe('#isSupportedNodeVersion()', () => {
+    describe('when package.json#engine.node is defined', () => {
+      describe('when there is a minimum version', () => {
+        it('should return true when the project supports the feature', () => {
+          const projectDir = path.posix.join(baseDir, 'with-node-with-minimum');
+          loadPackageJsons(projectDir, []);
+          expect(isSupported(path.posix.join(projectDir, 'index.js'), { node: '4.0.0' })).toBe(
+            true,
+          );
+        });
+        it('should return false when the project does not support the feature', () => {
+          const projectDir = path.posix.join(baseDir, 'with-node-with-minimum');
+          loadPackageJsons(projectDir, []);
+          expect(isSupported(path.posix.join(projectDir, 'index.js'), { node: '6.0.0' })).toBe(
+            false,
+          );
+        });
+      });
+
+      // coverage
+      describe('when there is no minimum version', () => {
+        it('should return true', () => {
+          const projectDir = path.posix.join(baseDir, 'with-node-no-minimum');
+          loadPackageJsons(projectDir, []);
+          expect(isSupported(path.posix.join(projectDir, 'index.js'), { node: '5.0.0' })).toBe(
+            true,
+          );
+        });
+      });
+    });
+    describe('when package.json#engine.node is undefined', () => {
+      it('should return true', () => {
+        const projectDir = path.posix.join(baseDir, 'no-node');
+        loadPackageJsons(projectDir, []);
+        expect(isSupported(path.posix.join(projectDir, 'index.js'), { node: '6.0.0' })).toBe(true);
+      });
+    });
+    describe('when no package.json is found', () => {
+      // we simply don't load the package.json files
+      it('should return true', () => {
+        const projectDir = path.posix.join(baseDir, 'no-node');
+        expect(isSupported(path.posix.join(projectDir, 'index.js'), { node: '6.0.0' })).toBe(true);
+      });
+    });
+  });
+});

sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S6661.html

@@ -1,8 +1,12 @@
 <h2>Why is this an issue?</h2>
-<p>When merging objects or copying properties from one object to another, use object spread syntax instead of <code>Object.assign()</code>. Object
-spread syntax was introduced in ES2018 and allows shallow-cloning or merging of objects with a more concise and readable syntax.</p>
+<p>When merging objects or copying properties from one object to another, use the object spread syntax instead of <code>Object.assign()</code>. The
+Object spread syntax was introduced in ES2018 and allows shallow-cloning or merging of objects with a more concise and readable syntax.</p>
 <p>The <code>Object.assign()</code> also allows to mutate an object, which is not possible with the spread syntax, so the rule only applies to cases
 where the first argument of the <code>Object.assign()</code> is an object literal.</p>
+<p>The object spread syntax improves clarity when you’re modifying an object, as demonstrated in this example: <code>foo = { bar: 42, …​baz
+}</code>.</p>
+<p>Additionally, it provides a more concise way to perform a shallow clone. Instead of using <code>foo = Object.assign({}, bar)</code>, you can simply
+write <code>foo = { …​bar }</code>.</p>
 <pre data-diff-id="1" data-diff-type="noncompliant">
 const a = Object.assign({}, foo); // Noncompliant: Use spread syntax to clone or merge objects
 const b = Object.assign({}, foo, bar); // Noncompliant: Use spread syntax to clone or merge objects