Fix suggestion for 'prefer-single-boolean-return' (#344)

Author: yassin-kammoun-sonarsource

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

@@ -32,6 +32,8 @@ const rule: TSESLint.RuleModule<string, string[]> = {
     messages: {
       replaceIfThenElseByReturn: 'Replace this if-then-else flow by a single return statement.',
       suggestIfThenElseReplacement: 'Replace with single return statement',
+      suggestUnsafeIfThenElseReplacement:
+        'Replace with single return statement (beware of return value usages)',
     },
     schema: [],
     type: 'suggestion',
@@ -95,24 +97,61 @@ const rule: TSESLint.RuleModule<string, string[]> = {
       return isReturnStatement(statement) && isBooleanLiteral(statement.argument || undefined);
     }
 
-    function getSuggestion(ifStmt: TSESTree.IfStatement): TSESLint.ReportSuggestionArray<string> {
-      return [
+    function getSuggestion(ifStmt: TSESTree.IfStatement) {
+      const getFix = (condition: string) => {
+        return (fixer: TSESLint.RuleFixer) => {
+          const singleReturn = `return ${condition};`;
+          if (ifStmt.alternate) {
+            return fixer.replaceText(ifStmt, singleReturn);
+          } else {
+            const parent = ifStmt.parent as TSESTree.BlockStatement;
+            const ifStmtIndex = parent.body.findIndex(stmt => stmt === ifStmt);
+            const returnStmt = parent.body[ifStmtIndex + 1];
+            const range: [number, number] = [ifStmt.range[0], returnStmt.range[1]];
+            return fixer.replaceTextRange(range, singleReturn);
+          }
+        };
+      };
+      const shouldNegate = isReturningFalse(ifStmt.consequent);
+      const shouldCast = !isBooleanExpression(ifStmt.test);
+      const testText = context.getSourceCode().getText(ifStmt.test);
+      let safeCondition: string;
+      if (shouldNegate) {
+        safeCondition = `!(${testText})`;
+      } else if (shouldCast) {
+        safeCondition = `!!(${testText})`;
+      } else {
+        safeCondition = testText;
+      }
+      const suggestions: TSESLint.ReportSuggestionArray<string> = [
         {
           messageId: 'suggestIfThenElseReplacement',
-          fix: fixer => {
-            const singleReturn = `return ${context.getSourceCode().getText(ifStmt.test)};`;
-            if (ifStmt.alternate) {
-              return fixer.replaceText(ifStmt, singleReturn);
-            } else {
-              const parent = ifStmt.parent as TSESTree.BlockStatement;
-              const ifStmtIndex = parent.body.findIndex(stmt => stmt === ifStmt);
-              const returnStmt = parent.body[ifStmtIndex + 1];
-              const range: [number, number] = [ifStmt.range[0], returnStmt.range[1]];
-              return fixer.replaceTextRange(range, singleReturn);
-            }
-          },
+          fix: getFix(safeCondition),
         },
       ];
+      if (shouldCast && !shouldNegate) {
+        suggestions.push({
+          messageId: 'suggestUnsafeIfThenElseReplacement',
+          fix: getFix(testText),
+        });
+      }
+      return suggestions;
+    }
+
+    function isReturningFalse(stmt: TSESTree.Statement): boolean {
+      const returnStmt = (
+        stmt.type === 'BlockStatement' ? stmt.body[0] : stmt
+      ) as TSESTree.ReturnStatement;
+      return (returnStmt.argument as TSESTree.Literal).value === false;
+    }
+
+    function isBooleanExpression(expr: TSESTree.Expression) {
+      return (
+        (expr.type === 'UnaryExpression' || expr.type === 'BinaryExpression') &&
+        ['!', '==', '===', '!=', '!==', '<', '<=', '>', '>=', 'in', 'instanceof'].includes(
+          expr.operator,
+        )
+      );
     }
   },
 };

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

@@ -246,6 +246,16 @@ function foo() {
             {
               messageId: 'suggestIfThenElseReplacement',
               output: `
+function foo() {
+  if (bar()) {
+    return !!(baz());
+  }
+  return qux();
+}`,
+            },
+            {
+              messageId: 'suggestUnsafeIfThenElseReplacement',
+              output: `
 function foo() {
   if (bar()) {
     return baz();
@@ -275,11 +285,117 @@ function foo() {
             {
               messageId: 'suggestIfThenElseReplacement',
               output: `
+function foo() {
+  if (bar()) {
+    return !!(baz());
+  }
+  return qux();
+}`,
+            },
+            {
+              messageId: 'suggestUnsafeIfThenElseReplacement',
+              output: `
 function foo() {
   if (bar()) {
     return baz();
   }
   return qux();
+}`,
+            },
+          ],
+        },
+      ],
+    },
+    {
+      code: `
+function foo() {
+  if (!bar()) {
+    return true;
+  } else {
+    return false;
+  }
+}`,
+      errors: [
+        {
+          messageId: 'replaceIfThenElseByReturn',
+          suggestions: [
+            {
+              messageId: 'suggestIfThenElseReplacement',
+              output: `
+function foo() {
+  return !bar();
+}`,
+            },
+          ],
+        },
+      ],
+    },
+    {
+      code: `
+function foo() {
+  if (bar() > 0) {
+    return true;
+  } else {
+    return false;
+  }
+}`,
+      errors: [
+        {
+          messageId: 'replaceIfThenElseByReturn',
+          suggestions: [
+            {
+              messageId: 'suggestIfThenElseReplacement',
+              output: `
+function foo() {
+  return bar() > 0;
+}`,
+            },
+          ],
+        },
+      ],
+    },
+    {
+      code: `
+function foo() {
+  if (baz() > 0) {
+    return false;
+  } else {
+    return true;
+  }
+}`,
+      errors: [
+        {
+          messageId: 'replaceIfThenElseByReturn',
+          suggestions: [
+            {
+              messageId: 'suggestIfThenElseReplacement',
+              output: `
+function foo() {
+  return !(baz() > 0);
+}`,
+            },
+          ],
+        },
+      ],
+    },
+    {
+      code: `
+function foo() {
+  if (baz()) {
+    return false;
+  } else {
+    return true;
+  }
+}`,
+      errors: [
+        {
+          messageId: 'replaceIfThenElseByReturn',
+          suggestions: [
+            {
+              messageId: 'suggestIfThenElseReplacement',
+              output: `
+function foo() {
+  return !(baz());
 }`,
             },
           ],