JS-106 Fix S3801: Detect calls to never-returning functions (#6155)

Co-authored-by: Claude Opus 4.5 <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: 3 people

its/ruling/src/test/expected/jsts/ace/javascript-S3801.json

@@ -64,7 +64,6 @@
 2747,
 2769,
 2955,
-5100,
 5333,
 5923,
 5970,
@@ -103,13 +102,6 @@
 "ace:lib/ace/mode/javascript_worker.js": [
 105
 ],
-"ace:lib/ace/mode/json/json_parse.js": [
-106,
-147,
-199,
-229,
-256
-],
 "ace:lib/ace/mode/jsoniq.js": [
 26,
 51

its/ruling/src/test/expected/jsts/desktop/typescript-S3801.json

@@ -128,9 +128,6 @@
 "desktop:app/src/ui/lib/list/list.tsx": [
 590
 ],
-"desktop:app/src/ui/local-changes-overwritten/local-changes-overwritten-dialog.tsx": [
-157
-],
 "desktop:app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx": [
 75,
 103,

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

@@ -24,6 +24,7 @@ import {
   generateMeta,
   getMainFunctionTokenLocation,
   getParent,
+  getSignatureFromCallee,
   getTypeFromTreeNode,
   isRequiredParserServices,
   report,
@@ -42,6 +43,8 @@ interface FunctionContext {
   switchStatements: estree.SwitchStatement[];
   /** Tracks whether last case of each switch has reachable exit (doesn't return/throw) */
   switchLastCaseReachable: Map<estree.SwitchStatement, boolean>;
+  /** The last call expression in the function body (may return never) */
+  lastCallExpression: estree.CallExpression | null;
 }
 
 interface FunctionLikeDeclaration {
@@ -122,7 +125,14 @@ export const rule: Rule.RuleModule = {
             isExhaustiveSwitch(switchStmt, services) &&
             !functionContext.switchLastCaseReachable.get(switchStmt),
         );
-        functionContext.containsImplicitReturn = !hasExhaustiveSwitch;
+
+        // Check if the last call expression returns 'never' (e.g., a throwing function)
+        // If so, the implicit return is unreachable
+        const lastCallReturnsNever =
+          functionContext.lastCallExpression &&
+          isNeverReturningCall(functionContext.lastCallExpression, services);
+
+        functionContext.containsImplicitReturn = !hasExhaustiveSwitch && !lastCallReturnsNever;
       } else {
         functionContext.containsImplicitReturn = hasReachableSegment;
       }
@@ -160,6 +170,7 @@ export const rule: Rule.RuleModule = {
           returnStatements: [],
           switchStatements: [],
           switchLastCaseReachable: new Map(),
+          lastCallExpression: null,
         });
         allCurrentSegments.push(currentSegments);
         currentSegments = new Set();
@@ -217,6 +228,16 @@ export const rule: Rule.RuleModule = {
           currentContext.switchLastCaseReachable.set(switchStmt, hasReachableExit);
         }
       },
+      ExpressionStatement(node: estree.Node) {
+        const currentContext = functionContextStack.at(-1);
+        if (currentContext) {
+          const expr = (node as estree.ExpressionStatement).expression;
+          if (expr.type === 'CallExpression') {
+            // Track any call expression - we'll check if it returns 'never' later
+            currentContext.lastCallExpression = expr;
+          }
+        }
+      },
       'FunctionDeclaration:exit': checkOnFunctionExit,
       'FunctionExpression:exit': checkOnFunctionExit,
       'ArrowFunctionExpression:exit': checkOnFunctionExit,
@@ -301,3 +322,21 @@ function isEnumType(type: ts.Type): boolean {
     type.symbol?.flags === ts.SymbolFlags.EnumMember
   );
 }
+
+/**
+ * Checks if a call expression returns 'never' (e.g., a function that always throws).
+ * This helps detect when an implicit return is actually unreachable because the
+ * last statement calls a function that never returns.
+ */
+function isNeverReturningCall(
+  callExpr: estree.CallExpression,
+  services: RequiredParserServices,
+): boolean {
+  const signature = getSignatureFromCallee(callExpr, services);
+  if (!signature) {
+    return false;
+  }
+
+  const returnType = signature.getReturnType();
+  return (returnType.flags & ts.TypeFlags.Never) !== 0;
+}

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

@@ -294,7 +294,8 @@ describe('S3801', () => {
         }`,
           errors: 1,
         },
-        // possible FP, see https://github.com/SonarSource/SonarJS/issues/2579
+        // FP without type information - we can't detect that throwError returns 'never'
+        // See JS-106. With type information, this is now fixed (see type-aware tests below)
         {
           code: `
       function throwError(message: string): never {
@@ -448,8 +449,53 @@ describe('S3801', () => {
             }
           `,
         },
+        {
+          // Issue JS-106: Calling a function that returns 'never' - should not raise
+          // The function terminates via the never-returning call, so no implicit return
+          code: `
+            function throwError(message: string): never {
+              throw new Error(message);
+            }
+            function formatDateOrThrow(value: unknown): string {
+              if (value instanceof Date) {
+                return value.toISOString();
+              }
+              throwError('Invalid date');
+            }
+          `,
+        },
+        {
+          // Calling a never-returning function without explicit return type on caller
+          code: `
+            function throwError(message: string): never {
+              throw new Error(message);
+            }
+            function withNeverType(a) {
+              if (a === 1) {
+                return true;
+              }
+              throwError('False');
+            }
+          `,
+        },
       ],
       invalid: [
+        {
+          // Calling a function without explicit 'never' return type - should raise
+          // TypeScript infers 'void' not 'never' for functions that only throw
+          code: `
+            function throwErrorNoAnnotation() {
+              throw new Error('error');
+            }
+            function test(a) {
+              if (a === 1) {
+                return true;
+              }
+              throwErrorNoAnnotation();
+            }
+          `,
+          errors: 1,
+        },
         {
           // Non-exhaustive switch - should raise
           code: `