improve 'prefer-single-boolean-return': report when next statement returns boolean (#324)

Co-authored-by: Dimitri POSTOLOV <[email protected]>

Author: vilchik-elena

docs/rules/no-empty-collection.md

@@ -11,5 +11,5 @@ if (strings.includes("foo")) {}  // Noncompliant
 
 for (str of strings) {}  // Noncompliant
 
-strings.forEach(str => doSomething(str)); // Noncompliant
+strings.forEach(str => doSomething(str)); // Noncompliant
 ```

docs/rules/prefer-single-boolean-return.md

@@ -1,6 +1,6 @@
 # prefer-single-boolean-return
 
-Return of boolean literal statements wrapped into `if-then-else` ones should be simplified.
+Return of boolean literal statements wrapped into `if-then-else` flow should be simplified.
 
 ## Noncompliant Code Example
 
@@ -12,6 +12,15 @@ if (expression) {
 }
 ```
 
+or
+
+```javascript
+if (expression) {
+  return true;
+}
+return false;
+```
+
 ## Compliant Solution
 
 ```javascript

ruling/snapshots/prefer-single-boolean-return

@@ -1,5 +1,58 @@
 src/Ghost/core/server/adapters/storage/utils.js: 39
 src/Ghost/core/server/api/notifications.js: 73
-src/brackets/src/extensions/default/DebugCommands/main.js: 345
+src/Ghost/core/server/config/utils.js: 13
+src/Ghost/core/server/update-check.js: 237
+src/Ghost/core/server/web/utils.js: 45
+src/angular.js/src/ngMock/angular-mocks.js: 1980
+src/angular.js/src/ngParseExt/ucd.js: 561,1215
+src/brackets/src/JSUtils/ScopeManager.js: 234,262
+src/brackets/src/LiveDevelopment/Documents/HTMLDocument.js: 255
+src/brackets/src/command/KeyBindingManager.js: 1069
+src/brackets/src/extensibility/node/package-validator.js: 74
+src/brackets/src/extensions/default/DebugCommands/main.js: 345,433
+src/brackets/src/extensions/default/InlineTimingFunctionEditor/TimingFunctionUtils.js: 157,239
+src/brackets/src/extensions/default/JavaScriptCodeHints/main.js: 784
+src/brackets/src/extensions/default/JavaScriptRefactoring/RefactoringUtils.js: 189
+src/brackets/src/extensions/default/PrefsCodeHints/main.js: 201,369
+src/brackets/src/extensions/default/UrlCodeHints/main.js: 308
+src/brackets/src/language/CSSUtils.js: 1130
+src/brackets/src/preferences/PreferencesBase.js: 1143
 src/brackets/src/search/FindUtils.js: 431
+src/brackets/src/utils/ValidationUtils.js: 46
+src/create-react-app/packages/react-dev-utils/eslintFormatter.js: 15
+src/jest/packages/expect/src/utils.js: 163
+src/jest/packages/jest-cli/src/cli/index.js: 252
+src/jest/packages/jest-cli/src/run_jest.js: 161
+src/jest/packages/jest-message-util/src/index.js: 188
+src/jest/packages/jest-runtime/src/should_instrument.js: 78
+src/jquery/external/qunit/qunit.js: 1703
+src/jquery/external/sinon/sinon.js: 220
+src/react-native/Libraries/Components/ScrollResponder.js: 179
+src/react-native/Libraries/Renderer/ReactFabric-dev.js: 10866
+src/react-native/Libraries/Renderer/ReactNativeRenderer-dev.js: 11236
+src/react/packages/react-dom/src/shared/DOMProperty.js: 101
+src/react/packages/react-reconciler/src/ReactFiberHydrationContext.js: 272
+src/react/packages/shared/isTextInputElement.js: 38
+src/reveal.js/js/reveal.js: 5203
+src/spectrum/admin/src/helpers/utils.js: 41
+src/spectrum/api/models/invoice.js: 83
+src/spectrum/api/queries/channel/isArchived.js: 5
+src/spectrum/api/queries/directMessageThread/utils.js: 16
+src/spectrum/api/queries/meta/isAdmin.js: 6
+src/spectrum/src/components/chatInput/index.js: 101
+src/spectrum/src/components/composer/utils.js: 25
 src/spectrum/src/components/message/index.js: 17
+src/spectrum/src/components/threadFeed/index.js: 156
+src/spectrum/src/helpers/utils.js: 85
+src/spectrum/src/views/community/components/memberGrid.js: 54
+src/spectrum/src/views/community/components/moderatorList.js: 36
+src/spectrum/src/views/dashboard/components/communityList.js: 68
+src/spectrum/src/views/dashboard/components/threadFeed.js: 81
+src/spectrum/src/views/dashboard/components/upsellExploreCommunities.js: 48
+src/spectrum/src/views/directMessages/index.js: 63
+src/spectrum/src/views/navbar/components/messagesTab.js: 64
+src/spectrum/src/views/navbar/components/notificationsTab.js: 109
+src/spectrum/src/views/navbar/index.js: 70
+src/spectrum/src/views/notifications/index.js: 120
+src/spectrum/src/views/pages/pricing/components/communityList.js: 43
+src/three.js/src/math/Ray.js: 352

src/rules/prefer-single-boolean-return.ts

@@ -30,8 +30,7 @@ import docsUrl from '../utils/docs-url';
 const rule: TSESLint.RuleModule<string, string[]> = {
   meta: {
     messages: {
-      replaceIfThenElseByReturn:
-        'Replace this if-then-else statement by a single return statement.',
+      replaceIfThenElseByReturn: 'Replace this if-then-else flow by a single return statement.',
     },
     schema: [],
     type: 'suggestion',
@@ -44,23 +43,35 @@ const rule: TSESLint.RuleModule<string, string[]> = {
   },
   create(context) {
     return {
-      IfStatement(node: TSESTree.Node) {
-        const ifStmt = node as TSESTree.IfStatement;
+      IfStatement(node: TSESTree.IfStatement) {
         if (
           // ignore `else if`
-          !isIfStatement(ifStmt.parent) &&
-          // `ifStmt.alternate` can be `null`, replace it with `undefined` in this case
-          returnsBoolean(ifStmt.alternate || undefined) &&
-          returnsBoolean(ifStmt.consequent)
+          !isIfStatement(node.parent) &&
+          returnsBoolean(node.consequent) &&
+          alternateReturnsBoolean(node)
         ) {
           context.report({
             messageId: 'replaceIfThenElseByReturn',
-            node: ifStmt,
+            node,
           });
         }
       },
     };
 
+    function alternateReturnsBoolean(node: TSESTree.IfStatement) {
+      if (node.alternate) {
+        return returnsBoolean(node.alternate);
+      }
+
+      const { parent } = node;
+      if (parent?.type === 'BlockStatement') {
+        const ifStmtIndex = parent.body.findIndex(stmt => stmt === node);
+        return isSimpleReturnBooleanLiteral(parent.body[ifStmtIndex + 1]);
+      }
+
+      return false;
+    }
+
     function returnsBoolean(statement: TSESTree.Statement | undefined) {
       return (
         statement !== undefined &&
@@ -76,7 +87,7 @@ const rule: TSESLint.RuleModule<string, string[]> = {
       );
     }
 
-    function isSimpleReturnBooleanLiteral(statement: TSESTree.Statement) {
+    function isSimpleReturnBooleanLiteral(statement: TSESTree.Node) {
       // `statement.argument` can be `null`, replace it with `undefined` in this case
       return isReturnStatement(statement) && isBooleanLiteral(statement.argument || undefined);
     }

tests/rules/prefer-single-boolean-return.test.ts

@@ -186,5 +186,46 @@ ruleTester.run('prefer-single-boolean-return', rule, {
         },
       ],
     },
+    {
+      code: `
+        function fn() {
+          if (foo) {
+            if (something) {
+              return true
+            }
+            return false
+          }
+
+          if (bar) {
+            if (something) {
+              return false
+            }
+            return true
+          }
+
+          if (baz) {
+            if (something) {
+              return false
+            }
+          }
+        }
+      `,
+      errors: [
+        {
+          messageId: 'replaceIfThenElseByReturn',
+          line: 4,
+          column: 13,
+          endLine: 6,
+          endColumn: 14,
+        },
+        {
+          messageId: 'replaceIfThenElseByReturn',
+          line: 11,
+          column: 13,
+          endLine: 13,
+          endColumn: 14,
+        },
+      ],
+    },
   ],
 });