add rule no-redundant-jump (#130)

README.md

@@ -30,6 +30,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
 * Functions should not have identical implementations ([`no-identical-functions`])
 * Boolean checks should not be inverted ([`no-inverted-boolean-check`])
 * Boolean literals should not be redundant ([`no-redundant-boolean`])
+* Jump statements should not be redundant ([`no-redundant-jump`])
 * Conditionals should start on new lines ([`no-same-line-conditional`])
 * "switch" statements should have at least 3 "case" clauses ([`no-small-switch`])
 * "catch" clauses should do more than rethrow ([`no-useless-catch`])
@@ -52,6 +53,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
 [`no-inverted-boolean-check`]: ./docs/rules/no-inverted-boolean-check.md
 [`no-one-iteration-loop`]: ./docs/rules/no-one-iteration-loop.md
 [`no-redundant-boolean`]: ./docs/rules/no-redundant-boolean.md
+[`no-redundant-jump`]: ./docs/rules/no-redundant-jump.md
 [`no-same-line-conditional`]: ./docs/rules/no-same-line-conditional.md
 [`no-small-switch`]: ./docs/rules/no-small-switch.md
 [`no-use-of-empty-return-value`]: ./docs/rules/no-use-of-empty-return-value.md

docs/rules/no-redundant-jump.md

@@ -0,0 +1,28 @@
+# no-redundant-jump
+
+Jump statements, such as `return`, `break` and `continue` let you change the default flow of program execution, but jump statements that direct the control flow to the original direction are just a waste of keystrokes.
+
+## Noncompliant Code Example
+
+```javascript
+function redundantJump(x) {
+  if (x == 1) {
+    console.log("x == 1");
+    return; // Noncompliant
+  }
+}
+```
+
+## Compliant Solution
+
+```javascript
+function redundantJump(x) {
+  if (x == 1) {
+    console.log("x == 1");
+  }
+}
+```
+
+## Exceptions
+
+`break` and `return` inside switch statement are ignored, because they are often used for consistency. continue with label is also ignored, because label is usually used for clarity. Also a jump statement being a single statement in a block is ignored.

ruling/snapshots/no-redundant-jump

@@ -0,0 +1,53 @@
+src/angular.js/src/ngTouch/swipe.js: 167
+src/brackets/src/extensibility/node/package-validator.js: 275
+src/brackets/src/extensions/default/CodeFolding/unittests.js: 167
+src/brackets/src/LiveDevelopment/MultiBrowserImpl/documents/LiveHTMLDocument.js: 241,252,263,274,285
+src/brackets/src/LiveDevelopment/MultiBrowserImpl/protocol/remote/LiveDevProtocolRemote.js: 88
+src/brackets/src/preferences/PreferencesBase.js: 618,638,733,754
+src/brackets/src/search/node/FindInFilesDomain.js: 283
+src/brackets/src/utils/DragAndDrop.js: 84
+src/create-react-app/packages/react-dev-utils/openBrowser.js: 58
+src/express/lib/router/index.js: 254
+src/react-native/bots/code-analysis-bot.js: 170
+src/react-native/Libraries/Lists/VirtualizedList.js: 1363
+src/react-native/local-cli/link/ios/removeFromPbxItemContainerProxySection.js: 22
+src/react-native/local-cli/link/ios/removeFromPbxReferenceProxySection.js: 21
+src/react-native/local-cli/link/ios/removeProductGroup.js: 17
+src/spectrum/athena/models/slackImports.js: 22
+src/spectrum/athena/queues/new-message-in-thread/group-replies.js: 41,44
+src/spectrum/pluto/changefeeds/openSourceStatus.js: 43
+src/spectrum/pluto/changefeeds/privateChannel.js: 27,40,79,101
+src/spectrum/pluto/queues/processAnalyticsRemoved.js: 63
+src/spectrum/pluto/queues/processModeratorRemoved.js: 92
+src/spectrum/pluto/queues/processOssStatusActivated.js: 155
+src/spectrum/pluto/queues/processOssStatusDisabled.js: 165
+src/spectrum/pluto/queues/processOssStatusEnabled.js: 155
+src/spectrum/pluto/queues/processPrioritySupportRemoved.js: 64
+src/spectrum/pluto/queues/processPrivateChannelRemoved.js: 92
+src/spectrum/pluto/webhooks/index.js: 65
+src/spectrum/src/components/composer/index.js: 485
+src/spectrum/src/components/editForm/user.js: 301
+src/spectrum/src/components/modals/ChangeChannelModal/index.js: 63
+src/spectrum/src/components/modals/CreateChannelModal/index.js: 247
+src/spectrum/src/components/modals/DeleteDoubleCheckModal/index.js: 95,123,152,181
+src/spectrum/src/components/modals/UpgradeModal/index.js: 80
+src/spectrum/src/components/profile/channel.js: 85
+src/spectrum/src/components/threadComposer/components/composer.js: 571
+src/spectrum/src/components/toggleChannelMembership/index.js: 85
+src/spectrum/src/components/toggleChannelNotifications/index.js: 62
+src/spectrum/src/components/upsell/joinChannel.js: 82
+src/spectrum/src/components/upsell/requestToJoinChannel.js: 73
+src/spectrum/src/helpers/directMessageThreads.js: 21
+src/spectrum/src/views/channel/components/notificationsToggle.js: 57
+src/spectrum/src/views/channelSettings/components/editForm.js: 130
+src/spectrum/src/views/channelSettings/index.js: 62,86
+src/spectrum/src/views/communityMembers/components/importSlack.js: 85
+src/spectrum/src/views/communitySettings/components/editForm.js: 236
+src/spectrum/src/views/directMessages/containers/newThread.js: 194,376,385,680
+src/spectrum/src/views/newCommunity/components/createCommunityForm/index.js: 401
+src/spectrum/src/views/newCommunity/components/editCommunityForm/index.js: 214
+src/spectrum/src/views/notifications/index.js: 106
+src/spectrum/vulcan/models/community.js: 21,36,55
+src/spectrum/vulcan/models/message.js: 25,40
+src/spectrum/vulcan/models/thread.js: 29,44,92,111
+src/spectrum/vulcan/models/user.js: 26,41,58,75

src/index.ts

@@ -34,6 +34,7 @@ const sonarjsRules: [string, Linter.RuleLevel][] = [
   ["no-inverted-boolean-check", "error"],
   ["no-one-iteration-loop", "error"],
   ["no-redundant-boolean", "error"],
+  ["no-redundant-jump", "error"],
   ["no-same-line-conditional", "error"],
   ["no-small-switch", "error"],
   ["no-use-of-empty-return-value", "error"],

src/rules/no-redundant-jump.ts

@@ -0,0 +1,75 @@
+/*
+ * eslint-plugin-sonarjs
+ * Copyright (C) 2018 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+// https://jira.sonarsource.com/browse/RSPEC-3626
+
+import { Rule } from "eslint";
+import * as estree from "estree";
+import { getParent } from "../utils/nodes";
+
+const message = "Remove this redundant jump.";
+const loops = "WhileStatement, ForStatement, DoWhileStatement, ForInStatement, ForOfStatement";
+
+const rule: Rule.RuleModule = {
+  create(context: Rule.RuleContext) {
+    function reportIfLastStatement(node: estree.ContinueStatement | estree.ReturnStatement) {
+      const withArgument = node.type === "ContinueStatement" ? !!node.label : !!node.argument;
+      if (!withArgument) {
+        const block = getParent(context) as estree.BlockStatement;
+        if (block.body[block.body.length - 1] === node && block.body.length > 1) {
+          context.report({
+            message,
+            node,
+          });
+        }
+      }
+    }
+
+    function reportIfLastStatementInsideIf(node: estree.ContinueStatement | estree.ReturnStatement) {
+      const ancestors = context.getAncestors();
+      const ifStatement = ancestors[ancestors.length - 2];
+      const upperBlock = ancestors[ancestors.length - 3] as estree.BlockStatement;
+      if (upperBlock.body[upperBlock.body.length - 1] === ifStatement) {
+        reportIfLastStatement(node);
+      }
+    }
+
+    return {
+      [`:matches(${loops}) > BlockStatement > ContinueStatement`]: (node: estree.Node) => {
+        reportIfLastStatement(node as estree.ContinueStatement);
+      },
+
+      [`:matches(${loops}) > BlockStatement > IfStatement > BlockStatement > ContinueStatement`]: (
+        node: estree.Node,
+      ) => {
+        reportIfLastStatementInsideIf(node as estree.ContinueStatement);
+      },
+
+      ":function > BlockStatement > ReturnStatement": (node: estree.Node) => {
+        reportIfLastStatement(node as estree.ReturnStatement);
+      },
+
+      ":function > BlockStatement > IfStatement > BlockStatement > ReturnStatement": (node: estree.Node) => {
+        reportIfLastStatementInsideIf(node as estree.ReturnStatement);
+      },
+    };
+  },
+};
+
+export = rule;

tests/rules/no-redundant-jump.test.ts

@@ -0,0 +1,155 @@
+/*
+ * eslint-plugin-sonarjs
+ * Copyright (C) 2018 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+import { RuleTester } from "eslint";
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } });
+import rule = require("../../src/rules/no-redundant-jump");
+
+function invalid(code: string) {
+  const line = code.split("\n").findIndex(str => str.includes("// Noncompliant")) + 1;
+  return {
+    code,
+    errors: [
+      {
+        message: "Remove this redundant jump.",
+        line,
+        endLine: line,
+      },
+    ],
+  };
+}
+
+ruleTester.run("Jump statements should not be redundant", rule, {
+  invalid: [
+    invalid(
+      `while (x == 1) {
+        console.log("x == 1");
+        continue; // Noncompliant
+      }`,
+    ),
+    invalid(
+      `function redundantJump(condition1, condition2) {
+        while (condition1) {
+          if (condition2) {
+            console.log("Hello");
+            continue;  // Noncompliant
+          } else {
+            console.log("else");
+          }
+        }
+      }`,
+    ),
+    invalid(
+      `function redundantJump(condition1, condition2) {
+        while (condition1) {
+          if (condition2) {
+            console.log("then");
+          } else {
+            console.log("else");
+            continue;  // Noncompliant
+          }
+        }
+      }`,
+    ),
+    invalid(
+      `function redundantJump() {
+        for (let i = 0; i < 10; i++) {
+          console.log("Hello");
+          continue; // Noncompliant
+        }
+      }`,
+    ),
+    invalid(
+      `function redundantJump(b) {
+        if (b) {
+          console.log("b");
+          return; // Noncompliant
+        }
+      }`,
+    ),
+    invalid(
+      `function redundantJump(x) {
+          console.log("x == 1");
+          return; // Noncompliant
+      }`,
+    ),
+    invalid(
+      `const redundantJump = (x) => {
+          console.log("x == 1");
+          return; // Noncompliant
+      }`,
+    ),
+  ],
+  valid: [
+    {
+      code: `
+      function return_with_value() {
+        foo();
+        return 42;
+      }
+
+      function switch_statements(x) {
+        switch (x) {
+          case 0:
+            foo();
+            break;
+          default:
+        }
+        foo();
+        switch (x) {
+          case 0:
+            foo();
+            return;
+          case 1:
+            bar();
+            return;
+        }
+      }
+
+      function loops_with_label() {
+        for (let i = 0; i < 10; i++) {
+          inner: for (let j = 0; j < 10; j++) {
+            continue inner;
+          }
+        }
+      }
+
+      function compliant(b) {
+        for (let i = 0; i < 10; i++) {
+          break;
+        }
+        if (b) {
+          console.log("b");
+          return;
+        }
+        console.log("useful");
+      }
+
+      while (x == 1) {
+        continue; // Ok, we ignore when 1 statement
+      }
+
+      function bar() {
+        return; // Ok, we ignore when 1 statement
+      }
+      `,
+    },
+  ],
+});