diff --git a/.codeclimate.yml b/.codeclimate.yml index 1acff4fbf..a9416a0a5 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -12,3 +12,4 @@ ratings: - lib/checks.js" exclude_paths: - "node_modules/**" +- "test/**" diff --git a/.eslintrc b/.eslintrc index 0578e2aed..e3e78971b 100644 --- a/.eslintrc +++ b/.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 --git a/Makefile b/Makefile index 3b5a0f776..c571bf19e 100644 --- a/Makefile +++ b/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 --git a/bin/eslint.js b/bin/eslint.js index 31122e6d0..9a2cc4801 100755 --- a/bin/eslint.js +++ b/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 --git a/circle.yml b/circle.yml index 9d97e5cda..c5becf697 100644 --- a/circle.yml +++ b/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 --git a/lib/checks.js b/lib/checks.js index e278d72a7..f7a960686 100644 --- a/lib/checks.js +++ b/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 --git a/package.json b/package.json index 8c91a5fe9..d3adf33a2 100644 --- a/package.json +++ b/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 --git a/test/checks_test.js b/test/checks_test.js new file mode 100644 index 000000000..ed086a240 --- /dev/null +++ b/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 }; + }; + }); +});