From b87a066832df36558ddcafc81b74e56fd2d48314 Mon Sep 17 00:00:00 2001
From: Jacob Trimble <modmaker@google.com>
Date: Tue, 7 May 2019 15:53:27 -0700
Subject: [PATCH] Add some additional style rules.

- Disallow mixing different types of some operators (e.g. && ||).
- Disallow useless constructors.
- Require a newline between class members.
- Require spacing around arrow functions.
- Require using the compound assignment when possible (e.g. +=).
- Check for possible errors in assignments involving "await".

Change-Id: Ib48167aea61a62b33f0b76bb869abe18398ee5b7
---
 .eslintrc.js                            | 9 +++++++++
 lib/media/closed_caption_parser.js      | 2 --
 lib/offline/session_deleter.js          | 2 --
 lib/routing/walker.js                   | 2 ++
 test/media/play_rate_controller_unit.js | 2 +-
 test/media/playhead_unit.js             | 2 +-
 test/test/util/util.js                  | 2 +-
 7 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index 1346c12e1e..c3a09e1661 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -76,6 +76,7 @@ module.exports = {
         "getter-return": "error",
         "no-await-in-loop": "error",
         "no-template-curly-in-string": "error",
+        "require-atomic-updates": "error",
         // }}}
 
         // "Best practices" rules we should be able to pass, but are not part of "eslint:recommended": {{{
@@ -133,6 +134,14 @@ module.exports = {
 
         // Style rules that don't seem to be in the Google style config: {{{
         "array-bracket-newline": ["error", "consistent"],
+        "arrow-spacing": "error",
+        "lines-between-class-members": "error",
+        "no-mixed-operators": ["error", {
+          "groups": [["&", "|", "^", "~", "<<", ">>", ">>>", "&&", "||"]],
+          "allowSamePrecedence": false,
+        }],
+        "no-useless-constructor": "error",
+        "operator-assignment": "error",
         // }}}
     },
     "overrides": [
diff --git a/lib/media/closed_caption_parser.js b/lib/media/closed_caption_parser.js
index 289bdb66ea..f1d92437f5 100644
--- a/lib/media/closed_caption_parser.js
+++ b/lib/media/closed_caption_parser.js
@@ -125,8 +125,6 @@ shaka.media.MuxJSClosedCaptionParser = class {
  * @final
  */
 shaka.media.NoopCaptionParser = class {
-  constructor() {}
-
   /**
    * @override
    */
diff --git a/lib/offline/session_deleter.js b/lib/offline/session_deleter.js
index 25a4420d72..eeefdc90c9 100644
--- a/lib/offline/session_deleter.js
+++ b/lib/offline/session_deleter.js
@@ -26,8 +26,6 @@ goog.require('shaka.util.ArrayUtils');
  * Contains a utility method to delete persistent EME sessions.
  */
 shaka.offline.SessionDeleter = class {
-  constructor() {}
-
   /**
    * Deletes the given sessions.  This never fails and instead logs the error.
    *
diff --git a/lib/routing/walker.js b/lib/routing/walker.js
index e3e56818a1..69bd726263 100644
--- a/lib/routing/walker.js
+++ b/lib/routing/walker.js
@@ -339,6 +339,8 @@ shaka.routing.Walker = class {
     // possible for "supported errors" to occur - errors that the code using
     // the walker can't predict but can recover from.
     try {
+      // TODO: This is probably a false-positive.  See eslint/eslint#11687.
+      // eslint-disable-next-line require-atomic-updates
       this.currentStep_ = this.implementation_.enterNode(
           /* node= */ this.currentlyAt_,
           /* has= */ this.currentlyWith_,
diff --git a/test/media/play_rate_controller_unit.js b/test/media/play_rate_controller_unit.js
index 69cec6a933..99991d28e7 100644
--- a/test/media/play_rate_controller_unit.js
+++ b/test/media/play_rate_controller_unit.js
@@ -113,7 +113,7 @@ describe('PlayRateController', () => {
 
   // Make sure that when the playback rate set, if the new rate matches the
   // current rate, the controller will not set the rate on the media element.
-  it('does not redundently set the playrate', ()=> {
+  it('does not redundently set the playrate', () => {
     // Make sure we don't see the play rate change before and after we set the
     // rate on the controller.
     expect(setPlayRateSpy).not.toHaveBeenCalled();
diff --git a/test/media/playhead_unit.js b/test/media/playhead_unit.js
index e24f6456d7..ad992a884c 100644
--- a/test/media/playhead_unit.js
+++ b/test/media/playhead_unit.js
@@ -162,7 +162,7 @@ describe('Playhead', function() {
 
   function setMockDate(seconds) {
     const minutes = Math.floor(seconds / 60);
-    seconds = seconds % 60;
+    seconds %= 60;
     const mockDate = new Date(2013, 9, 23, 7, minutes, seconds);
     jasmine.clock().mockDate(mockDate);
   }
diff --git a/test/test/util/util.js b/test/test/util/util.js
index 9b8688707e..66b8bccf2a 100644
--- a/test/test/util/util.js
+++ b/test/test/util/util.js
@@ -501,7 +501,7 @@ shaka.test.Util.waitForEndOrTimeout = (eventManager, target, timeout) => {
  */
 shaka.test.Util.customMatchers_ = {
   // Custom matcher for Element objects.
-  toEqualElement: (util, customEqualityTesters) =>{
+  toEqualElement: (util, customEqualityTesters) => {
     return {
       compare: shaka.test.Util.expectToEqualElementCompare_,
     };