JS-1608 Port TS6 rule FP fixes from #6626 (#6838)

vdiez · github-actions[bot] · web-flow · commit 6babfc729962 · 2026-04-17T16:48:34.000+02:00
Co-authored-by: github-actions[bot] &lt;41898282+github-actions[bot]@users.noreply.github.com&gt;
Co-authored-by: github-actions[bot] &lt;github-actions[bot]@users.noreply.github.com&gt;
diff --git a/its/ruling/src/test/expected/TypeScript/javascript-S3403.json b/its/ruling/src/test/expected/TypeScript/javascript-S3403.json
@@ -2,7 +2,6 @@
 "TypeScript:lib/tsc.js": [
 935,
 4077,
-6973,
 36581,
 36592,
 38926,
diff --git a/its/ruling/src/test/expected/ace/javascript-S1154.json b/its/ruling/src/test/expected/ace/javascript-S1154.json
diff --git a/its/ruling/src/test/expected/es5-shim/javascript-S1154.json b/its/ruling/src/test/expected/es5-shim/javascript-S1154.json
diff --git a/its/ruling/src/test/expected/paper.js/javascript-S1154.json b/its/ruling/src/test/expected/paper.js/javascript-S1154.json
diff --git a/its/ruling/src/test/expected/react-cloud-music/javascript-S3403.json b/its/ruling/src/test/expected/react-cloud-music/javascript-S3403.json
diff --git a/package.json b/package.json
@@ -12,7 +12,7 @@
     "ruling": "tsx --tsconfig packages/tsconfig.test.json --test packages/ruling/projects/*.ruling.test.ts",
     "sync-rspec": "tsx tools/sync-rspec.ts",
     "sync-rspec-all": "npm run sync-rspec -- --language javascript && npm run sync-rspec -- --language css && npm run deploy-rule-data",
-    "ruling-sync": "rsync -avh packages/ruling/actual/ its/ruling/src/test/expected/ --delete",
+    "ruling-sync": "node tools/ruling-sync.js",
     "bridge:compile:ts5": "tsc -b packages && tsx tools/generate-proto.ts --copy",
     "bridge:compile": "tsgo -p packages/tsconfig.app.json && tsgo -p packages/tsconfig.test.json && tsx tools/generate-proto.ts --copy",
     "bridge:test": "tsx --tsconfig packages/tsconfig.test.json --test --test-concurrency=4 --test-reporter=spec --test-reporter-destination stdout \"packages/**/src/*/rules/*[!node_modules]/**/*.test.ts\" \"packages/**/tests/**/*.test.ts\"",
diff --git a/packages/analysis/src/jsts/rules/S1154/rule.ts b/packages/analysis/src/jsts/rules/S1154/rule.ts
@@ -50,13 +50,34 @@ export const rule: Rule.RuleModule = {
       return variable;
     }
 
+    function isFunctionType(arg: estree.Expression | estree.SpreadElement | undefined) {
+      if (!arg || arg.type === 'SpreadElement') {
+        return false;
+      }
+      return getTypeFromTreeNode(arg, services).getCallSignatures().length > 0;
+    }
+
+    function isCallbackBasedReplacement(
+      property: estree.MemberExpression['property'],
+      args: Array<estree.Expression | estree.SpreadElement>,
+    ) {
+      if (property.type !== 'Identifier' || !['replace', 'replaceAll'].includes(property.name)) {
+        return false;
+      }
+      const replacementArg = args[1];
+      return isFunctionLike(replacementArg) || isFunctionType(replacementArg);
+    }
+
     return {
       'ExpressionStatement > CallExpression[callee.type="MemberExpression"]': (
         node: estree.Node,
       ) => {
-        const { object, property } = (node as estree.CallExpression)
-          .callee as estree.MemberExpression;
+        const callExpression = node as estree.CallExpression;
+        const { object, property } = callExpression.callee as estree.MemberExpression;
         if (isString(object) && property.type === 'Identifier') {
+          if (isCallbackBasedReplacement(property, callExpression.arguments)) {
+            return;
+          }
           context.report({
             messageId: 'uselessStringOp',
             data: {
@@ -69,3 +90,7 @@ export const rule: Rule.RuleModule = {
     };
   },
 };
+
+function isFunctionLike(arg: estree.Expression | estree.SpreadElement | undefined) {
+  return arg?.type === 'FunctionExpression' || arg?.type === 'ArrowFunctionExpression';
+}
diff --git a/packages/analysis/src/jsts/rules/S1154/unit.test.ts b/packages/analysis/src/jsts/rules/S1154/unit.test.ts
@@ -53,6 +53,17 @@ describe('S1154', () => {
         {
           code: `'hello'['whatever']();`,
         },
+        {
+          code: `
+        'hello'.replace(/l/g, () => 'x');
+      `,
+        },
+        {
+          code: `
+        const replacer = (value: string) => value.toUpperCase();
+        'hello'.replace(/l/g, replacer);
+      `,
+        },
       ],
       invalid: [
         {
@@ -99,6 +110,10 @@ describe('S1154', () => {
           code: `'hello'.substr(1, 2).toUpperCase();`,
           errors: 1,
         },
+        {
+          code: `'hello'.replace(/l/g, 'x');`,
+          errors: 1,
+        },
       ],
     });
   });
diff --git a/packages/analysis/src/jsts/rules/S3403/rule.ts b/packages/analysis/src/jsts/rules/S3403/rule.ts
@@ -23,6 +23,7 @@ import { generateMeta } from '../helpers/generate-meta.js';
 import { getTypeFromTreeNode } from '../helpers/type.js';
 import { isRequiredParserServices } from '../helpers/parser-services.js';
 import { report, toSecondaryLocation } from '../helpers/location.js';
+import { isNullLiteral, isUndefined } from '../helpers/ast.js';
 import * as meta from './generated-meta.js';
 
 export const rule: Rule.RuleModule = {
@@ -53,7 +54,11 @@ export const rule: Rule.RuleModule = {
     return {
       BinaryExpression: (node: estree.Node) => {
         const { left, operator, right } = node as estree.BinaryExpression;
-        if (['===', '!=='].includes(operator) && !isComparableTo(left, right)) {
+        if (
+          ['===', '!=='].includes(operator) &&
+          !isDefensiveIndexedNullishCheck(left, right) &&
+          !isComparableTo(left, right)
+        ) {
           const [actual, expected, outcome] =
             operator === '===' ? ['===', '==', 'false'] : ['!==', '!=', 'true'];
           const operatorToken = context.sourceCode
@@ -79,6 +84,32 @@ export const rule: Rule.RuleModule = {
   },
 };
 
+function isDefensiveIndexedNullishCheck(left: estree.Node, right: estree.Node) {
+  return (
+    (isNullishLiteral(left) && isIndexedAccess(right)) ||
+    (isNullishLiteral(right) && isIndexedAccess(left))
+  );
+}
+
+function isNullishLiteral(node: estree.Node) {
+  return (
+    isNullLiteral(node) ||
+    isUndefined(node) ||
+    (node.type === 'UnaryExpression' && node.operator === 'void')
+  );
+}
+
+function isIndexedAccess(node: estree.Node) {
+  if (node.type === 'MemberExpression') {
+    return node.computed;
+  }
+  return (
+    node.type === 'ChainExpression' &&
+    node.expression.type === 'MemberExpression' &&
+    node.expression.computed
+  );
+}
+
 /**
  * Checks if a type is indeterminate (its actual value cannot be determined at compile time).
  * - Unknown: can be compared with anything
diff --git a/packages/analysis/src/jsts/rules/S3403/unit.test.ts b/packages/analysis/src/jsts/rules/S3403/unit.test.ts
@@ -20,6 +20,7 @@ import {
   RuleTester,
 } from '../../../../tests/jsts/tools/testers/rule-tester.js';
 import { describe, it } from 'node:test';
+import path from 'node:path';
 
 describe('S3403', () => {
   it('S3403', () => {
@@ -38,6 +39,10 @@ describe('S3403', () => {
     );
 
     const ruleTesterTs = new RuleTester();
+    const strictProject = path.join(
+      import.meta.dirname,
+      '../../../../tests/jsts/tools/testers/fixtures/tsconfig.strict.json',
+    );
     ruleTesterTs.run(
       `Strict equality operators should not be used with dissimilar types [ts]`,
       rule,
@@ -154,6 +159,21 @@ describe('S3403', () => {
         let str = 'str', obj = {};
         str !== obj;`,
           },
+          {
+            // Defensive check on indexed access can be intentional in strict TS configurations.
+            code: `
+      const items = ['a'];
+      if (items[0] === undefined) {
+        doSomething();
+      }
+      function doSomething() {}
+      `,
+            languageOptions: {
+              parserOptions: {
+                project: strictProject,
+              },
+            },
+          },
           {
             code: `
       const foo = Symbol('foo');
@@ -262,6 +282,21 @@ describe('S3403', () => {
               },
             ],
           },
+          {
+            code: `
+      const s: string = 'a';
+      if (s === undefined) {
+        doSomething();
+      }
+      function doSomething() {}
+      `,
+            languageOptions: {
+              parserOptions: {
+                project: strictProject,
+              },
+            },
+            errors: 1,
+          },
         ],
       },
     );
diff --git a/packages/analysis/src/jsts/rules/S3757/rule.ts b/packages/analysis/src/jsts/rules/S3757/rule.ts
@@ -42,7 +42,10 @@ export const rule: Rule.RuleModule = {
 
     function isObjectType(...types: ts.Type[]): boolean {
       return types.some(
-        t => !!(t.getFlags() & ts.TypeFlags.Object) && !isDate(t) && t.symbol?.name !== 'Number',
+        t =>
+          !!(t.getFlags() & ts.TypeFlags.Object) &&
+          !isDate(t) &&
+          !['Number', 'Boolean'].includes(t.symbol?.name ?? ''),
       );
     }
 
diff --git a/packages/analysis/src/jsts/rules/S3757/unit.test.ts b/packages/analysis/src/jsts/rules/S3757/unit.test.ts
@@ -82,6 +82,11 @@ describe('S3757', () => {
        }
       `,
         },
+        {
+          code: `
+      new Boolean(true) - 1;
+    `,
+        },
       ],
       invalid: [
         {
diff --git a/packages/analysis/tests/jsts/tools/testers/fixtures/tsconfig.json b/packages/analysis/tests/jsts/tools/testers/fixtures/tsconfig.json
@@ -2,9 +2,13 @@
   "compilerOptions": {
     "allowJs": true,
     "noImplicitAny": true,
+    "strict": false,
+    "strictNullChecks": false,
     "lib": ["ESNext", "DOM"],
   },
-  "files": [
-    "placeholder.tsx"
+  "include": [
+    "*.ts",
+    "*.tsx",
+    "../../../../../src/jsts/rules/S6551/fixtures/*.ts"
   ]
 }
diff --git a/packages/analysis/tests/jsts/tools/testers/fixtures/tsconfig.strict.json b/packages/analysis/tests/jsts/tools/testers/fixtures/tsconfig.strict.json
@@ -0,0 +1,11 @@
+{
+  "compilerOptions": {
+    "allowJs": true,
+    "noImplicitAny": true,
+    "strict": true,
+    "lib": ["ESNext", "DOM"],
+  },
+  "files": [
+    "placeholder.tsx"
+  ]
+}
diff --git a/tools/ruling-sync.js b/tools/ruling-sync.js
@@ -0,0 +1,113 @@
+/*
+ * SonarQube JavaScript Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+import { cp, lstat, mkdir, readdir, rm } from 'node:fs/promises';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const currentFilePath = fileURLToPath(import.meta.url);
+const toolsDirectory = path.dirname(currentFilePath);
+const repositoryRoot = path.resolve(toolsDirectory, '..');
+
+const sourceDirectory = path.resolve(repositoryRoot, process.argv[2] ?? 'packages/ruling/actual');
+const destinationDirectory = path.resolve(
+  repositoryRoot,
+  process.argv[3] ?? 'its/ruling/src/test/expected',
+);
+
+ensurePathIsInsideRepository(sourceDirectory, 'source');
+ensurePathIsInsideRepository(destinationDirectory, 'destination');
+
+if (sourceDirectory === destinationDirectory) {
+  throw new Error('Source and destination directories must be different');
+}
+
+if (destinationDirectory.startsWith(`${sourceDirectory}${path.sep}`)) {
+  throw new Error('Destination directory cannot be nested inside source directory');
+}
+
+if (sourceDirectory.startsWith(`${destinationDirectory}${path.sep}`)) {
+  throw new Error('Source directory cannot be nested inside destination directory');
+}
+
+const sourceStat = await safeLstat(sourceDirectory);
+if (!sourceStat?.isDirectory()) {
+  throw new Error(`Source directory does not exist or is not a directory: ${sourceDirectory}`);
+}
+
+await mirrorDirectory(sourceDirectory, destinationDirectory);
+
+console.log(
+  `Mirrored ${path.relative(repositoryRoot, sourceDirectory)} -> ${path.relative(repositoryRoot, destinationDirectory)}`,
+);
+
+async function mirrorDirectory(source, destination) {
+  await mkdir(destination, { recursive: true });
+
+  const [sourceEntries, destinationEntries] = await Promise.all([
+    readdir(source, { withFileTypes: true }),
+    readdir(destination, { withFileTypes: true }),
+  ]);
+
+  const sourceEntryNames = new Set(sourceEntries.map(entry => entry.name));
+
+  for (const destinationEntry of destinationEntries) {
+    if (!sourceEntryNames.has(destinationEntry.name)) {
+      await rm(path.join(destination, destinationEntry.name), { recursive: true, force: true });
+    }
+  }
+
+  for (const sourceEntry of sourceEntries) {
+    const sourceEntryPath = path.join(source, sourceEntry.name);
+    const destinationEntryPath = path.join(destination, sourceEntry.name);
+    const destinationEntryStat = await safeLstat(destinationEntryPath);
+
+    if (sourceEntry.isDirectory()) {
+      if (destinationEntryStat && !destinationEntryStat.isDirectory()) {
+        await rm(destinationEntryPath, { recursive: true, force: true });
+      }
+      await mirrorDirectory(sourceEntryPath, destinationEntryPath);
+      continue;
+    }
+
+    if (destinationEntryStat?.isDirectory()) {
+      await rm(destinationEntryPath, { recursive: true, force: true });
+    }
+
+    await cp(sourceEntryPath, destinationEntryPath, {
+      force: true,
+      preserveTimestamps: true,
+    });
+  }
+}
+
+function ensurePathIsInsideRepository(candidatePath, label) {
+  const relativePath = path.relative(repositoryRoot, candidatePath);
+  if (relativePath.startsWith('..') || path.isAbsolute(relativePath)) {
+    throw new Error(`${label} path must be inside repository root: ${candidatePath}`);
+  }
+}
+
+async function safeLstat(candidatePath) {
+  try {
+    return await lstat(candidatePath);
+  } catch (error) {
+    if (error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT') {
+      return undefined;
+    }
+    throw error;
+  }
+}

-Original file line number
+Diff line change
 "TypeScript:lib/tsc.js": [
 ,
 ,
 -6973,
 ,
 ,
 ,