Merge pull request #60 from codeclimate/will/complexity

Mercy merged on behalf of will.

Author: codeclimate-shipbot

Diff for: .codeclimate.yml

@@ -12,3 +12,4 @@ ratings:
   - lib/checks.js"
 exclude_paths:
 - "node_modules/**"
+- "test/**"

Diff for: .eslintrc

@@ -6,8 +6,10 @@
     "brace-style": [2, "1tbs", { "allowSingleLine": true }],
     "comma-dangle": [2, "never"],
     "comma-style": [2, "first", { exceptions: {ArrayExpression: true, ObjectExpression: true} }],
+    "complexity": [2, 6],
     "curly": 2,
     "eqeqeq": [2, "allow-null"],
+    "max-statements": [2, 30],
     "no-shadow-restricted-names": 2,
     "no-undef": 2,
     "no-use-before-define": 2,

Diff for: Makefile

@@ -1,6 +1,9 @@
-.PHONY: image
+.PHONY: image test
 
 IMAGE_NAME ?= codeclimate/codeclimate-eslint
 
 image:
 	docker build --rm -t $(IMAGE_NAME) .
+
+test: image
+	docker run $(IMAGE_NAME) npm run test

Diff for: bin/eslint.js

@@ -60,7 +60,7 @@ function buildIssueJson(message, path) {
         }
       }
     },
-    remediation_points: checks.remediationPoints(checkName, message)
+    remediation_points: checks.remediationPoints(checkName, message, cli.getConfigForFile(path))
   };
   return JSON.stringify(issue);
 }

Diff for: circle.yml

@@ -11,7 +11,7 @@ machine:
 
 test:
   override:
-    - docker build -t=$PRIVATE_REGISTRY/$CIRCLE_PROJECT_REPONAME:b$CIRCLE_BUILD_NUM .
+    - IMAGE_NAME="$PRIVATE_REGISTRY/$CIRCLE_PROJECT_REPONAME:b$CIRCLE_BUILD_NUM" make test
 
 deployment:
   registry:

Diff for: lib/checks.js

@@ -14,6 +14,7 @@ var checkCategoryMapping = {
   "guard-for-in": "Bug Risk",
   "handle-callback-err": "Bug Risk",
   "init-declarations": "Clarity",
+  "max-statements": "Complexity",
   "no-alert": "Bug Risk",
   "no-caller": "Compatibility",
   "no-case-declarations": "Bug Risk",
@@ -113,21 +114,48 @@ var categories = function(checkName) {
   return [checkCategoryMapping[checkName] || "Style"];
 };
 
-var remediationPoints = function(checkName, message) {
-  if (checkName === "complexity") {
+// Here Be Dragons: this function extracts the relevant value that triggered the issue for
+// checks in the Complexity category. Unfortunately, these values are not available in a
+// structured way, so we extract them from strings. That means that any check categorized
+// as Complexity MUST have a rule here to extract value.
+//
+// If a matching string segment cannot be found, `null` will be returned.
+var messageMetricValue = function(message) {
+  var match = null;
+  switch (message.ruleId) {
+    case "complexity":
+      match = message.message.match(/complexity of (\d+)/);
+      break;
+    case "max-statements":
+      match = message.message.match(/too many statements \((\d+)\)/);
+      break;
+  }
+  if (null !== match) {
+    return parseInt(match[1], 10);
+  }
+  return null;
+};
+
+var metricThreshold = function(ruleId, eslintConfig) {
+   return eslintConfig.rules[ruleId][1];
+};
+
+var remediationPoints = function(checkName, message, eslintConfig) {
+  if (categories(checkName)[0] === "Complexity") {
     // (@base_cost + (overage * @cost_per))*1_000_000
     // cost_per: 0.1,  base: 1
-    var costPer = 100000
+    var costPer = 70000
       , points = 1000000
-      , threshold = 10 // TODO: Get this from the eslintrc
+      , threshold = metricThreshold(message.ruleId, eslintConfig)
       , overage
-      , complexity;
-
-    complexity = message.message.match(/complexity of (\d+)/)[1];
-    overage = complexity - threshold;
+      , metricValue;
 
-    if (overage > 0) {
-      points += (costPer * overage);
+    metricValue = messageMetricValue(message);
+    if (null !== metricValue) {
+      overage = metricValue - threshold;
+      if (overage > 0) {
+        points += (costPer * overage);
+      }
     }
 
     return points;

Diff for: package.json

@@ -15,5 +15,12 @@
     "eslint-plugin-react": "3.6.3",
     "glob": "5.0.14"
   },
+  "devDependencies": {
+    "chai": "^2.1.0",
+    "mocha": "^2.3.4"
+  },
+  "scripts": {
+    "test": "mocha test"
+  },
   "engine": "node >= 0.12.4"
 }

Diff for: test/checks_test.js

@@ -0,0 +1,49 @@
+var checks = require("../lib/checks");
+var expect = require("chai").expect;
+
+describe("checks module", function() {
+  describe(".categories()", function() {
+    it("returns complexity for max-statements", function() {
+      expect(checks.categories("max-statements")).to.deep.eq(["Complexity"]);
+    });
+
+    it("returns style for an unknown check type", function() {
+      expect(checks.categories("floofle")).to.deep.eq(["Style"]);
+    });
+  });
+
+  describe(".remediationPoints()", function() {
+    it("returns the default of 50,000 for a non-complexity issue", function() {
+      var issue = { ruleId: "eqeqeq", message: "always use ==="};
+      expect(checks.remediationPoints(issue.ruleId, issue, null)).to.eq(50000);
+    });
+
+    it("calculates the cost for a cyclomatic complexity issue", function() {
+      var issue = { ruleId: "complexity", message: "Function 'complex' has a complexity of 8." }
+        , config = eslintConfig({ "complexity": [2, 6] });
+      expect(checks.remediationPoints(issue.ruleId, issue, config)).to.eq(1140000);
+    });
+
+    it("calculates the cost for a max-statements issue", function() {
+      var issue = { ruleId: "max-statements", message: "This function has too many statements (38). Maximum allowed is 30." }
+        , config = eslintConfig({ "max-statements": [2, 30] });
+      expect(checks.remediationPoints(issue.ruleId, issue, config)).to.eq(1560000);
+    });
+
+    it("uses base complexity cost if metric cannot be found", function() {
+      var issue = { ruleId: "complexity", message: "has a complexity of \"8\"" }
+        , config = eslintConfig({ "complexity": [2, 6] });
+      expect(checks.remediationPoints(issue.ruleId, issue, config)).to.eq(1000000);
+    });
+
+    it("uses base complexity cost if overage is negative somehow", function() {
+      var issue = { ruleId: "complexity", message: "has a complexity of 8" }
+        , config = eslintConfig({ "complexity": [2, 10] });
+      expect(checks.remediationPoints(issue.ruleId, issue, config)).to.eq(1000000);
+    });
+
+    var eslintConfig = function(rulesConfig) {
+      return { rules: rulesConfig };
+    };
+  });
+});