JS-1580 Fix S1788 false positive for Redux reducer with destructured action (#6819)

Co-authored-by: Vibe Bot <vibe-bot@sonarsource.com>

Author: sonar-nigel[bot]

packages/analysis/src/jsts/rules/S1788/cb.fixture.js

@@ -1,14 +1,28 @@
-function multiply(b, a = 1) {}
+const initialState = {};
 
-function multiply(b = 2, a = 1) {}
+function multiplyDefaultLast(b, a = 1) { return b * a; }
 
-function appReducer(state = initialState, action) {}
+function multiplyBothDefault(b = 2, a = 1) { return b * a; }
 
-function multiply(a = 1, b) {} // Noncompliant {{Default parameters should be last.}}
+function appReducerNamed(state = initialState, action) { return state; }
+
+// Redux reducer with destructured action containing 'type' property
+function appReducerWithType(state = initialState, { type }) { return state; }
+
+const dataReducer = (state = null, { type, payload }) => state;
+
+export const isPanelOpen = (state = false, { type, isShowing }) =>
+  type === 'SET_IS_SHOWING' ? isShowing : state;
+
+function multiply(a = 1, b) { return a * b; } // Noncompliant {{Default parameters should be last.}}
 //                ^^^^^
 
-function appReducer(state = initialState, action, param) {} // Noncompliant {{Default parameters should be last.}}
+function appReducer(state = initialState, action, param) { return state; } // Noncompliant {{Default parameters should be last.}}
 //                  ^^^^^^^^^^^^^^^^^^^^
 
-function appReducer(status = initialState, action) {} // Noncompliant {{Default parameters should be last.}}
-//                  ^^^^^^^^^^^^^^^^^^^^^
+function appReducerWrongParam(status = initialState, action) { return status; } // Noncompliant {{Default parameters should be last.}}
+//                            ^^^^^^^^^^^^^^^^^^^^^
+
+// Destructured action without 'type' is not a Redux reducer
+function notReducer(state = initialState, { payload }) { return state; } // Noncompliant {{Default parameters should be last.}}
+//                  ^^^^^^^^^^^^^^^^^^^^

packages/analysis/src/jsts/rules/S1788/cb.test.ts

@@ -15,10 +15,32 @@
  * along with this program; if not, see https://sonarsource.com/license/ssal/
  */
 import { test } from '../../../../tests/jsts/tools/testers/comment-based/checker.js';
+import { DefaultParserRuleTester } from '../../../../tests/jsts/tools/testers/rule-tester.js';
 import { rule } from './index.js';
-import { describe } from 'node:test';
+import { rules as tsEslintRules } from '../external/typescript-eslint/index.js';
+import { describe, it } from 'node:test';
 import * as meta from './generated-meta.js';
 
+const upstreamRule = tsEslintRules['default-param-last'];
+
+// Sentinel: verify that the upstream ESLint rule still raises on the patterns our decorator fixes.
+// If this test starts failing (i.e., the upstream rule no longer reports these patterns),
+// it signals that the decorator can be safely removed.
+describe('S1788 upstream sentinel', () => {
+  it('upstream default-param-last raises on destructured action patterns that decorator suppresses', () => {
+    const ruleTester = new DefaultParserRuleTester();
+    ruleTester.run('default-param-last', upstreamRule, {
+      valid: [],
+      invalid: [
+        // (state = x, { type }) — suppressed by decorator, raised by upstream
+        { code: `function appReducer(state = initialState, { type }) {}`, errors: 1 },
+        // (state = x, { type, payload }) — suppressed by decorator, raised by upstream
+        { code: `const dataReducer = (state = null, { type, payload }) => {};`, errors: 1 },
+      ],
+    });
+  });
+});
+
 describe('Rule S1788', () => {
   test(meta, rule, import.meta.dirname);
 });

packages/analysis/src/jsts/rules/S1788/decorator.ts

@@ -54,11 +54,15 @@ function reportExempting(
 function isReduxReducer(enclosingFunction: BaseFunction) {
   if (enclosingFunction.params.length === NUM_ARGS_REDUX_REDUCER) {
     const [firstParam, secondParam] = enclosingFunction.params;
-    return (
-      firstParam.type === 'AssignmentPattern' &&
-      isIdentifier(firstParam.left, 'state') &&
-      isIdentifier(secondParam, 'action')
-    );
+    if (firstParam.type === 'AssignmentPattern' && isIdentifier(firstParam.left, 'state')) {
+      return (
+        isIdentifier(secondParam, 'action') ||
+        (secondParam.type === 'ObjectPattern' &&
+          secondParam.properties.some(
+            prop => prop.type === 'Property' && isIdentifier(prop.key, 'type'),
+          ))
+      );
+    }
   }
   return false;
 }