ESLINTJS-17 Allow functions function names for prefer-immediate-return (#5581)

Author: zglicz

.cirrus.template.yml

@@ -415,7 +415,8 @@ js_ts_ruling_task:
     - npm run build:fast
     - npm run ruling
   on_failure:
-    debug_script: diff -rq its/ruling/src/test/expected/jsts packages/ruling/actual/jsts
+    debug_script:
+      - sh tools/ruling-debug-script.sh packages/ruling/actual/jsts
 
 ruling_task:
   depends_on:
@@ -435,8 +436,8 @@ ruling_task:
     - mvn test -Dtest=JsTsRulingTest -DskipTests=false -Dsonar.runtimeVersion=LATEST_RELEASE -Dmaven.test.redirectTestOutputToFile=false -Djunit.jupiter.execution.parallel.config.dynamic.factor=1 -B -e -V
   cleanup_before_cache_script: cleanup_maven_repository
   on_failure:
-    diff_artifacts:
-      path: '**/target/actual/**/*'
+    debug_script:
+      - sh tools/ruling-debug-script.sh its/ruling/target/actual/jsts
 
 css_ruling_task:
   depends_on:

.cirrus.yml

@@ -52,7 +52,7 @@ into the directory with the expected issues `its/ruling/src/test/resources/expec
 
 From the project root, run: `npm run ruling-sync`
 
-You can review the Ruling difference by running `diff -rq its/ruling/src/test/expected/jsts packages/ruling/actual/jsts`.
+You can review the Ruling difference by running `sh tools/ruling-debug-script.sh`.
 For CSS, run `diff -rq its/ruling/src/test/expected/css `
 
 #### CSS (and old way for JS/TS)

docs/DEV.md

@@ -5,12 +5,6 @@
 "ace:experiments/animate_folding.html": [
 241
 ],
-"ace:lib/ace/mode/css/csslint.js": [
-4287
-],
-"ace:lib/ace/mode/html/saxparser.js": [
-5907
-],
 "ace:src/editor.js": [
 1598
 ],

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

@@ -2,18 +2,9 @@
 "ant-design:components/breadcrumb/Breadcrumb.tsx": [
 38
 ],
-"ant-design:components/calendar/generateCalendar.tsx": [
-82
-],
 "ant-design:components/card/Card.tsx": [
 46
 ],
-"ant-design:components/message/hooks/useMessage.tsx": [
-20
-],
-"ant-design:components/notification/hooks/useNotification.tsx": [
-19
-],
 "ant-design:components/steps/index.tsx": [
 83
 ],

its/ruling/src/test/expected/jsts/ant-design/typescript-S1488.json

@@ -9,16 +9,14 @@
 ],
 "p5.js:lib/addons/p5.sound.js": [
 1237,
-1527,
 1803,
 3823,
 3832,
 3912,
 4401,
 5264,
 7879,
-8000,
-12876
+8000
 ],
 "p5.js:src/dom/dom.js": [
 1795,

its/ruling/src/test/expected/jsts/p5.js/javascript-S1488.json

@@ -17,7 +17,12 @@
 // https://sonarsource.github.io/rspec/#/rspec/S1488
 
 import type { TSESTree } from '@typescript-eslint/utils';
-import { generateMeta, isIdentifier, isRequiredParserServices } from '../helpers/index.js';
+import {
+  generateMeta,
+  isFunction,
+  isIdentifier,
+  isRequiredParserServices,
+} from '../helpers/index.js';
 import type { Rule } from 'eslint';
 import type estree from 'estree';
 import * as meta from './generated-meta.js';
@@ -59,8 +64,10 @@ export const rule: Rule.RuleModule = {
             );
           });
 
-          if (hasJSDoc((lastButOne as estree.VariableDeclaration).declarations[0].id)) {
-            // The temporary object is probably due to adding the jsdoc. Skip in this case.
+          const id = (lastButOne as estree.VariableDeclaration).declarations[0].id;
+          if (hasJSDoc(id) || isNamedFunctionExpression(id)) {
+            // 1) The temporary variable might be due to adding the jsdoc. Skip in this case.
+            // 2) The temporary variable might be a name of a function, which helps with recognizing in a call stack.
             return;
           }
 
@@ -135,5 +142,13 @@ export const rule: Rule.RuleModule = {
       }
       return !!getJSDocType(services.esTreeNodeToTSNodeMap.get(node as TSESTree.Node));
     }
+
+    function isNamedFunctionExpression(node: estree.Node) {
+      const services = context.sourceCode.parserServices;
+      if (!isRequiredParserServices(services)) {
+        return false;
+      }
+      return isFunction(node, services);
+    }
   },
 };

its/ruling/src/test/expected/jsts/qunit/javascript-S1488.json

@@ -27,12 +27,20 @@ describe('S1488', () => {
       valid: [
         {
           code: `const schemas = schemaFileNames.map((schemaFileName) => {
-        /** @type {import('ajv').AnySchema} 
+        /** @type {import('ajv').AnySchema}
         **/
         const result = parseJson(fs.readFileSync(path.join(schemaDir, schemaFileName), {encoding: 'utf8'}));
         return result;
     });`,
         },
+        {
+          code: `const makeFooHandler = () => {
+  const fooHandler = () => {
+    //
+  }
+  return fooHandler
+}`,
+        },
       ],
       invalid: [
         {
@@ -609,6 +617,20 @@ describe('S1488', () => {
         return parseJson(fs.readFileSync(path.join(schemaDir, schemaFileName), {encoding: 'utf8'}));
     });`,
         },
+        {
+          code: `const makeFooHandler = () => {
+  const fooHandler = () => {
+    //
+  }
+  return fooHandler
+}`,
+          errors: 1,
+          output: `const makeFooHandler = () => {
+  return () => {
+    //
+  }
+}`,
+        },
       ],
     });
   });