From 350121ef0b698637c45ee1a3f31673965fbda12f Mon Sep 17 00:00:00 2001
From: Patrick Hulce <patrick.hulce@gmail.com>
Date: Tue, 28 Aug 2018 15:36:26 -0700
Subject: [PATCH 1/5] core: remove esprima

---
 .../audits/byte-efficiency/unminified-css.js  |  54 +-----
 .../byte-efficiency/unminified-javascript.js  |  14 +-
 lighthouse-core/lib/minification-estimator.js | 154 ++++++++++++++++++
 .../byte-efficiency/unminified-css-test.js    | 103 ------------
 .../test/lib/minification-estimator-test.js   | 111 +++++++++++++
 5 files changed, 269 insertions(+), 167 deletions(-)
 create mode 100644 lighthouse-core/lib/minification-estimator.js
 create mode 100644 lighthouse-core/test/lib/minification-estimator-test.js

diff --git a/lighthouse-core/audits/byte-efficiency/unminified-css.js b/lighthouse-core/audits/byte-efficiency/unminified-css.js
index 7b0bcd609b10..23245aa243ec 100644
--- a/lighthouse-core/audits/byte-efficiency/unminified-css.js
+++ b/lighthouse-core/audits/byte-efficiency/unminified-css.js
@@ -8,6 +8,7 @@
 const ByteEfficiencyAudit = require('./byte-efficiency-audit');
 const UnusedCSSRules = require('./unused-css-rules');
 const i18n = require('../../lib/i18n');
+const computeTokenLength = require('../../lib/minification-estimator').estimateCSSTokenLength;
 
 const UIStrings = {
   /** Imperative title of a Lighthouse audit that tells the user to minify (remove whitespace) the page's CSS code. This is displayed in a list of audit titles that Lighthouse generates. */
@@ -46,58 +47,7 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
    * @return {number}
    */
   static computeTokenLength(content) {
-    let totalTokenLength = 0;
-    let isInComment = false;
-    let isInLicenseComment = false;
-    let isInString = false;
-    let stringOpenChar = null;
-
-    for (let i = 0; i < content.length; i++) {
-      const twoChars = content.substr(i, 2);
-      const char = twoChars.charAt(0);
-
-      const isWhitespace = char === ' ' || char === '\n' || char === '\t';
-      const isAStringOpenChar = char === `'` || char === '"';
-
-      if (isInComment) {
-        if (isInLicenseComment) totalTokenLength++;
-
-        if (twoChars === '*/') {
-          if (isInLicenseComment) totalTokenLength++;
-          isInComment = false;
-          i++;
-        }
-      } else if (isInString) {
-        totalTokenLength++;
-        if (char === '\\') {
-          totalTokenLength++;
-          i++;
-        } else if (char === stringOpenChar) {
-          isInString = false;
-        }
-      } else {
-        if (twoChars === '/*') {
-          isInComment = true;
-          isInLicenseComment = content.charAt(i + 2) === '!';
-          if (isInLicenseComment) totalTokenLength += 2;
-          i++;
-        } else if (isAStringOpenChar) {
-          isInString = true;
-          stringOpenChar = char;
-          totalTokenLength++;
-        } else if (!isWhitespace) {
-          totalTokenLength++;
-        }
-      }
-    }
-
-    // If the content contained unbalanced comments, it's either invalid or we had a parsing error.
-    // Report the token length as the entire string so it will be ignored.
-    if (isInComment || isInString) {
-      return content.length;
-    }
-
-    return totalTokenLength;
+    return computeTokenLength(content);
   }
 
   /**
diff --git a/lighthouse-core/audits/byte-efficiency/unminified-javascript.js b/lighthouse-core/audits/byte-efficiency/unminified-javascript.js
index 79b424a76b3f..469a4ee942ce 100644
--- a/lighthouse-core/audits/byte-efficiency/unminified-javascript.js
+++ b/lighthouse-core/audits/byte-efficiency/unminified-javascript.js
@@ -6,8 +6,8 @@
 'use strict';
 
 const ByteEfficiencyAudit = require('./byte-efficiency-audit');
-const esprima = require('esprima');
 const i18n = require('../../lib/i18n');
+const computeTokenLength = require('../../lib/minification-estimator').estimateJSTokenLength;
 
 const UIStrings = {
   /** Imperative title of a Lighthouse audit that tells the user to minify the page’s JS code to reduce file size. This is displayed in a list of audit titles that Lighthouse generates. */
@@ -53,17 +53,7 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
    */
   static computeWaste(scriptContent, networkRecord) {
     const contentLength = scriptContent.length;
-    let totalTokenLength = 0;
-
-    /** @type {Array<esprima.Token> & {errors: Error[]}} */
-    const tokens = (esprima.tokenize(scriptContent, {tolerant: true}));
-    if (!tokens.length && tokens.errors && tokens.errors.length) {
-      throw tokens.errors[0];
-    }
-
-    for (const token of tokens) {
-      totalTokenLength += token.value.length;
-    }
+    const totalTokenLength = computeTokenLength(scriptContent);
 
     const totalBytes = ByteEfficiencyAudit.estimateTransferSize(networkRecord, contentLength,
       'Script');
diff --git a/lighthouse-core/lib/minification-estimator.js b/lighthouse-core/lib/minification-estimator.js
new file mode 100644
index 000000000000..060cde2e4364
--- /dev/null
+++ b/lighthouse-core/lib/minification-estimator.js
@@ -0,0 +1,154 @@
+
+// https://www.ecma-international.org/ecma-262/9.0/index.html#prod-Punctuator
+const PUNCTUATOR_REGEX = /(return|{|\(|\[|\.\.\.|;|,|<|>|<=|>=|==|!=|===|!==|\+|-|\*|%|\*\*|\+\+|--|<<|>>|>>>|&|\||\^|!|~|&&|\|\||\?|:|=|\+=|-=|\*=|%=|\*\*=|<<=|>>=|>>>=|&=|\|=|\^=|=>)$/
+const WHITESPACE_REGEX = /( |\n|\t)+$/
+
+/**
+ * @param {string} content
+ * @param {number} startPosition
+ */
+function hasPunctuatorBefore(content, startPosition) {
+  for (let i = startPosition - 1; i >= 0; i--) {
+    if (i < 3) return true
+
+    const precedingCharacters = content.substr(i - 5, 6);
+    if (WHITESPACE_REGEX.test(precedingCharacters)) continue;
+    return PUNCTUATOR_REGEX.test(precedingCharacters);
+  }
+
+  return true;
+}
+
+
+/**
+ *
+ * @param {string} content
+ * @param {{singlelineComments: boolean, regex: boolean}} features
+ */
+function computeTokenLength(content, features) {
+  let totalTokenLength = 0;
+  let isInSinglelineComment = false;
+  let isInMultilineComment = false;
+  let isInLicenseComment = false;
+  let isInString = false;
+  let isInRegex = false;
+  let stringOpenChar = null;
+
+  for (let i = 0; i < content.length; i++) {
+    const twoChars = content.substr(i, 2);
+    const char = twoChars.charAt(0);
+
+    const isWhitespace = char === ' ' || char === '\n' || char === '\t';
+    const isAStringOpenChar = char === `'` || char === '"' || char === '`';
+
+    if (twoChars === 'yO') debugger
+
+    if (isInSinglelineComment) {
+      if (char === '\n') {
+        // console.log(i, 'leaving comment')
+        // End the comment when you hit a newline
+        isInSinglelineComment = false;
+      }
+    } else if (isInMultilineComment) {
+      // License comments count
+      if (isInLicenseComment) totalTokenLength++;
+
+      if (twoChars === '*/') {
+        // License comments count, account for the '/' character we're skipping over
+        if (isInLicenseComment) totalTokenLength++;
+        // End the comment when we hit the closing sequence
+        isInMultilineComment = false;
+        // Skip over the '/' character since we've already processed it
+        i++;
+      }
+    } else if (isInString) {
+      // String characters count
+      totalTokenLength++;
+
+      if (char === '\\') {
+        // Skip over any escaped characters
+        totalTokenLength++;
+        i++;
+      } else if (char === stringOpenChar) {
+        // End the string when we hit the same stringOpenCharacter
+        isInString = false;
+        // console.log(i, 'exiting string', stringOpenChar)
+      }
+    } else if (isInRegex) {
+      // Regex characters count
+      totalTokenLength++;
+
+      if (char === '\\') {
+        // Skip over any escaped characters
+        totalTokenLength++;
+        i++;
+      } else if (char === '/') {
+        // End the string when we hit the regex close character
+        isInRegex = false;
+        // console.log(i, 'leaving regex', char)
+      }
+    } else {
+      if (twoChars === '/*') {
+        // Start the multi-line comment
+        isInMultilineComment = true;
+        // Check if it's a license comment so we know whether to count it
+        isInLicenseComment = content.charAt(i + 2) === '!';
+        // += 2 because we are processing 2 characters, not just 1
+        if (isInLicenseComment) totalTokenLength += 2;
+        // Skip over the '*' character since we've already processed it
+        i++;
+      } else if (twoChars === '//' && features.singlelineComments) {
+        // console.log(i, 'entering comment')
+        // Start the single-line comment
+        isInSinglelineComment = true;
+        isInMultilineComment = false;
+        isInLicenseComment = false;
+        // Skip over the second '/' character since we've already processed it
+        i++;
+      } else if (char === '/' && features.regex && hasPunctuatorBefore(content, i)) {
+        // Start the regex
+        isInRegex = true;
+        // Regex characters count
+        totalTokenLength++;
+        // console.log(i, 'entering regex', char)
+      } else if (isAStringOpenChar) {
+        // Start the string
+        isInString = true;
+        // Save the open character for later so we know when to close it
+        stringOpenChar = char;
+        // String characters count
+        totalTokenLength++;
+        // console.log(i, 'entering string', char)
+      } else if (!isWhitespace) {
+        // All non-whitespace, non-semicolon characters count
+        totalTokenLength++;
+      }
+    }
+  }
+
+  // console.log(content.substr(50081 - 80, 82))
+
+  // If the content contained unbalanced comments, it's either invalid or we had a parsing error.
+  // Report the token length as the entire string so it will be ignored.
+  if (isInMultilineComment || isInString) {
+    return content.length;
+  }
+
+  return totalTokenLength;
+}
+
+/**
+ * @param {string} content
+ */
+function computeJSTokenLength(content) {
+  return computeTokenLength(content, {singlelineComments: true, regex: true})
+}
+
+/**
+ * @param {string} content
+ */
+function computeCSSTokenLength(content) {
+  return computeTokenLength(content, {singlelineComments: false, regex: false})
+}
+
+module.exports = {computeJSTokenLength, computeCSSTokenLength}
diff --git a/lighthouse-core/test/audits/byte-efficiency/unminified-css-test.js b/lighthouse-core/test/audits/byte-efficiency/unminified-css-test.js
index d2f358eb02fe..9d9d55aa3f1c 100644
--- a/lighthouse-core/test/audits/byte-efficiency/unminified-css-test.js
+++ b/lighthouse-core/test/audits/byte-efficiency/unminified-css-test.js
@@ -13,109 +13,6 @@ const assert = require('assert');
 
 const resourceType = 'Stylesheet';
 describe('Page uses optimized css', () => {
-  describe('#computeTokenLength', () => {
-    it('should compute length of meaningful content', () => {
-      const full = `
-        /*
-         * a complicated comment
-         * that is
-         * several
-         * lines
-         */
-        .my-class {
-          /* a simple comment */
-          width: 100px;
-          height: 100px;
-        }
-      `;
-
-      const minified = '.my-class{width:100px;height:100px;}';
-      assert.equal(UnminifiedCssAudit.computeTokenLength(full), minified.length);
-    });
-
-    it('should handle string edge cases', () => {
-      const pairs = [
-        ['.my-class { content: "/*"; }', '.my-class{content:"/*";}'],
-        ['.my-class { content: \'/* */\'; }', '.my-class{content:\'/* */\';}'],
-        ['.my-class { content: "/*\\\\a"; }', '.my-class{content:"/*\\\\a";}'],
-        ['.my-class { content: "/*\\"a"; }', '.my-class{content:"/*\\"a";}'],
-        ['.my-class { content: "hello }', '.my-class { content: "hello }'],
-        ['.my-class { content: "hello" }', '.my-class{content:"hello"}'],
-      ];
-
-      for (const [full, minified] of pairs) {
-        assert.equal(
-          UnminifiedCssAudit.computeTokenLength(full),
-          minified.length,
-          `did not handle ${full} properly`
-        );
-      }
-    });
-
-    it('should handle comment edge cases', () => {
-      const full = `
-        /* here is a cool "string I found" */
-        .my-class {
-          content: "/*";
-        }
-      `;
-
-      const minified = '.my-class{content:"/*";}';
-      assert.equal(UnminifiedCssAudit.computeTokenLength(full), minified.length);
-    });
-
-    it('should handle license comments', () => {
-      const full = `
-        /*!
-         * @LICENSE
-         * Apache 2.0
-         */
-        .my-class {
-          width: 100px;
-        }
-      `;
-
-      const minified = `/*!
-         * @LICENSE
-         * Apache 2.0
-         */.my-class{width:100px;}`;
-      assert.equal(UnminifiedCssAudit.computeTokenLength(full), minified.length);
-    });
-
-    it('should handle unbalanced comments', () => {
-      const full = `
-        /*
-        .my-class {
-          width: 100px;
-        }
-      `;
-
-      assert.equal(UnminifiedCssAudit.computeTokenLength(full), full.length);
-    });
-
-    it('should handle data URIs', () => {
-      const uri = 'data:image/jpeg;base64,asdfadiosgjwiojasfaasd';
-      const full = `
-        .my-other-class {
-          background: data("${uri}");
-          height: 100px;
-        }
-     `;
-
-      const minified = `.my-other-class{background:data("${uri}");height:100px;}`;
-      assert.equal(UnminifiedCssAudit.computeTokenLength(full), minified.length);
-    });
-
-    it('should handle reeally long strings', () => {
-      let hugeCss = '';
-      for (let i = 0; i < 10000; i++) {
-        hugeCss += `.my-class-${i} { width: 100px; height: 100px; }\n`;
-      }
-
-      assert.ok(UnminifiedCssAudit.computeTokenLength(hugeCss) < 0.9 * hugeCss.length);
-    });
-  });
-
   it('fails when given unminified stylesheets', () => {
     const auditResult = UnminifiedCssAudit.audit_(
       {
diff --git a/lighthouse-core/test/lib/minification-estimator-test.js b/lighthouse-core/test/lib/minification-estimator-test.js
new file mode 100644
index 000000000000..6de03e38237f
--- /dev/null
+++ b/lighthouse-core/test/lib/minification-estimator-test.js
@@ -0,0 +1,111 @@
+const assert = require('assert');
+const {computeCSSTokenLength, computeJSTokenLength} = require('../../lib/minification-estimator');
+
+describe('minification estimator', () => {
+  describe('CSS', () => {
+    it('should compute length of meaningful content', () => {
+      const full = `
+        /*
+         * a complicated comment
+         * that is
+         * several
+         * lines
+         */
+        .my-class {
+          /* a simple comment */
+          width: 100px;
+          height: 100px;
+        }
+      `;
+
+      const minified = '.my-class{width:100px;height:100px;}';
+      assert.equal(computeCSSTokenLength(full), minified.length);
+    });
+
+    it('should handle string edge cases', () => {
+      const pairs = [
+        ['.my-class { content: "/*"; }', '.my-class{content:"/*";}'],
+        ['.my-class { content: \'/* */\'; }', '.my-class{content:\'/* */\';}'],
+        ['.my-class { content: "/*\\\\a"; }', '.my-class{content:"/*\\\\a";}'],
+        ['.my-class { content: "/*\\"a"; }', '.my-class{content:"/*\\"a";}'],
+        ['.my-class { content: "hello }', '.my-class { content: "hello }'],
+        ['.my-class { content: "hello" }', '.my-class{content:"hello"}'],
+      ];
+
+      for (const [full, minified] of pairs) {
+        assert.equal(
+          computeCSSTokenLength(full),
+          minified.length,
+          `did not handle ${full} properly`
+        );
+      }
+    });
+
+    it('should handle comment edge cases', () => {
+      const full = `
+        /* here is a cool "string I found" */
+        .my-class {
+          content: "/*";
+        }
+      `;
+
+      const minified = '.my-class{content:"/*";}';
+      assert.equal(computeCSSTokenLength(full), minified.length);
+    });
+
+    it('should handle license comments', () => {
+      const full = `
+        /*!
+         * @LICENSE
+         * Apache 2.0
+         */
+        .my-class {
+          width: 100px;
+        }
+      `;
+
+      const minified = `/*!
+         * @LICENSE
+         * Apache 2.0
+         */.my-class{width:100px;}`;
+      assert.equal(computeCSSTokenLength(full), minified.length);
+    });
+
+    it('should handle unbalanced comments', () => {
+      const full = `
+        /*
+        .my-class {
+          width: 100px;
+        }
+      `;
+
+      assert.equal(computeCSSTokenLength(full), full.length);
+    });
+
+    it('should handle data URIs', () => {
+      const uri = 'data:image/jpeg;base64,asdfadiosgjwiojasfaasd';
+      const full = `
+        .my-other-class {
+          background: data("${uri}");
+          height: 100px;
+        }
+     `;
+
+      const minified = `.my-other-class{background:data("${uri}");height:100px;}`;
+      assert.equal(computeCSSTokenLength(full), minified.length);
+    });
+
+    it('should handle reeally long strings', () => {
+      let hugeCss = '';
+      for (let i = 0; i < 10000; i++) {
+        hugeCss += `.my-class-${i} { width: 100px; height: 100px; }\n`;
+      }
+
+      assert.ok(computeCSSTokenLength(hugeCss) < 0.9 * hugeCss.length);
+    });
+  });
+
+  describe('JS', () => {
+
+  });
+})

From 238a4daf865c6d0c88208d5d8d54bffe0a07001c Mon Sep 17 00:00:00 2001
From: Patrick Hulce <patrick.hulce@gmail.com>
Date: Tue, 28 Aug 2018 15:46:39 -0700
Subject: [PATCH 2/5] fixes

---
 lighthouse-core/audits/byte-efficiency/unminified-css.js        | 2 +-
 lighthouse-core/audits/byte-efficiency/unminified-javascript.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lighthouse-core/audits/byte-efficiency/unminified-css.js b/lighthouse-core/audits/byte-efficiency/unminified-css.js
index 23245aa243ec..199f6f325245 100644
--- a/lighthouse-core/audits/byte-efficiency/unminified-css.js
+++ b/lighthouse-core/audits/byte-efficiency/unminified-css.js
@@ -8,7 +8,7 @@
 const ByteEfficiencyAudit = require('./byte-efficiency-audit');
 const UnusedCSSRules = require('./unused-css-rules');
 const i18n = require('../../lib/i18n');
-const computeTokenLength = require('../../lib/minification-estimator').estimateCSSTokenLength;
+const computeTokenLength = require('../../lib/minification-estimator').computeCSSTokenLength;
 
 const UIStrings = {
   /** Imperative title of a Lighthouse audit that tells the user to minify (remove whitespace) the page's CSS code. This is displayed in a list of audit titles that Lighthouse generates. */
diff --git a/lighthouse-core/audits/byte-efficiency/unminified-javascript.js b/lighthouse-core/audits/byte-efficiency/unminified-javascript.js
index 469a4ee942ce..a7be66ddaf2b 100644
--- a/lighthouse-core/audits/byte-efficiency/unminified-javascript.js
+++ b/lighthouse-core/audits/byte-efficiency/unminified-javascript.js
@@ -7,7 +7,7 @@
 
 const ByteEfficiencyAudit = require('./byte-efficiency-audit');
 const i18n = require('../../lib/i18n');
-const computeTokenLength = require('../../lib/minification-estimator').estimateJSTokenLength;
+const computeTokenLength = require('../../lib/minification-estimator').computeJSTokenLength;
 
 const UIStrings = {
   /** Imperative title of a Lighthouse audit that tells the user to minify the page’s JS code to reduce file size. This is displayed in a list of audit titles that Lighthouse generates. */

From ee3e20e9a69411b8c7b48f91f2cbbaaaf40b0d58 Mon Sep 17 00:00:00 2001
From: Patrick Hulce <patrick.hulce@gmail.com>
Date: Tue, 28 Aug 2018 15:47:08 -0700
Subject: [PATCH 3/5] debugs

---
 lighthouse-core/lib/minification-estimator.js | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/lighthouse-core/lib/minification-estimator.js b/lighthouse-core/lib/minification-estimator.js
index 060cde2e4364..e46dbd52134e 100644
--- a/lighthouse-core/lib/minification-estimator.js
+++ b/lighthouse-core/lib/minification-estimator.js
@@ -41,8 +41,6 @@ function computeTokenLength(content, features) {
     const isWhitespace = char === ' ' || char === '\n' || char === '\t';
     const isAStringOpenChar = char === `'` || char === '"' || char === '`';
 
-    if (twoChars === 'yO') debugger
-
     if (isInSinglelineComment) {
       if (char === '\n') {
         // console.log(i, 'leaving comment')
@@ -126,8 +124,6 @@ function computeTokenLength(content, features) {
     }
   }
 
-  // console.log(content.substr(50081 - 80, 82))
-
   // If the content contained unbalanced comments, it's either invalid or we had a parsing error.
   // Report the token length as the entire string so it will be ignored.
   if (isInMultilineComment || isInString) {

From 9677eed8aa6b0867e2c8f45de9f63361e7b16ad1 Mon Sep 17 00:00:00 2001
From: Patrick Hulce <patrick.hulce@gmail.com>
Date: Fri, 21 Sep 2018 17:06:05 -0500
Subject: [PATCH 4/5] clean up + tests

---
 lighthouse-core/lib/minification-estimator.js | 33 ++++++---
 .../test/lib/minification-estimator-test.js   | 70 +++++++++++++++++++
 2 files changed, 92 insertions(+), 11 deletions(-)

diff --git a/lighthouse-core/lib/minification-estimator.js b/lighthouse-core/lib/minification-estimator.js
index e46dbd52134e..007c7d62c978 100644
--- a/lighthouse-core/lib/minification-estimator.js
+++ b/lighthouse-core/lib/minification-estimator.js
@@ -1,21 +1,35 @@
+/**
+ * @license Copyright 2018 Google Inc. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+'use strict';
 
-// https://www.ecma-international.org/ecma-262/9.0/index.html#prod-Punctuator
-const PUNCTUATOR_REGEX = /(return|{|\(|\[|\.\.\.|;|,|<|>|<=|>=|==|!=|===|!==|\+|-|\*|%|\*\*|\+\+|--|<<|>>|>>>|&|\||\^|!|~|&&|\|\||\?|:|=|\+=|-=|\*=|%=|\*\*=|<<=|>>=|>>>=|&=|\|=|\^=|=>)$/
+// https://www.ecma-international.org/ecma-262/9.0/index.html#sec-punctuators
+const PUNCTUATOR_REGEX = /(return|{|\(|\[|\.\.\.|;|,|<|>|<=|>=|==|!=|===|!==|\+|-|\*|%|\*\*|\+\+|--|<<|>>|>>>|&|\||\^|!|~|&&|\|\||\?|:|=|\+=|-=|\*=|%=|\*\*=|<<=|>>=|>>>=|&=|\|=|\^=|=>|\/|\/=|\})$/
 const WHITESPACE_REGEX = /( |\n|\t)+$/
 
 /**
+ * Look backwards from `startPosition` in `content` for an ECMAScript punctuator.
+ * This is used to differentiate a RegExp from a divide statement.
+ * If a punctuator immediately precedes a lone `/`, the `/` must be the start of a RegExp.
+ *
  * @param {string} content
  * @param {number} startPosition
  */
 function hasPunctuatorBefore(content, startPosition) {
-  for (let i = startPosition - 1; i >= 0; i--) {
-    if (i < 3) return true
-
-    const precedingCharacters = content.substr(i - 5, 6);
+  for (let i = startPosition; i > 0; i--) {
+    // Try to grab at least 6 characters so we can check for `return`
+    const sliceStart = Math.max(0, i - 6);
+    const precedingCharacters = content.slice(sliceStart, i);
+    // Skip over any ending whitespace
     if (WHITESPACE_REGEX.test(precedingCharacters)) continue;
+    // Check if it's a punctuator
     return PUNCTUATOR_REGEX.test(precedingCharacters);
   }
 
+  // The beginning of the content counts too for our purposes.
+  // i.e. a script can't start with a divide symbol
   return true;
 }
 
@@ -43,7 +57,6 @@ function computeTokenLength(content, features) {
 
     if (isInSinglelineComment) {
       if (char === '\n') {
-        // console.log(i, 'leaving comment')
         // End the comment when you hit a newline
         isInSinglelineComment = false;
       }
@@ -86,6 +99,7 @@ function computeTokenLength(content, features) {
         // console.log(i, 'leaving regex', char)
       }
     } else {
+      // We're not in any particular token mode, look for the start of different
       if (twoChars === '/*') {
         // Start the multi-line comment
         isInMultilineComment = true;
@@ -96,7 +110,6 @@ function computeTokenLength(content, features) {
         // Skip over the '*' character since we've already processed it
         i++;
       } else if (twoChars === '//' && features.singlelineComments) {
-        // console.log(i, 'entering comment')
         // Start the single-line comment
         isInSinglelineComment = true;
         isInMultilineComment = false;
@@ -108,7 +121,6 @@ function computeTokenLength(content, features) {
         isInRegex = true;
         // Regex characters count
         totalTokenLength++;
-        // console.log(i, 'entering regex', char)
       } else if (isAStringOpenChar) {
         // Start the string
         isInString = true;
@@ -116,9 +128,8 @@ function computeTokenLength(content, features) {
         stringOpenChar = char;
         // String characters count
         totalTokenLength++;
-        // console.log(i, 'entering string', char)
       } else if (!isWhitespace) {
-        // All non-whitespace, non-semicolon characters count
+        // All non-whitespace characters count
         totalTokenLength++;
       }
     }
diff --git a/lighthouse-core/test/lib/minification-estimator-test.js b/lighthouse-core/test/lib/minification-estimator-test.js
index 6de03e38237f..3a2e9ee43e01 100644
--- a/lighthouse-core/test/lib/minification-estimator-test.js
+++ b/lighthouse-core/test/lib/minification-estimator-test.js
@@ -1,6 +1,8 @@
 const assert = require('assert');
 const {computeCSSTokenLength, computeJSTokenLength} = require('../../lib/minification-estimator');
 
+/* eslint-env jest */
+
 describe('minification estimator', () => {
   describe('CSS', () => {
     it('should compute length of meaningful content', () => {
@@ -106,6 +108,74 @@ describe('minification estimator', () => {
   });
 
   describe('JS', () => {
+    it('should compute the length of tokens', () => {
+      const js = `
+        const foo = 1;
+        const bar = 2;
+        console.log(foo + bar);
+      `;
+
+      const tokensOnly = 'constfoo=1;constbar=2;console.log(foo+bar);';
+      assert.equal(computeJSTokenLength(js), tokensOnly.length);
+    });
+
+    it('should handle single-line comments', () => {
+      const js = `
+        // ignore me
+        12345
+      `;
+
+      assert.equal(computeJSTokenLength(js), 5);
+    });
+
+    it('should handle multi-line comments', () => {
+      const js = `
+        /* ignore
+         * me
+         * too
+         */
+        12345
+      `;
+
+      assert.equal(computeJSTokenLength(js), 5);
+    });
+
+    it('should handle strings', () => {
+      const pairs = [
+        [`'//123' // ignored`, 7], // single quotes
+        [`"//123" // ignored`, 7], // double quotes
+        [`'     ' // ignored`, 7], // whitespace in strings count
+        [`"\\" // not ignored"`, 19], // escaped quotes handled
+      ];
+
+      for (const [js, len] of pairs) {
+        assert.equal(computeJSTokenLength(js), len, `expected '${js}' to have token length ${len}`);
+      }
+    });
+
+    it('should handle template literals', () => {
+      const js = `
+        \`/* don't ignore this */\` // 25 characters
+        12345
+      `;
+
+      assert.equal(computeJSTokenLength(js), 25 + 5);
+    });
 
+    it('should handle regular expressions', () => {
+      const js = `
+        /regex '/ // this should be in comment not string 123456789
+      `;
+
+      assert.equal(computeJSTokenLength(js), 9);
+    });
+
+    it('should distinguish regex from divide', () => {
+      const js = `
+        return 1 / 2 // hello
+      `;
+
+      assert.equal(computeJSTokenLength(js), 9);
+    });
   });
 })

From 3fedd1fb9d9bc05a565b95513ee36f1ccd2b80b1 Mon Sep 17 00:00:00 2001
From: Patrick Hulce <patrick.hulce@gmail.com>
Date: Fri, 21 Sep 2018 17:24:52 -0500
Subject: [PATCH 5/5] more cleanup

---
 lighthouse-core/lib/minification-estimator.js | 11 ++++----
 .../unminified-javascript-test.js             | 25 +++++++++----------
 .../test/lib/minification-estimator-test.js   |  9 ++++++-
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/lighthouse-core/lib/minification-estimator.js b/lighthouse-core/lib/minification-estimator.js
index 007c7d62c978..88686e1ca802 100644
--- a/lighthouse-core/lib/minification-estimator.js
+++ b/lighthouse-core/lib/minification-estimator.js
@@ -6,8 +6,9 @@
 'use strict';
 
 // https://www.ecma-international.org/ecma-262/9.0/index.html#sec-punctuators
-const PUNCTUATOR_REGEX = /(return|{|\(|\[|\.\.\.|;|,|<|>|<=|>=|==|!=|===|!==|\+|-|\*|%|\*\*|\+\+|--|<<|>>|>>>|&|\||\^|!|~|&&|\|\||\?|:|=|\+=|-=|\*=|%=|\*\*=|<<=|>>=|>>>=|&=|\|=|\^=|=>|\/|\/=|\})$/
-const WHITESPACE_REGEX = /( |\n|\t)+$/
+// eslint-disable-next-line max-len
+const PUNCTUATOR_REGEX = /(return|{|\(|\[|\.\.\.|;|,|<|>|<=|>=|==|!=|===|!==|\+|-|\*|%|\*\*|\+\+|--|<<|>>|>>>|&|\||\^|!|~|&&|\|\||\?|:|=|\+=|-=|\*=|%=|\*\*=|<<=|>>=|>>>=|&=|\|=|\^=|=>|\/|\/=|\})$/;
+const WHITESPACE_REGEX = /( |\n|\t)+$/;
 
 /**
  * Look backwards from `startPosition` in `content` for an ECMAScript punctuator.
@@ -148,14 +149,14 @@ function computeTokenLength(content, features) {
  * @param {string} content
  */
 function computeJSTokenLength(content) {
-  return computeTokenLength(content, {singlelineComments: true, regex: true})
+  return computeTokenLength(content, {singlelineComments: true, regex: true});
 }
 
 /**
  * @param {string} content
  */
 function computeCSSTokenLength(content) {
-  return computeTokenLength(content, {singlelineComments: false, regex: false})
+  return computeTokenLength(content, {singlelineComments: false, regex: false});
 }
 
-module.exports = {computeJSTokenLength, computeCSSTokenLength}
+module.exports = {computeJSTokenLength, computeCSSTokenLength};
diff --git a/lighthouse-core/test/audits/byte-efficiency/unminified-javascript-test.js b/lighthouse-core/test/audits/byte-efficiency/unminified-javascript-test.js
index c11662888099..cd285fd9ab34 100644
--- a/lighthouse-core/test/audits/byte-efficiency/unminified-javascript-test.js
+++ b/lighthouse-core/test/audits/byte-efficiency/unminified-javascript-test.js
@@ -43,7 +43,7 @@ describe('Page uses optimized responses', () => {
             const foo = 1
             /Edge\/\d*\.\d*/.exec('foo')
           `,
-        '123.4': '#$*% non sense',
+        '123.4': '#$*%dense',
       },
     }, [
       {requestId: '123.1', url: 'foo.js', transferSize: 20 * KB, resourceType},
@@ -52,17 +52,16 @@ describe('Page uses optimized responses', () => {
       {requestId: '123.4', url: 'invalid.js', transferSize: 100 * KB, resourceType},
     ]);
 
-    assert.ok(auditResult.warnings.length);
-    assert.equal(auditResult.items.length, 3);
-    assert.equal(auditResult.items[0].url, 'foo.js');
-    assert.equal(Math.round(auditResult.items[0].wastedPercent), 57);
-    assert.equal(Math.round(auditResult.items[0].wastedBytes / 1024), 11);
-    assert.equal(auditResult.items[1].url, 'other.js');
-    assert.equal(Math.round(auditResult.items[1].wastedPercent), 53);
-    assert.equal(Math.round(auditResult.items[1].wastedBytes / 1024), 27);
-    assert.equal(auditResult.items[2].url, 'valid-ish.js');
-    assert.equal(Math.round(auditResult.items[2].wastedPercent), 72);
-    assert.equal(Math.round(auditResult.items[2].wastedBytes / 1024), 72);
+    const results = auditResult.items.map(item => Object.assign(item, {
+      wastedKB: Math.round(item.wastedBytes / 1024),
+      wastedPercent: Math.round(item.wastedPercent),
+    }));
+
+    expect(results).toMatchObject([
+      {url: 'foo.js', wastedPercent: 57, wastedKB: 11},
+      {url: 'other.js', wastedPercent: 53, wastedKB: 27},
+      {url: 'valid-ish.js', wastedPercent: 39, wastedKB: 39},
+    ]);
   });
 
   it('passes when scripts are already minified', () => {
@@ -85,7 +84,7 @@ describe('Page uses optimized responses', () => {
       },
     }, [
       {requestId: '123.1', url: 'foo.js', transferSize: 20 * KB, resourceType},
-      {requestId: '123.2', url: 'other.js', transferSize: 3 * KB, resourceType},
+      {requestId: '123.2', url: 'other.js', transferSize: 3 * KB, resourceType}, // too small
       {requestId: '123.3', url: 'invalid.js', transferSize: 20 * KB, resourceType},
     ]);
 
diff --git a/lighthouse-core/test/lib/minification-estimator-test.js b/lighthouse-core/test/lib/minification-estimator-test.js
index 3a2e9ee43e01..07cbf7b98bca 100644
--- a/lighthouse-core/test/lib/minification-estimator-test.js
+++ b/lighthouse-core/test/lib/minification-estimator-test.js
@@ -1,3 +1,10 @@
+/**
+ * @license Copyright 2018 Google Inc. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+'use strict';
+
 const assert = require('assert');
 const {computeCSSTokenLength, computeJSTokenLength} = require('../../lib/minification-estimator');
 
@@ -178,4 +185,4 @@ describe('minification estimator', () => {
       assert.equal(computeJSTokenLength(js), 9);
     });
   });
-})
+});