Reorder eslint rules and add some more.

This reorders the rules so they mostly are ordered the same as the eslint documentation. The exception is the few at the top, most of which should be removed. This also removes some rules that appear in the recommended or the Google set. Lastly this adds some more rules: - Require parenthesis when using "new". - Disallow confusing regex (e.g. /=/). - Disallow some confusing coercions (e.g. +foo). - Disallow some Unicode characters in a regex. - Disallow using "async" in a Promise constructor. Change-Id: I597b472bdaee5b4cb92354f057e7ae6aeb96eefa
Pavelas · May 9, 2019 · f225c04 · f225c04
1 parent c92a876
commit f225c04
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 71 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -58,43 +58,37 @@ module.exports = {
         "require-jsdoc": "off",
         // }}}
 
-        // Google style rules that need options: {{{
-        "block-spacing": ["error", "always"],
-        "brace-style": ["error", "1tbs", { "allowSingleLine": true }],
-        "key-spacing": ["error", {"beforeColon": false, "afterColon": true}],
-        "no-multi-spaces": ["error", { "ignoreEOLComments": true }],
-        "prefer-const": ["error", {"ignoreReadBeforeAssign": true}],
-        // }}}
-
-        // "Possible error" rules in "eslint:recommended" that need options: {{{
-        "no-empty": ["error", {"allowEmptyCatch": true}],
-        // }}}
-
-        // "Possible error" rules we should be able to pass, but are not part of "eslint:recommended": {{{
-        "for-direction": "error",
-        "getter-return": "error",
+        // "Possible error" rules: {{{
+        "no-async-promise-executor": "error",
         "no-await-in-loop": "error",
+        "no-empty": ["error", {"allowEmptyCatch": true}],
+        "no-misleading-character-class": "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": {{{
+        // "Best practices" rules: {{{
         "accessor-pairs": "error",
         "array-callback-return": "error",
+        "class-methods-use-this": "off",  // causes issues when implementing an interface
         "consistent-return": "error",
         "dot-location": ["error", "property"],
+        "dot-notation": "off",  // We use bracket notation in tests on purpose
+        "eqeqeq": "off",  // Compiler handles type checking in advance
+        "guard-for-in": "off",
         "no-alert": "error",
         "no-caller": "error",
-        "no-catch-shadow": "error",
+        "no-div-regex": "error",
         "no-extend-native": "error",  // May conflict with future polyfills
         "no-extra-label": "error",
         "no-floating-decimal": "error",
+        "no-implicit-coercion": ["error", {"allow": ["!!"]}],
         "no-implied-eval": "error",
         "no-invalid-this": "error",
         "no-iterator": "error",
-        "no-label-var": "error",
         "no-labels": "error",
         "no-lone-blocks": "error",
+        "no-multi-spaces": ["error", {"ignoreEOLComments": true}],
         "no-multi-str": "error",
         "no-new": "error",
         "no-new-func": "error",
@@ -108,42 +102,46 @@ module.exports = {
         "no-sequences": "error",
         "no-throw-literal": "error",
         "no-unmodified-loop-condition": "error",
+        "no-unused-vars": "off",  // Interface impls may not require all args
         "no-useless-call": "error",
+        "no-useless-catch": "error",
         "no-useless-concat": "error",
         "no-useless-return": "error",
         "no-void": "error",
-        "no-with": "error",
+        "no-warning-comments": "off",  // TODO and FIXME are fine
         "radix": ["error", "always"],
         "require-await": "error",
         "wrap-iife": ["error", "inside"],
+        "yoda": ["error", "never"],
         // }}}
 
-        // Style rules we don't need: {{{
-        "class-methods-use-this": "off",  // causes issues when implementing an interface
-        "dot-notation": "off",  // We use bracket notation in tests on purpose
-        "eqeqeq": "off",  // Compiler handles type checking in advance
-        "guard-for-in": "off",
-        "no-div-regex": "off",  // Conflicts with no-useless-escape
+        // "Variables" rules: {{{
+        "no-label-var": "error",
+        "no-shadow-restricted-names": "error",
         "no-undef-init": "off",  // Sometimes necessary with hacky compiler casts
         "no-undefined": "off",  // We use undefined in many places, legitimately
-        "no-unused-vars": "off",  // Interface impls may not require all args
         "no-use-before-define": "off",  // Does not know when things are executed, false positives
-        "no-warning-comments": "off",  // TODO and FIXME are fine
-        "vars-on-top": "off",
-        "yoda": ["error", "never"],
         // }}}
 
-        // Style rules that don't seem to be in the Google style config: {{{
+        // "Stylistic Issues" rules: {{{
         "array-bracket-newline": ["error", "consistent"],
-        "arrow-spacing": "error",
+        "block-spacing": ["error", "always"],
+        "brace-style": ["error", "1tbs", { "allowSingleLine": true }],
         "lines-between-class-members": "error",
+        "new-parens": "error",
         "no-mixed-operators": ["error", {
           "groups": [["&", "|", "^", "~", "<<", ">>", ">>>", "&&", "||"]],
           "allowSamePrecedence": false,
         }],
-        "no-useless-constructor": "error",
+        "no-whitespace-before-property": "error",
         "operator-assignment": "error",
+        // }}}
+
+        // "ECMAScript 6" rules: {{{
+        "arrow-spacing": "error",
+        "no-useless-constructor": "error",
         "prefer-arrow-callback": "error",
+        "prefer-const": ["error", {"ignoreReadBeforeAssign": true}],
         // }}}
     },
     "overrides": [

diff --git a/lib/util/uint8array_utils.js b/lib/util/uint8array_utils.js
@@ -52,7 +52,7 @@ shaka.util.Uint8ArrayUtils.toBase64 = function(arr, padding) {
   padding = (padding == undefined) ? true : padding;
   const base64 = shaka.util.Uint8ArrayUtils.toStandardBase64(arr)
     .replace(/\+/g, '-').replace(/\//g, '_');
-  return padding ? base64 : base64.replace(/=*$/, '');
+  return padding ? base64 : base64.replace(/[=]*$/, '');
 };
 
 /**

diff --git a/test/dash/mpd_utils_unit.js b/test/dash/mpd_utils_unit.js
@@ -533,7 +533,7 @@ describe('MpdUtils', () => {
       // infinitely recurse if there isn't a depth limit.
       for (let i = 1; i < 20; i++) {
         const key = 'https://xlink' + i;
-        const value = makeRecursiveXMLString(0, 'https://xlink' + (i + 1) + '');
+        const value = makeRecursiveXMLString(0, 'https://xlink' + (i + 1));
 
         fakeNetEngine.setResponseText(key, value);
       }
@@ -656,7 +656,7 @@ describe('MpdUtils', () => {
       // didn't abort it.
       for (let i = 1; i < 3; i++) {
         const key = 'https://xlink' + i;
-        const value = makeRecursiveXMLString(0, 'https://xlink' + (i + 1) + '');
+        const value = makeRecursiveXMLString(0, 'https://xlink' + (i + 1));
 
         fakeNetEngine.setResponseText(key, value);
       }

diff --git a/test/media/adaptation_set_unit.js b/test/media/adaptation_set_unit.js
@@ -185,7 +185,7 @@ describe('AdaptationSet', () => {
       label: null,
       language: '',
       mimeType: mimeType,
-      originalId: '' + id,
+      originalId: String(id),
       primary: false,
       roles: roles,
       trickModeVideo: null,

diff --git a/test/offline/storage_unit.js b/test/offline/storage_unit.js
@@ -33,7 +33,7 @@ describe('Storage', () => {
 
   const noMetadata = {};
 
-  const kbps = 1000;
+  const kbps = (k) => k * 1000;
 
   beforeEach(async () => {
     // Make sure we start with a clean slate between each run.
@@ -284,12 +284,12 @@ describe('Storage', () => {
 
     it('selects the largest SD video with middle quality audio', () => {
       const tracks = [
-        variantTrack(0, 360, englishUS, 1 * kbps),
-        variantTrack(1, 480, englishUS, 2.0 * kbps),
-        variantTrack(2, 480, englishUS, 2.1 * kbps),
-        variantTrack(3, 480, englishUS, 2.2 * kbps),
-        variantTrack(4, 720, englishUS, 3 * kbps),
-        variantTrack(5, 1080, englishUS, 4 * kbps),
+        variantTrack(0, 360, englishUS, kbps(1.0)),
+        variantTrack(1, 480, englishUS, kbps(2.0)),
+        variantTrack(2, 480, englishUS, kbps(2.1)),
+        variantTrack(3, 480, englishUS, kbps(2.2)),
+        variantTrack(4, 720, englishUS, kbps(3.0)),
+        variantTrack(5, 1080, englishUS, kbps(4.0)),
       ];
 
       const selected =
@@ -299,7 +299,7 @@ describe('Storage', () => {
       expect(selected[0]).toBeTruthy();
       expect(selected[0].language).toBe(englishUS);
       expect(selected[0].height).toBe(480);
-      expect(selected[0].bandwidth).toBe(2.1 * kbps);
+      expect(selected[0].bandwidth).toBe(kbps(2.1));
     });
 
     it('selects all text tracks', () => {
@@ -320,9 +320,9 @@ describe('Storage', () => {
     describe('language matching', () => {
       it('finds exact match', () => {
         const tracks = [
-          variantTrack(0, 480, 'eng-us', 1 * kbps),
-          variantTrack(1, 480, 'fr-ca', 1 * kbps),
-          variantTrack(2, 480, 'eng-ca', 1 * kbps),
+          variantTrack(0, 480, 'eng-us', kbps(1)),
+          variantTrack(1, 480, 'fr-ca', kbps(1)),
+          variantTrack(2, 480, 'eng-ca', kbps(1)),
         ];
 
         const selected =
@@ -335,10 +335,10 @@ describe('Storage', () => {
 
       it('finds exact match with only base', () => {
         const tracks = [
-          variantTrack(0, 480, 'eng-us', 1 * kbps),
-          variantTrack(1, 480, 'fr-ca', 1 * kbps),
-          variantTrack(2, 480, 'eng-ca', 1 * kbps),
-          variantTrack(3, 480, 'eng', 1 * kbps),
+          variantTrack(0, 480, 'eng-us', kbps(1)),
+          variantTrack(1, 480, 'fr-ca', kbps(1)),
+          variantTrack(2, 480, 'eng-ca', kbps(1)),
+          variantTrack(3, 480, 'eng', kbps(1)),
         ];
 
         const selected = PlayerConfiguration.defaultTrackSelect(tracks, 'eng');
@@ -350,9 +350,9 @@ describe('Storage', () => {
 
       it('finds base match when exact match is not found', () => {
         const tracks = [
-          variantTrack(0, 480, 'eng-us', 1 * kbps),
-          variantTrack(1, 480, 'fr-ca', 1 * kbps),
-          variantTrack(2, 480, 'eng-ca', 1 * kbps),
+          variantTrack(0, 480, 'eng-us', kbps(1)),
+          variantTrack(1, 480, 'fr-ca', kbps(1)),
+          variantTrack(2, 480, 'eng-ca', kbps(1)),
         ];
 
         const selected = PlayerConfiguration.defaultTrackSelect(tracks, 'fr');
@@ -364,9 +364,9 @@ describe('Storage', () => {
 
       it('finds common base when exact match is not found', () => {
         const tracks = [
-          variantTrack(0, 480, 'eng-us', 1 * kbps),
-          variantTrack(1, 480, 'fr-ca', 1 * kbps),
-          variantTrack(2, 480, 'eng-ca', 1 * kbps),
+          variantTrack(0, 480, 'eng-us', kbps(1)),
+          variantTrack(1, 480, 'fr-ca', kbps(1)),
+          variantTrack(2, 480, 'eng-ca', kbps(1)),
         ];
 
         const selected =
@@ -379,9 +379,9 @@ describe('Storage', () => {
 
       it('finds primary track when no match is found', () => {
         const tracks = [
-          variantTrack(0, 480, 'eng-us', 1 * kbps),
-          variantTrack(1, 480, 'fr-ca', 1 * kbps),
-          variantTrack(2, 480, 'eng-ca', 1 * kbps),
+          variantTrack(0, 480, 'eng-us', kbps(1)),
+          variantTrack(1, 480, 'fr-ca', kbps(1)),
+          variantTrack(2, 480, 'eng-ca', kbps(1)),
         ];
 
         tracks[0].primary = true;
@@ -629,9 +629,9 @@ describe('Storage', () => {
       const manifest = new shaka.test.ManifestGenerator()
           .setPresentationDuration(20)
           .addPeriod(0)
-              .addVariant(0).language(englishUS).bandwidth(13 * kbps)
-                  .addVideo(1).size(100, 200).bandwidth(10 * kbps)
-                  .addAudio(2).language(englishUS).bandwidth(3 * kbps)
+              .addVariant(0).language(englishUS).bandwidth(kbps(13))
+                  .addVideo(1).size(100, 200).bandwidth(kbps(10))
+                  .addAudio(2).language(englishUS).bandwidth(kbps(3))
           .build();
 
       const audio = manifest.periods[0].variants[0].audio;
@@ -1256,12 +1256,12 @@ describe('Storage', () => {
     const manifest = new shaka.test.ManifestGenerator()
         .setPresentationDuration(20)
         .addPeriod(0)
-            .addVariant(0).language(englishUS).bandwidth(13 * kbps)
-                .addVideo(1).size(100, 200).bandwidth(10 * kbps)
-                .addAudio(2).language(englishUS).bandwidth(3 * kbps)
-            .addVariant(3).language(frenchCanadian).bandwidth(13 * kbps)
-                .addVideo(4).size(100, 200).bandwidth(10 * kbps)
-                .addAudio(5).language(frenchCanadian).bandwidth(3 * kbps)
+            .addVariant(0).language(englishUS).bandwidth(kbps(13))
+                .addVideo(1).size(100, 200).bandwidth(kbps(10))
+                .addAudio(2).language(englishUS).bandwidth(kbps(3))
+            .addVariant(3).language(frenchCanadian).bandwidth(kbps(13))
+                .addVideo(4).size(100, 200).bandwidth(kbps(10))
+                .addAudio(5).language(frenchCanadian).bandwidth(kbps(3))
         .build();
 
     getAllStreams(manifest).forEach((stream) => {

diff --git a/test/player_src_equals_integration.js b/test/player_src_equals_integration.js
@@ -39,7 +39,7 @@ describe('Player Src Equals', () => {
   beforeEach(() => {
     player = new shaka.Player();
     player.addEventListener('error', fail);
-    eventManager = new shaka.util.EventManager;
+    eventManager = new shaka.util.EventManager();
   });
 
   afterEach(async () => {

diff --git a/test/test/util/streaming_engine_util.js b/test/test/util/streaming_engine_util.js
@@ -227,7 +227,7 @@ shaka.test.StreamingEngineUtil.createManifest = function(
 
     const d = segmentDurations[type];
     const getUris = function() {
-      return ['' + periodNumber + '_' + type + '_' + position];
+      return [periodNumber + '_' + type + '_' + position];
     };
     return new shaka.media.SegmentReference(
         position, (position - 1) * d, position * d, getUris, 0, null);