add rule no-collapsible-if (#110)

Author: andrea-guarino-sonarsource

README.md

@@ -24,6 +24,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
 
 * Cognitive Complexity of functions should not be too high ([`cognitive-complexity`])
 * "switch" statements should not have too many "case" clauses ([`max-switch-cases`])
+* Collapsible "if" statements should be merged ([`no-collapsible-if`])
 * String literals should not be duplicated ([`no-duplicate-string`])
 * Two branches in a conditional structure should not have exactly the same implementation ([`no-duplicated-branches`])
 * Functions should not have identical implementations ([`no-identical-functions`])
@@ -39,6 +40,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
 [`cognitive-complexity`]: ./docs/rules/cognitive-complexity.md
 [`max-switch-cases`]: ./docs/rules/max-switch-cases.md
 [`no-all-duplicated-branches`]: ./docs/rules/no-all-duplicated-branches.md
+[`no-collapsible-if`]: ./docs/rules/no-collapsible-if.md
 [`no-duplicate-string`]: ./docs/rules/no-duplicate-string.md
 [`no-duplicated-branches`]: ./docs/rules/no-duplicated-branches.md
 [`no-element-overwrite`]: ./docs/rules/no-element-overwrite.md

docs/rules/no-collapsible-if.md

@@ -0,0 +1,21 @@
+# no-collapsible-if
+
+Merging collapsible if statements increases the code's readability.
+
+## Noncompliant Code Example
+
+```javascript
+if (x != undefined) {
+  if (y === 2) {
+    // ...
+  }
+}
+```
+
+## Compliant Solution
+
+```javascript
+if (x != undefined && y === 2) {
+  // ...
+}
+```

ruling/snapshots/no-collapsible-if

@@ -0,0 +1,148 @@
+src/angular.js/src/ng/location.js: 933
+src/angular.js/src/ng/parse.js: 1597,1614
+src/angular.js/src/ngRoute/route.js: 661,662
+src/brackets/src/command/KeyBindingManager.js: 685,707
+src/brackets/src/command/Menus.js: 675,1015
+src/brackets/src/document/Document.js: 166,183,302
+src/brackets/src/document/DocumentCommandHandlers.js: 877
+src/brackets/src/document/DocumentManager.js: 128
+src/brackets/src/editor/CodeHintManager.js: 592
+src/brackets/src/editor/Editor.js: 876,2435
+src/brackets/src/editor/ImageViewer.js: 332
+src/brackets/src/extensions/default/CSSCodeHints/main.js: 98,166
+src/brackets/src/extensions/default/DebugCommands/main.js: 495
+src/brackets/src/extensions/default/HTMLCodeHints/main.js: 91,135
+src/brackets/src/extensions/default/InlineColorEditor/ColorEditor.js: 454,686,687
+src/brackets/src/extensions/default/InlineColorEditor/InlineColorEditor.js: 267
+src/brackets/src/extensions/default/InlineTimingFunctionEditor/BezierCurveEditor.js: 168
+src/brackets/src/extensions/default/InlineTimingFunctionEditor/StepEditor.js: 180
+src/brackets/src/extensions/default/JavaScriptCodeHints/main.js: 486
+src/brackets/src/extensions/default/JavaScriptCodeHints/thirdparty/requirejs/require.js: 12,16
+src/brackets/src/extensions/default/JavaScriptQuickEdit/unittests.js: 152
+src/brackets/src/extensions/default/NavigationAndHistory/NavigationProvider.js: 341
+src/brackets/src/extensions/default/QuickView/main.js: 604
+src/brackets/src/extensions/default/StaticServer/node/node_modules/connect/node_modules/multiparty/node_modules/readable-stream/zlib.js: 232,239,246,253,260,270
+src/brackets/src/extensions/default/UrlCodeHints/main.js: 115,345
+src/brackets/src/filesystem/FileIndex.js: 122
+src/brackets/src/JSUtils/ScopeManager.js: 541
+src/brackets/src/language/CSSUtils.js: 1098,1393,1654
+src/brackets/src/language/HTMLDOMDiff.js: 406,484
+src/brackets/src/language/HTMLSimpleDOM.js: 460
+src/brackets/src/language/HTMLUtils.js: 137
+src/brackets/src/language/JSUtils.js: 137,149,159
+src/brackets/src/language/XMLUtils.js: 78,90,91,103,241
+src/brackets/src/LiveDevelopment/LiveDevMultiBrowser.js: 549,742
+src/brackets/src/LiveDevelopment/MultiBrowserImpl/language/HTMLInstrumentation.js: 149
+src/brackets/src/LiveDevelopment/MultiBrowserImpl/language/HTMLSimpleDOM.js: 422
+src/brackets/src/project/FileSyncManager.js: 128
+src/brackets/src/search/FindBar.js: 366,367,368
+src/brackets/src/search/FindInFiles.js: 383,788
+src/brackets/src/utils/DragAndDrop.js: 110
+src/brackets/src/utils/DropdownEventHandler.js: 309
+src/brackets/src/utils/StringMatch.js: 351
+src/brackets/src/view/MainViewManager.js: 468
+src/brackets/src/view/Pane.js: 948
+src/brackets/src/widgets/Dialogs.js: 193
+src/Chart.js/src/core/core.controller.js: 918
+src/Chart.js/src/core/core.helpers.js: 584
+src/Chart.js/src/core/core.ticks.js: 39
+src/Chart.js/src/scales/scale.linearbase.js: 41,131
+src/create-react-app/packages/react-dev-utils/webpackHotDevClient.js: 90
+src/create-react-app/packages/react-scripts/scripts/eject.js: 154
+src/express/lib/response.js: 200
+src/freeCodeCamp/public/js/lib/loop-protect/loop-protect.js: 241
+src/freeCodeCamp/server/boot/authentication.js: 129
+src/Ghost/core/server/api/users.js: 138
+src/Ghost/core/server/api/utils.js: 349
+src/Ghost/core/server/data/validation/index.js: 198,213,229,250
+src/Ghost/core/server/models/base/index.js: 107,208,214,220,226,248,255,261,802,816
+src/Ghost/core/server/models/plugins/collision.js: 57
+src/Ghost/core/server/models/subscriber.js: 65
+src/Ghost/core/server/models/user.js: 335
+src/Ghost/core/server/services/permissions/public.js: 12
+src/Ghost/core/server/services/url/utils.js: 334
+src/jquery/external/qunit/qunit.js: 107,397
+src/jquery/external/requirejs/require.js: 95,711,1286
+src/jquery/external/sinon/sinon.js: 269,275,631,914,4649
+src/jquery/src/effects.js: 180
+src/jquery/src/event.js: 195,343
+src/jquery/src/event/trigger.js: 126,132
+src/react-native/IntegrationTests/SizeFlexibilityUpdateTest.js: 50,56,62,68
+src/react-native/Libraries/Components/Navigation/NavigatorIOS.ios.js: 705
+src/react-native/Libraries/Components/ScrollView/ScrollView.js: 673,684
+src/react-native/Libraries/Core/InitializeCore.js: 187
+src/react-native/Libraries/Experimental/WindowedListView.js: 419
+src/react-native/Libraries/Lists/VirtualizedList.js: 1414,1520
+src/react-native/Libraries/Network/XHRInterceptor.js: 125
+src/react-native/Libraries/Performance/Systrace.js: 78,94,112
+src/react-native/Libraries/polyfills/Object.es6.js: 36
+src/react-native/Libraries/ReactNative/AppContainer.js: 59,94
+src/react-native/Libraries/Renderer/ReactFabric-dev.js: 589,2856,4804,4813,6156,6350,6462,6470,6516,6524,7253,7318,7436,7497,9805,12437,12472
+src/react-native/Libraries/Renderer/ReactNativeRenderer-dev.js: 634,3306,5140,5149,6492,6686,6798,6806,6852,6860,7589,7654,7772,7833,10175,12807,12842
+src/react-native/Libraries/Renderer/ReactNativeRenderer-prod.js: 1042
+src/react-native/Libraries/Renderer/shims/ReactNativeViewConfigRegistry.js: 37
+src/react-native/Libraries/vendor/core/Map.js: 454
+src/react-native/scripts/run-ci-e2e-tests.js: 50,59
+src/react-router/website/modules/components/DelegateMarkdownLinks.js: 11
+src/react-router/website/modules/LoadServiceWorker.js: 3
+src/react/packages/events/EventPluginUtils.js: 143
+src/react/packages/react-call-return/src/ReactCallReturn.js: 41,64
+src/react/packages/react-dom/src/client/ReactDOM.js: 1007,1025,1309,1315
+src/react/packages/react-dom/src/client/ReactDOMFiberComponent.js: 254,387,388,653,936,1057
+src/react/packages/react-dom/src/client/ReactDOMFiberOption.js: 41
+src/react/packages/react-dom/src/client/ReactDOMFiberSelect.js: 165
+src/react/packages/react-dom/src/events/BeforeInputEventPlugin.js: 226
+src/react/packages/react-dom/src/events/SimpleEventPlugin.js: 277
+src/react/packages/react-dom/src/server/ReactPartialRenderer.js: 180,271,305,433,455,475,515,587,953
+src/react/packages/react-dom/src/shared/CSSPropertyOperations.js: 62
+src/react/packages/react-native-renderer/src/__mocks__/ReactNativeViewConfigRegistry.js: 36
+src/react/packages/react-native-renderer/src/NativeMethodsMixinUtils.js: 30
+src/react/packages/react-reconciler/src/ReactChildFiber.js: 113,506,594,673,792,857,971,1032,1282
+src/react/packages/react-reconciler/src/ReactDebugFiberPerf.js: 244,256
+src/react/packages/react-reconciler/src/ReactFiberClassComponent.js: 663,775,973,1085,1093,1139,1147
+src/react/packages/react-reconciler/src/ReactFiberCommitWork.js: 171,392
+src/react/packages/react-reconciler/src/ReactFiberHydrationContext.js: 283
+src/react/packages/react-reconciler/src/ReactFiberReconciler.js: 325,379
+src/react/packages/react-reconciler/src/ReactFiberScheduler.js: 1168,1190,1207,1248
+src/react/packages/react-reconciler/src/ReactFiberStack.js: 59,89
+src/react/packages/react-reconciler/src/ReactFiberUpdateQueue.js: 157
+src/react/packages/react/src/ReactChildren.js: 165
+src/react/packages/react/src/ReactElement.js: 27,39,214,231,232
+src/react/packages/react/src/ReactElementValidator.js: 196
+src/react/packages/shared/invokeGuardedCallback.js: 56
+src/react/packages/shared/ReactDOMFrameScheduling.js: 25
+src/react/scripts/rollup/shims/react-native/ReactNativeViewConfigRegistry.js: 37
+src/redux/src/combineReducers.js: 125
+src/reveal.js/js/reveal.js: 4174
+src/reveal.js/plugin/markdown/markdown.js: 333
+src/reveal.js/plugin/search/search.js: 54
+src/socket.io/lib/index.js: 364,394
+src/spectrum/api/utils/create-graphql-error-formatter.js: 18
+src/spectrum/shared/graphql/queries/thread/getThreadMessageConnection.js: 58
+src/spectrum/src/components/chatInput/index.js: 263
+src/spectrum/src/components/stripeCardForm/modalWell.js: 33
+src/three.js/editor/js/Sidebar.Object.js: 486
+src/three.js/src/animation/KeyframeTrack.js: 406
+src/three.js/src/core/BufferGeometry.js: 333
+src/three.js/src/extras/core/ShapePath.js: 255
+src/three.js/src/loaders/ImageLoader.js: 78
+src/three.js/src/loaders/JSONLoader.js: 54
+src/three.js/src/loaders/LoadingManager.js: 25
+src/three.js/src/loaders/ObjectLoader.js: 148,644
+src/three.js/src/objects/Mesh.js: 251
+src/three.js/src/renderers/webgl/WebGLCapabilities.js: 46
+src/three.js/src/renderers/webgl/WebGLShadowMap.js: 381
+src/three.js/src/renderers/webgl/WebGLTextures.js: 497
+src/three.js/src/renderers/WebGLRenderer.js: 842,1613
+src/vue/packages/vue-server-renderer/basic.js: 1759
+src/vue/packages/vue-server-renderer/build.js: 1765,4101,5562
+src/vue/packages/vue-template-compiler/build.js: 3075,4189
+src/vue/packages/weex-template-compiler/build.js: 3614
+src/vue/packages/weex-vue-framework/factory.js: 1637,3384,4559,5996,6034
+src/vue/src/compiler/to-function.js: 88
+src/vue/src/core/instance/state.js: 133
+src/vue/src/core/util/props.js: 141
+src/vue/src/core/vdom/create-element.js: 74
+src/vue/src/core/vdom/patch.js: 557,593
+src/vue/src/platforms/web/compiler/directives/model.js: 28
+src/vue/src/platforms/web/runtime/transition-util.js: 103

src/index.ts

@@ -23,6 +23,7 @@ const sonarjsRules: [string, Linter.RuleLevel][] = [
   ["cognitive-complexity", "error"],
   ["max-switch-cases", "error"],
   ["no-all-duplicated-branches", "error"],
+  ["no-collapsible-if", "error"],
   ["no-duplicate-string", "error"],
   ["no-duplicated-branches", "error"],
   ["no-element-overwrite", "error"],

src/rules/no-collapsible-if.ts

@@ -0,0 +1,66 @@
+/*
+ * 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-1066
+
+import { Rule } from "eslint";
+import * as estree from "estree";
+import { isIfStatement, isBlockStatement } from "../utils/nodes";
+import { report, issueLocation } from "../utils/locations";
+
+const rule: Rule.RuleModule = {
+  meta: {
+    schema: [
+      {
+        // internal parameter
+        enum: ["sonar-runtime"],
+      },
+    ],
+  },
+  create(context: Rule.RuleContext) {
+    return {
+      IfStatement(node: estree.Node) {
+        let { consequent } = node as estree.IfStatement;
+        if (isBlockStatement(consequent) && consequent.body.length === 1) {
+          consequent = consequent.body[0];
+        }
+        if (isIfStatementWithoutElse(node) && isIfStatementWithoutElse(consequent)) {
+          const ifKeyword = context.getSourceCode().getFirstToken(consequent);
+          const enclosingIfKeyword = context.getSourceCode().getFirstToken(node);
+          if (ifKeyword && enclosingIfKeyword) {
+            report(
+              context,
+              {
+                message: `Merge this if statement with the enclosing one.`,
+                loc: ifKeyword.loc,
+              },
+              [issueLocation(enclosingIfKeyword.loc, enclosingIfKeyword.loc, `Enclosing "if" statement`)],
+            );
+          }
+        }
+      },
+    };
+
+    function isIfStatementWithoutElse(node: estree.Node): node is estree.IfStatement {
+      return isIfStatement(node) && !node.alternate;
+    }
+  },
+};
+
+export = rule;

tests/rules/no-collapsible-if.test.ts

@@ -0,0 +1,108 @@
+/*
+ * 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: 2015 } });
+import rule = require("../../src/rules/no-collapsible-if");
+
+ruleTester.run("no-collapsible-if", rule, {
+  valid: [
+    {
+      code: `
+      if (x) {
+        console.log(x);
+      }`,
+    },
+    {
+      code: `
+      if (x) {
+        if (y) {}
+          console.log(x);
+      }`,
+    },
+    {
+      code: `
+      if (x) {
+        console.log(x);
+        if (y) {}
+      }`,
+    },
+    {
+      code: `
+      if (x) {
+        if (y) {}
+      } else {}`,
+    },
+    {
+      code: `
+      if (x) {
+        if (y) {} else {}
+      }`,
+    },
+  ],
+
+  invalid: [
+    {
+      code: `
+      if (x) {
+    //^^ > {{Enclosing "if" statement}}
+        if (y) {}
+      //^^ {{Merge this if statement with the enclosing one.}}
+      }`,
+      options: ["sonar-runtime"],
+      errors: [
+        {
+          message: JSON.stringify({
+            secondaryLocations: [
+              {
+                line: 2,
+                column: 6,
+                endLine: 2,
+                endColumn: 8,
+                message: `Enclosing "if" statement`,
+              },
+            ],
+            message: "Merge this if statement with the enclosing one.",
+          }),
+          line: 4,
+          column: 9,
+          endLine: 4,
+          endColumn: 11,
+        },
+      ],
+    },
+    {
+      code: `
+      if (x)
+        if(y) {}`,
+      errors: [{ message: "Merge this if statement with the enclosing one." }],
+    },
+    {
+      code: `
+      if (x) {
+        if(y) {
+          if(z) {
+          }
+        }
+      }`,
+      errors: 2,
+    },
+  ],
+});