Merge pull request #27 from hypothesis/eslint

Add ESLint configuration and use it to replace JSCS / JSHint

.eslintignore

+build/**
+**/vendor/**/*.js

.eslintrc

+{
+  "env": {
+    "mocha": true,
+    "commonjs": true,
+    "browser": true,
+  },
+  "extends": "eslint:recommended",
+  "globals": {
+    "assert": false,
+    "sinon": false,
+    "Promise": false,
+  },
+  "rules": {
+    "array-callback-return": "error",
+    "block-scoped-var": "error",
+    "curly": "error",
+    "eqeqeq": "error",
+    "no-console": [
+      "error",
+      { allow: ["warn", "error"] },
+    ],
+    "no-self-compare": "error",
+    "no-throw-literal": "error",
+    "no-unused-expressions": "error",
+    "no-use-before-define": [
+      "error",
+      {"functions": false},
+    ],
+    "semi": "error",
+    "strict": ["error", "safe"],
+  },
+  "parserOptions": {
+    "ecmaVersion": 6,
+  }
+}

.jscsrc

-{
-  "preset": "google",
-  "requireSemicolons": true,
-  "excludeFiles": [
-    ".tox/**",
-    "build/**",
-    "docs/_build/**",
-    "dist/**",
-    "h/browser/chrome/content/**",
-    "h/static/scripts/vendor/**",
-    "h/static/styles/vendor/**",
-    "node_modules/**"
-  ],
-  "disallowSpacesInAnonymousFunctionExpression": null,
-  "disallowSpacesInFunctionDeclaration": {
-    "beforeOpeningRoundBrace": true
-  },
-  "disallowSpacesInFunctionExpression": null,
-  "disallowSpacesInNamedFunctionExpression": {
-    "beforeOpeningRoundBrace": true
-  },
-  "requireSpacesInFunctionDeclaration": null,
-  "requireSpacesInFunctionExpression": null,
-  "requireSpacesInAnonymousFunctionExpression": {
-    "beforeOpeningRoundBrace": true,
-    "beforeOpeningCurlyBrace": true
-  }
-}

.jshintignore

-build/
-docs/
-node_modules/
-h/browser/chrome/content/
-h/static/scripts/vendor/
-h/templates/
-**/*.min.js

.jshintrc

-{
-  "bitwise": true,
-  "curly": true,
-  "eqeqeq": true,
-  "forin": true,
-  "freeze": true,
-  "latedef": "nofunc",
-  "maxcomplexity": 10,
-  "strict": "global",
-  "undef": true,
-  "unused": true,
-  "esnext": true,
-  "globals": {
-    "chrome": false,
-    "Promise": false,
-    "chai": false,
-    "sinon": false,
-    "JSON": false
-  },
-  "browser": true,
-  "browserify": true,
-  "mocha": true,
-  "phantom": true,
-  "jquery": true,
-  "predef": [
-    "Promise",
-    "URL",
-    "after",
-    "afterEach",
-    "assert",
-    "before",
-    "beforeEach",
-    "chrome",
-    "context",
-    "describe",
-    "it",
-    "require",
-    "sinon"
-  ]
-}

.travis.yml

 language: node_js
-node_js: '4.3'
 cache:
   directories:
     - node_modules
@@ -7,3 +6,12 @@ notifications:
   slack:
     rooms:
       - secure: "iYTftqYShXlMgoeuC7nfmBNwS59ZubIppTD4rpKg6m2PNLal9MtmH5vNlQMfFe+kuLV1pMuPOMfp9y+o3lJGO08lKHKlu+ziAuQcnPW2koR5ViYQcU3NCCZ2Pk4lsolcxwZrS6mbSbL3P/Sv6uYnU+QUNSVPCCeOLxKG+f3Vav+qQx3gduTOLRJEGVtpUpTLmSf8+WZpSM88Y00XK3LVUqOOy2Fq8e13PSERhfWbC391EnuOHpanHtzAdwE5rrWG+EaCYctT//zc/gmy4Cf+coq60RwC0WMhK+zEyLciXRGpBDn0JrGR+24rnI84y9AxdMHXv68OOVx1hZixaKt/8gvpJ78iDRGhEDOrjpGVRrTIApabx02YUsMfJB6XpyCVNRt31LYTW6I4F2+B76UKIEahzdir4LjGbdDhEn+f5adO9bOi46TuCAIhOai220mq/CMv70I4bk8WACuqW1nNq4UaLOZztF0ahmFmHbeGhko5Vlmd3IXhE2tgVN4bk+niOqFWXyJ7Yw/jpe18SkiDoM8jSy6tfQ2ycfwQRHCUHATALAGPYkXFkzKdqrkoRuQBb4hS8MWcOgVdNQwsekul7yR/RFZMBBEj4tUQdBQDP+S1WyCPj9yG5lJ1Ph9jxZ76lBDt3fW2J60UfFtELk4f0f3WR5ipyFSbVqSEmaXad2E="
+matrix:
+  include:
+    # 'node_js' is listed for each job to ensure that only two builds are run.
+    # See https://github.com/hypothesis/client/pull/27#discussion_r70611726
+    - env: ACTION=lint
+      node_js: '4.3'
+      script: npm run lint
+    - env: ACTION=test
+      node_js: '4.3'

Makefile

@@ -15,6 +15,10 @@ clean:
 test: node_modules/.uptodate
 	npm test
 
+.PHONY: lint
+lint: node_modules/.uptodate
+	npm run lint
+
 ################################################################################
 
 build/manifest.json: node_modules/.uptodate

gulpfile.js

+/* eslint-env node */
+
 'use strict';
 
 require('core-js/es6/promise');
@@ -119,7 +121,7 @@ gulp.task('build-app-js', ['build-vendor-js'], function () {
 });
 
 gulp.task('watch-app-js', ['build-vendor-js'], function () {
-  appBundleConfigs.map(function (config) {
+  appBundleConfigs.forEach(function (config) {
     createBundle(config, {watch: true});
   });
 });

h/static/scripts/annotator/range-util.js

@@ -67,9 +67,8 @@ function forEachNodeInRange(range, callback) {
   var nodeIter = root.ownerDocument.createNodeIterator(root,
     NodeFilter.SHOW_ALL, null /* filter */, false /* expandEntityReferences */);
 
-  /* jshint -W084 */
   var currentNode;
-  while (currentNode = nodeIter.nextNode()) {
+  while (currentNode = nodeIter.nextNode()) { // eslint-disable-line no-cond-assign
     if (isNodeInRange(range, currentNode)) {
       callback(currentNode);
     }

h/static/scripts/app.js

 'use strict';
 
-require('./polyfills');
 var queryString = require('query-string');
 
+require('./polyfills');
+
+var raven;
+
 // Initialize Raven. This is required at the top of this file
 // so that it happens early in the app's startup flow
 var settings = require('./settings')(document);
 Object.assign(settings, queryString.parse(window.location.search));
 if (settings.raven) {
-  var raven = require('./raven');
+  raven = require('./raven');
   raven.init(settings.raven);
 }
 

h/static/scripts/build-thread.js

@@ -121,7 +121,7 @@ function threadAnnotations(annotations) {
   // Collect the set of threads which have no parent as
   // children of the thread root
   var roots = [];
-  Object.keys(threads).map(function (id) {
+  Object.keys(threads).forEach(function (id) {
     if (!threads[id].parent) {
       // Top-level threads are collapsed by default
       threads[id].collapsed = true;

h/static/scripts/date-util.js

+'use strict';
+
 // cached date formatting instance.
 // See https://github.com/hypothesis/h/issues/2820#issuecomment-166285361
 var formatter;

h/static/scripts/directive/dropdown-menu-btn.js

@@ -2,7 +2,7 @@
 
 // @ngInject
 function DropdownMenuBtnController($scope, $timeout) {
-  this.onClick = function($event) {
+  this.onClick = function () {
     $scope.onClick();
   };
 
@@ -11,7 +11,7 @@ function DropdownMenuBtnController($scope, $timeout) {
     $timeout(function () {
       $scope.onToggleDropdown();
     }, 0);
-  }
+  };
 }
 
 module.exports = function () {

h/static/scripts/directive/group-list.js

 'use strict';
 
-var events = require('../events');
-
 // @ngInject
 function GroupListController($scope, $window, groups) {
   $scope.leaveGroup = function (groupId) {
@@ -11,11 +9,11 @@ function GroupListController($scope, $window, groups) {
     if ($window.confirm(message)) {
       groups.leave(groupId);
     }
-  }
+  };
 
   $scope.focusGroup = function (groupId) {
     groups.focus(groupId);
-  }
+  };
 }
 
 /**
@@ -28,12 +26,12 @@ function GroupListController($scope, $window, groups) {
 function groupList( $window, groups, settings) {
   return {
     controller: GroupListController,
-    link: function ($scope, elem, attrs) {
+    link: function ($scope) {
       $scope.groups = groups;
 
       $scope.createNewGroup = function() {
         $window.open(settings.serviceUrl + 'groups/new', '_blank');
-      }
+      };
     },
     restrict: 'E',
     scope: {
@@ -41,7 +39,7 @@ function groupList( $window, groups, settings) {
     },
     template: require('../../../templates/client/group_list.html'),
   };
-};
+}
 
 module.exports = {
   directive: groupList,

h/static/scripts/directive/h-autofocus.js

@@ -16,7 +16,7 @@
 module.exports = function() {
   return {
     restrict: 'A',
-    link: function($scope, $element, $attrs) {
+    link: function($scope, $element) {
       $element[0].focus();
     }
   };

h/static/scripts/directive/markdown.js

 'use strict';
 
-var angular = require('angular');
 var debounce = require('lodash.debounce');
 
 var commands = require('../markdown-commands');

h/static/scripts/directive/publish-annotation-btn.js

@@ -16,15 +16,15 @@ module.exports = function () {
 
       this.publishDestination = function () {
         return this.isShared ? this.group.name : this.privateLabel;
-      }
+      };
 
       this.groupType = function () {
         return this.group.public ? 'public' : 'group';
-      }
+      };
 
       this.setPrivacy = function (level) {
         this.onSetPrivacy({level: level});
-      }
+      };
     },
     controllerAs: 'vm',
     restrict: 'E',
@@ -38,4 +38,4 @@ module.exports = function () {
     },
     template: require('../../../templates/client/publish_annotation_btn.html'),
   };
-}
+};

h/static/scripts/directive/spinner.js

+'use strict';
+
 module.exports = ['$animate', function($animate) {
-  'use strict';
   return {
     link: function(scope, elem) {
       // ngAnimate conflicts with the spinners own CSS

h/static/scripts/directive/test/annotation-test.js

@@ -29,7 +29,7 @@ function annotationDirective() {
       username: noop,
     },
     '../annotation-metadata': {
-      domainAndTitle: function (annot) {
+      domainAndTitle: function (annot) { // eslint-disable-line no-unused-vars
         return fakeDocumentMeta;
       },
     }

h/static/scripts/directive/test/group-list-test.js

@@ -6,7 +6,6 @@ var groupList = require('../group-list');
 var util = require('./util');
 
 describe('groupList', function () {
-  var $rootScope;
   var $window;
 
   var GROUP_LINK = 'https://hypothes.is/groups/hdevs';
@@ -32,8 +31,7 @@ describe('groupList', function () {
     angular.mock.module('app');
   });
 
-  beforeEach(angular.mock.inject(function (_$rootScope_, _$window_) {
-    $rootScope = _$rootScope_;
+  beforeEach(angular.mock.inject(function (_$window_) {
     $window = _$window_;
 
     groups = [{

h/static/scripts/directive/test/publish-annotation-btn-test.js

@@ -57,7 +57,7 @@ describe('publishAnnotationBtn', function () {
         type: 'group'
       },
       isShared: true
-    })
+    });
     var buttons = element.find('button');
     assert.equal(buttons[0].innerHTML, 'Post to Research Lab');
   });

h/static/scripts/directive/test/spinner-test.js

+'use strict';
+
+var angular = require('angular');
+
 var module = angular.mock.module;
 var inject = angular.mock.inject;
 
 describe('spinner', function () {
   var $animate = null;
-  var $element = null
+  var $element = null;
   var sandbox = null;
 
   before(function () {

h/static/scripts/directive/test/window-scroll-test.js

+'use strict';
+
+var angular = require('angular');
+
 var inject = angular.mock.inject;
 
 var windowScroll = require('../window-scroll');
@@ -97,7 +101,7 @@ describe('windowScroll', function () {
         assert.calledOnce(callback);
 
         view.scrollY = 1800;
-        handler()
+        handler();
         assert.calledTwice(callback);
       });
     });

h/static/scripts/directive/window-scroll.js

-var angular = require('angular');
+'use strict';
 
 module.exports = function () {
   return {

h/static/scripts/filter/persona.js

+'use strict';
+
 /**
  * Parses H account names of the form 'acct:<username>@<provider>'
  * into a {username, provider} object or null if the input does not

h/static/scripts/filter/test/persona-test.js

+'use strict';
+
 var persona = require('../persona');
 
 describe('persona', function () {

h/static/scripts/frame-rpc.js

+'use strict';
+
+/* eslint curly: "off" */
+
 /** This software is released under the MIT license:
 
   Permission is hereby granted, free of charge, to any person obtaining a copy

h/static/scripts/live-reload-client.js

 'use strict';
 
+/* eslint no-console: "off" */
+
 var queryString = require('query-string');
 
 var Socket = require('./websocket');

h/static/scripts/render-markdown.js

@@ -44,7 +44,7 @@ function extractMath(content) {
   var pos = 0;
   var replacedContent = content;
 
-  while (true) {
+  while (true) { // eslint-disable-line no-constant-condition
     var blockMathStart = replacedContent.indexOf('$$', pos);
     var inlineMathStart = replacedContent.indexOf('\\(', pos);
 

h/static/scripts/test/bootstrap.js

+'use strict';
+
 // Expose the sinon assertions.
 sinon.assert.expose(assert, {prefix: null});
 

h/static/scripts/test/streamer-test.js

@@ -8,7 +8,7 @@ var proxyquire = require('proxyquire');
 // the most recently created FakeSocket instance
 var fakeWebSocket = null;
 
-function FakeSocket(url) {
+function FakeSocket() {
   fakeWebSocket = this;
 
   this.messages = [];
@@ -25,7 +25,7 @@ function FakeSocket(url) {
   };
 
   this.close = function () {
-    this.didClose = true
+    this.didClose = true;
   };
 }
 inherits(FakeSocket, EventEmitter);
@@ -36,11 +36,10 @@ describe('streamer', function () {
   var fakeRootScope;
   var fakeSession;
   var fakeSettings;
-  var activeStreamer;
   var streamer;
 
   function createDefaultStreamer() {
-    activeStreamer = streamer.connect(
+    streamer.connect(
       fakeRootScope,
       fakeAnnotationMapper,
       fakeGroups,
@@ -81,7 +80,7 @@ describe('streamer', function () {
   });
 
   it('should not create a websocket connection if websocketUrl is not provided', function () {
-    fakeSettings = {}
+    fakeSettings = {};
     createDefaultStreamer();
     assert.isNull(fakeWebSocket);
   });
@@ -95,9 +94,8 @@ describe('streamer', function () {
 
   it('should close any existing socket', function () {
     createDefaultStreamer();
-    var oldStreamer = activeStreamer;
     var oldWebSocket = fakeWebSocket;
-    var newStreamer = streamer.connect(
+    streamer.connect(
       fakeRootScope,
       fakeAnnotationMapper,
       fakeGroups,

h/static/scripts/test/widget-controller-test.js

@@ -61,7 +61,6 @@ describe('WidgetController', function () {
   var fakeStreamer;
   var fakeStreamFilter;
   var sandbox;
-  var viewer;
 
   before(function () {
     angular.module('h', [])
@@ -138,7 +137,7 @@ describe('WidgetController', function () {
     $rootScope = _$rootScope_;
     $scope = $rootScope.$new();
     annotationUI = _annotationUI_;
-    viewer = $controller('WidgetController', {$scope: $scope});
+    $controller('WidgetController', {$scope: $scope});
   }));
 
   afterEach(function () {

package.json

@@ -37,6 +37,7 @@
     "end-of-stream": "^1.1.0",
     "escape-html": "^1.0.3",
     "escape-string-regexp": "^1.0.5",
+    "eslint": "^3.0.1",
     "exorcist": "^0.4.0",
     "extend": "^2.0.0",
     "gulp": "^3.9.1",
@@ -51,7 +52,6 @@
     "inherits": "^2.0.1",
     "jquery": "1.11.1",
     "js-polyfills": "^0.1.16",
-    "jscs": "^3.0.2",
     "karma": "^1.1.0",
     "karma-browserify": "^5.0.5",
     "karma-chai": "^0.1.0",
@@ -135,6 +135,7 @@
   "scripts": {
     "build": "gulp build-app",
     "deps": "check-dependencies",
+    "lint": "eslint .",
     "test": "gulp test-app",
     "preversion": "npm run test",
     "version": "make clean all && ./scripts/update-changelog.js && git add CHANGELOG.md",

scripts/.eslintrc

+{
+  "env": {
+    "node": true,
+  },
+  "rules": {
+    "no-console": "off",
+  },
+}

scripts/create-github-release.js

 #!/usr/bin/env node
 
+'use strict';
+
 /**
  * Creates a GitHub release for the repository.
  *

scripts/decaf/decaf.js

@@ -81,10 +81,9 @@ function stripUnnecessaryReturns(js) {
       for (var i=0; i < stripReturnPatterns.length; i++) {
         if (remainder.match(stripReturnPatterns[i])) {
           return remainder;
-        } else {
-          return remainder;
         }
       }
+      return line;
     } else {
       return line;
     }