Merge pull request #1920 from mroderick/upgrade-linting

Upgrade linting

.eslintrc.yml

@@ -1,4 +1,6 @@
-extends: sinon
+extends:
+  - sinon
+  - plugin:prettier/recommended
 
 globals:
   ArrayBuffer: false
@@ -10,8 +12,10 @@ globals:
 plugins:
   - ie11
   - local-rules
+  - prettier
 
 rules:
+  prettier/prettier: error
   ie11/no-collection-args: error
   ie11/no-for-in-const: error
   ie11/no-loop-func: warn

.prettierrc

@@ -0,0 +1,4 @@
+{
+  "printWidth": 120,
+  "tabWidth": 4
+}

CONTRIBUTING.md

@@ -81,17 +81,28 @@ Sinon.JS aims at supporting the following runtimes:
 * Node LTS versions
 
 
-### Style
+### Linting and style
 
-Sinon.JS uses [ESLint](http://eslint.org) to keep consistent style. You probably want to install a plugin for your editor.
+Sinon.JS uses [ESLint](http://eslint.org) to keep the codebase free of lint, and uses [Prettier](https://prettier.io) to keep consistent style.
 
-The ESLint test will be run before unit tests in the CI environment, your build will fail if it doesn't pass the style check.
+If you are contributing to a Sinon project, you'll probably want to configure your editors ([ESLint](https://eslint.org/docs/user-guide/integrations#editors), [Prettier][https://prettier.io/docs/en/editors.html]) to make editing code a more enjoyable experience.
+
+The ESLint verification (which includes Prettier) will be run before unit tests in the CI environment. The build will fail if the source code does not pass the style check.
+
+
+You can run the linter locally:
 
 ```
 $ npm run lint
 ```
 
-To ensure consistent reporting of lint warnings, you should use the same version as CI environment (defined in `package.json`)
+You can fix a lot lint and style violations automatically:
+
+```
+$ npm run lint -- --fix
+```
+
+To ensure consistent reporting of lint warnings, you should use the same versions of ESLint and Prettier as defined in `package.json` (which is what the CI servers use).
 
 ### Run the tests
 

build.js

@@ -10,8 +10,7 @@ var pkg = require("./package.json");
 var date = new Date().toISOString().split("T")[0];
 
 // Keep the preamble on one line to retain source maps
-var preamble = "/* Sinon.JS " + pkg.version + ", " + date
-    + ", @license BSD-3 */";
+var preamble = "/* Sinon.JS " + pkg.version + ", " + date + ", @license BSD-3 */";
 
 try {
     fs.mkdirSync("pkg");
@@ -23,15 +22,14 @@ function makeBundle(name, config) {
     // Create a UMD wrapper and install the "sinon" global:
     config.standalone = "sinon";
 
-    browserify("./lib/sinon.js", config)
-        .bundle(function (err, buffer) {
-            if (err) {
-                throw err;
-            }
+    browserify("./lib/sinon.js", config).bundle(function(err, buffer) {
+        if (err) {
+            throw err;
+        }
 
-            var script = preamble + buffer.toString();
-            fs.writeFileSync("pkg/" + name + ".js", script);
-        });
+        var script = preamble + buffer.toString();
+        fs.writeFileSync("pkg/" + name + ".js", script);
+    });
 }
 
 makeBundle("sinon", {

eslint-local-rules.js

@@ -2,7 +2,7 @@
 
 function getPrototypeMethods(prototype) {
     /* eslint-disable local-rules/no-prototype-methods */
-    return Object.getOwnPropertyNames(prototype).filter(function (name) {
+    return Object.getOwnPropertyNames(prototype).filter(function(name) {
         return typeof prototype[name] === "function" && prototype.hasOwnProperty(name);
     });
 }
@@ -25,19 +25,19 @@ module.exports = {
             schema: []
         },
 
-        create: function (context) {
+        create: function(context) {
             /**
-            * Reports if a disallowed property is used in a CallExpression
-            * @param {ASTNode} node The CallExpression node.
-            * @returns {void}
-            */
+             * Reports if a disallowed property is used in a CallExpression
+             * @param {ASTNode} node The CallExpression node.
+             * @returns {void}
+             */
             function disallowBuiltIns(node) {
                 if (
-                    node.callee.type !== "MemberExpression"
-                    || node.callee.computed
+                    node.callee.type !== "MemberExpression" ||
+                    node.callee.computed ||
                     // allow static method calls
-                    || node.callee.object.name === "Array"
-                    || node.callee.object.name === "Object"
+                    node.callee.object.name === "Array" ||
+                    node.callee.object.name === "Object"
                 ) {
                     return;
                 }
@@ -53,8 +53,7 @@ module.exports = {
                         },
                         node: node
                     });
-                }
-                else if (DISALLOWED_ARRAY_PROPS.indexOf(propName) > -1) {
+                } else if (DISALLOWED_ARRAY_PROPS.indexOf(propName) > -1) {
                     context.report({
                         message: "Do not access {{obj}} prototype method '{{prop}}' from target object.",
                         loc: node.callee.property.loc.start,

lib/sinon.js

@@ -35,7 +35,7 @@ var apiMethods = {
     createFakeServer: nise.fakeServer.create.bind(nise.fakeServer),
     createFakeServerWithClock: nise.fakeServerWithClock.create.bind(nise.fakeServerWithClock),
 
-    addBehavior: function (name, fn) {
+    addBehavior: function(name, fn) {
         behavior.addBehavior(stub, name, fn);
     }
 };

lib/sinon/assert.js

@@ -19,7 +19,7 @@ var assert;
 function verifyIsStub() {
     var args = arraySlice(arguments);
 
-    forEach(args, function (method) {
+    forEach(args, function(method) {
         if (!method) {
             assert.fail("fake is not a spy");
         }
@@ -46,8 +46,12 @@ function verifyIsValidAssertion(assertionMethod, assertionArgs) {
         case "calledTwice":
         case "calledThrice":
             if (assertionArgs.length !== 0) {
-                assert.fail(assertionMethod +
-                            " takes 1 argument but was called with " + (assertionArgs.length + 1) + " arguments");
+                assert.fail(
+                    assertionMethod +
+                        " takes 1 argument but was called with " +
+                        (assertionArgs.length + 1) +
+                        " arguments"
+                );
             }
             break;
         default:
@@ -56,45 +60,43 @@ function verifyIsValidAssertion(assertionMethod, assertionArgs) {
 }
 
 function failAssertion(object, msg) {
-    object = object || global;
-    var failMethod = object.fail || assert.fail;
-    failMethod.call(object, msg);
+    var obj = object || global;
+    var failMethod = obj.fail || assert.fail;
+    failMethod.call(obj, msg);
 }
 
 function mirrorPropAsAssertion(name, method, message) {
+    var msg = message;
+    var meth = method;
     if (arguments.length === 2) {
-        message = method;
-        method = name;
+        msg = method;
+        meth = name;
     }
 
-    assert[name] = function (fake) {
+    assert[name] = function(fake) {
         verifyIsStub(fake);
 
         var args = arraySlice(arguments, 1);
         var failed = false;
 
         verifyIsValidAssertion(name, args);
 
-        if (typeof method === "function") {
-            failed = !method(fake);
+        if (typeof meth === "function") {
+            failed = !meth(fake);
         } else {
-            failed = typeof fake[method] === "function" ?
-                !fake[method].apply(fake, args) : !fake[method];
+            failed = typeof fake[meth] === "function" ? !fake[meth].apply(fake, args) : !fake[meth];
         }
 
         if (failed) {
-            failAssertion(this, (fake.printf || fake.proxy.printf).apply(fake, concat([message], args)));
+            failAssertion(this, (fake.printf || fake.proxy.printf).apply(fake, concat([msg], args)));
         } else {
             assert.pass(name);
         }
     };
 }
 
 function exposedName(prefix, prop) {
-    return !prefix || /^fail/.test(prop)
-        ? prop
-        : prefix + stringSlice(prop, 0, 1).toUpperCase() + stringSlice(prop, 1)
-    ;
+    return !prefix || /^fail/.test(prop) ? prop : prefix + stringSlice(prop, 0, 1).toUpperCase() + stringSlice(prop, 1);
 }
 
 assert = {
@@ -107,7 +109,9 @@ assert = {
         throw error;
     },
 
-    pass: function pass() {},
+    pass: function pass() {
+        return;
+    },
 
     callOrder: function assertCallOrder() {
         verifyIsStub.apply(null, arguments);
@@ -129,8 +133,7 @@ assert = {
                 // If this fails, we'll just fall back to the blank string
             }
 
-            failAssertion(this, "expected " + expected + " to be " +
-                        "called in order but were called as " + actual);
+            failAssertion(this, "expected " + expected + " to be called in order but were called as " + actual);
         } else {
             assert.pass("callOrder");
         }
@@ -140,8 +143,7 @@ assert = {
         verifyIsStub(method);
 
         if (method.callCount !== count) {
-            var msg = "expected %n to be called " + timesInWords(count) +
-                " but was called %c%C";
+            var msg = "expected %n to be called " + timesInWords(count) + " but was called %c%C";
             failAssertion(this, method.printf(msg));
         } else {
             assert.pass("callCount");
@@ -154,11 +156,11 @@ assert = {
         }
 
         var o = options || {};
-        var prefix = typeof o.prefix === "undefined" && "assert" || o.prefix;
-        var includeFail = typeof o.includeFail === "undefined" || !!o.includeFail;
+        var prefix = (typeof o.prefix === "undefined" && "assert") || o.prefix;
+        var includeFail = typeof o.includeFail === "undefined" || Boolean(o.includeFail);
         var instance = this;
 
-        forEach(Object.keys(instance), function (method) {
+        forEach(Object.keys(instance), function(method) {
             if (method !== "expose" && (includeFail || !/^(fail)/.test(method))) {
                 target[exposedName(prefix, method)] = instance[method];
             }
@@ -184,17 +186,18 @@ assert = {
 };
 
 mirrorPropAsAssertion("called", "expected %n to have been called at least once but was never called");
-mirrorPropAsAssertion("notCalled", function (spy) {
-    return !spy.called;
-}, "expected %n to not have been called but was called %c%C");
+mirrorPropAsAssertion(
+    "notCalled",
+    function(spy) {
+        return !spy.called;
+    },
+    "expected %n to not have been called but was called %c%C"
+);
 mirrorPropAsAssertion("calledOnce", "expected %n to be called once but was called %c%C");
 mirrorPropAsAssertion("calledTwice", "expected %n to be called twice but was called %c%C");
 mirrorPropAsAssertion("calledThrice", "expected %n to be called thrice but was called %c%C");
 mirrorPropAsAssertion("calledOn", "expected %n to be called with %1 as this but was called with %t");
-mirrorPropAsAssertion(
-    "alwaysCalledOn",
-    "expected %n to always be called with %1 as this but was called with %t"
-);
+mirrorPropAsAssertion("alwaysCalledOn", "expected %n to always be called with %1 as this but was called with %t");
 mirrorPropAsAssertion("calledWithNew", "expected %n to be called with new");
 mirrorPropAsAssertion("alwaysCalledWithNew", "expected %n to always be called with new");
 mirrorPropAsAssertion("calledWith", "expected %n to be called with arguments %D");