[[TEST]] Introduce jscs for style guide enforcement

Adds JSCS with a set of rules that required minimal changes to the codebase. Further work to enforce more rules can be done later. Closes jshintgh-1997
KevinSoulat · Jan 13, 2015 · acb88f2 · acb88f2
1 parent 3c81989
commit acb88f2
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 29 deletions.
diff --git a/.jscsrc b/.jscsrc
@@ -0,0 +1,45 @@
+{
+    "requireOperatorBeforeLineBreak": true,
+    "maximumLineLength": {
+      "value": 100,
+      "allowComments": true,
+      "allowRegex": true
+    },
+    // disabled due to https://github.com/jscs-dev/node-jscs/issues/915
+    // "validateIndentation": 2,
+
+    "disallowMultipleLineStrings": true,
+    "disallowMixedSpacesAndTabs": true,
+    "disallowTrailingWhitespace": true,
+    "disallowSpaceAfterPrefixUnaryOperators": true,
+    "disallowSpaceBeforePostfixUnaryOperators": true,
+
+    "requireSpaceAfterKeywords": [
+      "if",
+      "else",
+      "for",
+      "while",
+      "do",
+      "switch",
+      "return",
+      "try",
+      "catch"
+    ],
+    "requireSpaceBeforeBinaryOperators": true,
+    "requireSpaceAfterBinaryOperators": true,
+    "requireSpacesInConditionalExpression": true,
+    "requireSpaceBeforeBlockStatements": true,
+    "requireLineFeedAtFileEnd": true,
+    "requireSpacesInFunctionExpression": {
+        "beforeOpeningCurlyBrace": true
+    },
+    "requireSpacesInAnonymousFunctionExpression": {
+        "beforeOpeningRoundBrace": true
+    },
+    "requireSpacesInsideObjectBrackets": "all",
+    // disabled until consensus can be formed
+    // "disallowSpacesInsideArrayBrackets": "all",
+    "disallowSpacesInsideParentheses": true,
+
+    "disallowNewlineBeforeBlockStatements": true
+}
diff --git a/.jshintrc b/.jshintrc
@@ -2,7 +2,6 @@
   "boss":      true,
   "node":      true,
   "strict":    true,
-  "maxlen":    100,
   "newcap":    false,
   "undef":     true,
   "unused":    true,

diff --git a/package.json b/package.json
@@ -27,7 +27,7 @@
     "build": "node bin/build",
     "coverage": "istanbul -- cover ./node_modules/.bin/nodeunit tests/unit",
     "data": "node scripts/generate-identifier-data",
-    "pretest": "jshint src",
+    "pretest": "jshint src && jscs src",
     "test": "nodeunit tests tests/regression tests/unit"
   },
 
@@ -48,6 +48,7 @@
     "browserify":    "6.x",
     "coveralls":     "2.11.x",
     "istanbul":      "0.3.x",
+    "jscs":          "1.9.x",
     "jshint":        "2.5.x",
     "mock-stdin":    "0.3.x",
     "nodeunit":      "0.9.x",

diff --git a/src/cli.js b/src/cli.js
@@ -560,7 +560,7 @@ var exports = {
       (!opts.extensions ? "" : "|" +
         opts.extensions.replace(/,/g, "|").replace(/[\. ]/g, "")) + ")$");
 
-    var ignores = !opts.ignores ? loadIgnores({cwd: opts.cwd}) :
+    var ignores = !opts.ignores ? loadIgnores({ cwd: opts.cwd }) :
                                   opts.ignores.map(function (target) {
                                     return path.resolve(target);
                                   });
@@ -735,12 +735,12 @@ var exports = {
       args:       cli.args,
       config:     config,
       reporter:   reporter,
-      ignores:    loadIgnores({exclude: options.exclude, excludePath: options["exclude-path"]}),
+      ignores:    loadIgnores({ exclude: options.exclude, excludePath: options["exclude-path"] }),
       extensions: options["extra-ext"],
       verbose:    options.verbose,
       extract:    options.extract,
       filename:   options.filename,
-      useStdin:   {"-": true, "/dev/stdin": true}[args[args.length - 1]]
+      useStdin:   { "-": true, "/dev/stdin": true }[args[args.length - 1]]
     }, done));
   }
 };

diff --git a/src/jshint.js b/src/jshint.js
@@ -180,7 +180,7 @@ var JSHINT = (function () {
       for (var enforceopt in options.bool.enforcing) {
         if (state.option[enforceopt] === undefined) {
           state.option[enforceopt] = true;
-          }
+        }
       }
       for (var relaxopt in options.bool.relaxing) {
         if (state.option[relaxopt] === undefined) {
@@ -2079,7 +2079,7 @@ var JSHINT = (function () {
   infix(",", function (left, that) {
     var expr;
     that.exprs = [left];
-    if (!comma({peek: true})) {
+    if (!comma({ peek: true })) {
       return that;
     }
     while (true) {
@@ -2467,8 +2467,7 @@ var JSHINT = (function () {
       i += 1;
       pn1 = pn;
       pn = peek(i);
-    } while (!(parens === 0 && pn1.value === ")") &&
-             pn.value !== ";" && pn.type !== "(end)");
+    } while (!(parens === 0 && pn1.value === ")") && pn.value !== ";" && pn.type !== "(end)");
 
     if (state.tokens.next.id === "function") {
       triggerFnExpr = state.tokens.next.immed = true;
@@ -2639,7 +2638,7 @@ var JSHINT = (function () {
             warning("W128");
             do {
               advance(",");
-            } while(state.tokens.next.id === ",");
+            } while (state.tokens.next.id === ",");
             continue;
           }
         }
@@ -3944,8 +3943,7 @@ var JSHINT = (function () {
     do {
       nextop = peek(i);
       ++i;
-    } while (!_.contains(inof, nextop.value) && nextop.value !== ";" &&
-          nextop.type !== "(end)");
+    } while (!_.contains(inof, nextop.value) && nextop.value !== ";" && nextop.type !== "(end)");
 
     // if we're in a for (… in|of …) statement
     if (_.contains(inof, nextop.value)) {
@@ -4556,7 +4554,7 @@ var JSHINT = (function () {
       // otherwise we warn about it
       return (l === 0);
     }
-    return {stack: function () {
+    return { stack: function () {
           _current = new CompArray();
           _carrays.push(_current);
         },
@@ -4789,7 +4787,7 @@ var JSHINT = (function () {
     };
   };
 
-  var escapeRegex = function(str) {
+  var escapeRegex = function (str) {
     return str.replace(/[-\/\\^$*+?.()|[\]{}]/g, "\\$&");
   };
 
@@ -4950,7 +4948,7 @@ var JSHINT = (function () {
 
         reIgnore = new RegExp(reIgnoreStr, "ig");
 
-        s = s.replace(reIgnore, function(match) {
+        s = s.replace(reIgnore, function (match) {
           return match.replace(/./g, " ");
         });
       });

diff --git a/src/lex.js b/src/lex.js
@@ -538,7 +538,7 @@ Lexer.prototype = {
       return (/^[0-9a-fA-F]$/).test(str);
     }
 
-    var readUnicodeEscapeSequence = function() {
+    var readUnicodeEscapeSequence = function () {
       /*jshint validthis:true */
       index += 1;
 
@@ -619,7 +619,7 @@ Lexer.prototype = {
     }.bind(this);
 
     function removeEscapeSequences(id) {
-      return id.replace(/\\u([0-9a-fA-F]{4})/g, function(m0, codepoint) {
+      return id.replace(/\\u([0-9a-fA-F]{4})/g, function (m0, codepoint) {
         return String.fromCharCode(parseInt(codepoint, 16));
       });
     }
@@ -964,7 +964,7 @@ Lexer.prototype = {
       break;
     }
 
-    return {char: char, jump: jump, allowNewLine: allowNewLine};
+    return { char: char, jump: jump, allowNewLine: allowNewLine };
   },
 
   /*

diff --git a/src/name-stack.js b/src/name-stack.js
@@ -5,7 +5,7 @@ function NameStack() {
 }
 
 Object.defineProperty(NameStack.prototype, "length", {
-  get: function() {
+  get: function () {
     return this._stack.length;
   }
 });
@@ -14,14 +14,14 @@ Object.defineProperty(NameStack.prototype, "length", {
  * Create a new entry in the stack. Useful for tracking names across
  * expressions.
  */
-NameStack.prototype.push = function() {
+NameStack.prototype.push = function () {
   this._stack.push(null);
 };
 
 /**
  * Discard the most recently-created name on the stack.
  */
-NameStack.prototype.pop = function() {
+NameStack.prototype.pop = function () {
   this._stack.pop();
 };
 
@@ -31,7 +31,7 @@ NameStack.prototype.pop = function() {
  * @param {object} token The token to consider as the source for the most
  *                       recent name.
  */
-NameStack.prototype.set = function(token) {
+NameStack.prototype.set = function (token) {
   this._stack[this.length - 1] = token;
 };
 
@@ -40,7 +40,7 @@ NameStack.prototype.set = function(token) {
  *
  * @returns {string}
  */
-NameStack.prototype.infer = function() {
+NameStack.prototype.infer = function () {
   var nameToken = this._stack[this.length - 1];
   var prefix = "";
   var type;

diff --git a/src/reporters/checkstyle.js b/src/reporters/checkstyle.js
@@ -2,8 +2,7 @@
 // http://github.com/relaxnow
 module.exports =
 {
-  reporter: function (results, data, opts)
-  {
+  reporter: function (results, data, opts) {
     "use strict";
 
     var files = {},
@@ -43,7 +42,7 @@ module.exports =
 
       var typeNo = result.error.code;
       var severity = '';
-      switch(typeNo[0]) {
+      switch (typeNo[0]) {
         case 'I':
           severity = 'info';
           break;

diff --git a/src/reporters/jslint_xml.js b/src/reporters/jslint_xml.js
@@ -1,7 +1,6 @@
 module.exports =
 {
-  reporter: function (results)
-  {
+  reporter: function (results) {
     "use strict";
 
     var files = {},

diff --git a/src/reporters/unix.js b/src/reporters/unix.js
@@ -5,7 +5,7 @@
 // The format is "FILENAME:LINE:COL: MESSAGE".
 
 module.exports = {
-  reporter: function(results, data, opts) {
+  reporter: function (results, data, opts) {
     var len = results.length;
     var str = "";
     var prevfile;