initial closure type checking for report v2

GoogleChrome · Apr 20, 2017 · 4826cf8 · 4826cf8
1 parent bbe7f3b
commit 4826cf8
Show file tree

Hide file tree

Showing 10 changed files with 165 additions and 174 deletions.
diff --git a/lighthouse-core/closure/closure-type-checking.js b/lighthouse-core/closure/closure-type-checking.js
@@ -22,67 +22,78 @@ const gulp = require('gulp');
 const gutil = require('gulp-util');
 const replace = require('gulp-replace');
 
+// Flags to generate additional debug information.
+const PRINT_AST = false;
+const PRINT_CODE = !PRINT_AST && false;
+const OUTPUT_FILE = PRINT_AST ? '../closure-tree.txt' : 'closure-output.js';
+
 /* eslint-disable camelcase */
-gulp.task('js-compile', function() {
+gulp.task('compile-report', () => {
   return gulp.src([
-    'closure/typedefs/*.js',
-    'closure/third_party/*.js',
-    'audits/**/*.js',
-    'lib/event-helpers.js',
-    'lib/icons.js',
-    'lib/styles-helpers.js',
-    'lib/url-shim.js',
-    'aggregator/**/*.js'
+    // externs
+    'closure/third_party/commonjs.js',
+
+    'report/v2/renderer/details-renderer.js',
+    'report/v2/renderer/dom.js',
+    'report/v2/renderer/report-renderer.js',
   ])
-    // Hack to remove `require`s that Closure currently can't resolve.
-    .pipe(replace('require(\'../lib/web-inspector\').Color.parse;',
-        'WebInspector.Color.parse;'))
-    .pipe(replace('require(\'../lib/traces/tracing-processor\');', '/** @type {?} */ (null);'))
-    .pipe(replace('require(\'../lib/traces/devtools-timeline-model\');',
-        'DevtoolsTimelineModel'))
-    .pipe(replace('require(\'speedline\');', 'function(arg) {};'))
-    .pipe(replace(/require\('(\.\.\/)*report\/formatter'\);/g, '{};'))
 
-    // Replace any non-local import (e.g. not starting with .) with a dummy type. These are likely
-    // the built-in Node modules. But not always, so TODO(samthor): Fix this.
-    .pipe(replace(/require\(\'[^\.].*?\'\)/g, '/** @type {*} */ ({})'))
+  // Ignore `module.exports` and `self.ClassName = ClassName` statements.
+  .pipe(replace(/^\s\smodule\.exports = \w+;$/gm, ';'))
+  .pipe(replace(/^\s\sself\.(\w+) = \1;$/gm, ';'))
+
+  .pipe(closureCompiler({
+    compilation_level: 'SIMPLE',
+    // new_type_inf: true,
+    language_in: 'ECMASCRIPT6_STRICT',
+    language_out: 'ECMASCRIPT5_STRICT',
+    warning_level: process.env.CI ? 'QUIET' : 'VERBOSE',
+    jscomp_error: [
+      'checkTypes',
+    ],
+    jscomp_warning: [
+      // https://github.com/google/closure-compiler/wiki/Warnings
+      'accessControls',
+      'checkRegExp',
+      'const',
+      'reportUnknownTypes',
+      'missingProperties',
+      'missingReturn',
+      'strictModuleDepCheck',
+      'typeInvalidation',
+      'undefinedNames',
+      'visibility',
 
-    .pipe(closureCompiler({
-      compilation_level: 'SIMPLE',
-      process_common_js_modules: true,
-      new_type_inf: true,
-      checks_only: true,
-      language_in: 'ECMASCRIPT6_STRICT',
-      language_out: 'ECMASCRIPT5_STRICT',
-      warning_level: process.env.CI ? 'QUIET' : 'VERBOSE',
-      jscomp_error: [
-        'checkTypes',
-        'conformanceViolations'
-      ],
-      jscomp_warning: [
-        // https://github.com/google/closure-compiler/wiki/Warnings
-        'accessControls',
-        'checkRegExp',
-        'const',
-        // 'reportUnknownTypes',
-        'missingProperties',
-        'missingReturn',
-        'newCheckTypes',
-        'strictModuleDepCheck',
-        'typeInvalidation',
-        'undefinedNames',
-        'visibility'
-      ],
-      conformance_configs: 'closure/conformance_config.textproto'
-    }))
-    .on('error', error => {
-      gutil.log('Closure compilation failed. Check `closure-error.log` for details.');
-      require('fs').writeFileSync('closure-error.log', error);
-    })
-    .on('end', () => {
-      gutil.log('Closure compilation successful.');
-    });
+      'checkDebuggerStatement',
+      'externsValidation',
+      'uselessCode',
+      'ambiguousFunctionDecl',
+      'checkTypes',
+      'es3',
+      'es5Strict',
+      'globalThis',
+      'nonStandardJsDocs',
+      'suspiciousCode',
+      'unknownDefines',
+
+      // nullable/undefined checker when new_type_inf enabled.
+      'newCheckTypesAllChecks',
+    ],
+    conformance_configs: 'closure/conformance_config.textproto',
+
+    // Debug output control.
+    checks_only: !PRINT_CODE,
+    print_tree: PRINT_AST,
+    js_output_file: OUTPUT_FILE,
+    formatting: 'PRETTY_PRINT',
+    preserve_type_annotations: true,
+  }))
+  .pipe(gulp.dest('../'))
+  .on('end', () => {
+    gutil.log('Closure compilation successful.');
+  });
 });
+
 /* eslint-enable */
 
-gulp.start('js-compile');
+gulp.start('compile-report');
diff --git a/lighthouse-core/closure/conformance_config.textproto b/lighthouse-core/closure/conformance_config.textproto
@@ -15,19 +15,19 @@
 # This file defines the additional JS conformance tests run over lighthouse code
 # when compiled with the Closure Compiler. For more, see https://github.com/google/closure-compiler/wiki/JS-Conformance-Framework
 
+# Better covered by newCheckTypesAllChecks under NTI
+# requirement: {
+#   type: CUSTOM
+#   java_class: 'com.google.javascript.jscomp.ConformanceRules$BanNullDeref'
+#   error_message: 'Dereferencing `null` or `undefined` is usually an error. See https://github.com/google/closure-compiler/wiki/JS-Conformance-Framework#bannullderef'
+# }
+
 requirement: {
   type: CUSTOM
-  java_class: 'com.google.javascript.jscomp.ConformanceRules$BanNullDeref'
-  error_message: 'Dereferencing `null` or `undefined` is usually an error. See https://github.com/google/closure-compiler/wiki/JS-Conformance-Framework#bannullderef'
+  java_class: 'com.google.javascript.jscomp.ConformanceRules$BanUnknownThis'
+  error_message: 'References to `this` that are typed as `unknown` are not allowed. See https://github.com/google/closure-compiler/wiki/JS-Conformance-Framework#banunknownthis'
 }
 
-# TODO(bckenny): fix unknown this error(s)
-# requirement: {
-#  type: CUSTOM
-#  java_class: 'com.google.javascript.jscomp.ConformanceRules$BanUnknownThis'
-#  error_message: 'References to `this` that are typed as `unknown` are not allowed. See https://github.com/google/closure-compiler/wiki/JS-Conformance-Framework#banunknownthis'
-# }
-
 requirement: {
   type: CUSTOM
   java_class: 'com.google.javascript.jscomp.ConformanceRules$BanUnknownTypedClassPropsReferences'

diff --git a/lighthouse-core/closure/third_party/commonjs.js b/lighthouse-core/closure/third_party/commonjs.js
@@ -0,0 +1,25 @@
+/**
+ * Copyright 2017 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.
+ */
+
+/**
+ * @externs
+ */
+
+/** @const */
+const module = {};
+
+/** @type {*} */
+module.exports;
diff --git a/lighthouse-core/report/v2/renderer/details-renderer.js b/lighthouse-core/report/v2/renderer/details-renderer.js
@@ -22,23 +22,22 @@ class DetailsRenderer {
    * @param {!DOM} dom
    */
   constructor(dom) {
+    /** @private {!DOM} */
     this._dom = dom;
   }
 
   /**
-   * @param {(!DetailsRenderer.DetailsJSON|!DetailsRenderer.CardsDetailsJSON)} details
+   * @param {!DetailsRenderer.DetailsJSON} details
    * @return {!Element}
    */
   render(details) {
     switch (details.type) {
       case 'text':
         return this._renderText(details);
-      case 'block':
-        return this._renderBlock(details);
       case 'cards':
         return this._renderCards(/** @type {!DetailsRenderer.CardsDetailsJSON} */ (details));
       case 'list':
-        return this._renderList(details);
+        return this._renderList(/** @type {!DetailsRenderer.ListDetailsJSON} */ (details));
       default:
         throw new Error(`Unknown type: ${details.type}`);
     }
@@ -55,20 +54,7 @@ class DetailsRenderer {
   }
 
   /**
-   * @param {!DetailsRenderer.DetailsJSON} block
-   * @return {!Element}
-   */
-  _renderBlock(block) {
-    const element = this._dom.createElement('div', 'lh-block');
-    const items = block.items || [];
-    for (const item of items) {
-      element.appendChild(this.render(item));
-    }
-    return element;
-  }
-
-  /**
-   * @param {!DetailsRenderer.DetailsJSON} list
+   * @param {!DetailsRenderer.ListDetailsJSON} list
    * @return {!Element}
    */
   _renderList(list) {
@@ -80,8 +66,7 @@ class DetailsRenderer {
     }
 
     const itemsElem = this._dom.createElement('div', 'lh-list__items');
-    const items = list.items || [];
-    for (const item of items) {
+    for (const item of list.items) {
       itemsElem.appendChild(this.render(item));
     }
     element.appendChild(itemsElem);
@@ -128,17 +113,23 @@ if (typeof module !== 'undefined' && module.exports) {
 /**
  * @typedef {{
  *     type: string,
- *     text: (string|undefined),
- *     header: (!DetailsRenderer.DetailsJSON|undefined),
- *     items: (!Array<!DetailsRenderer.DetailsJSON>|undefined)
+ *     text: (string|undefined)
  * }}
  */
 DetailsRenderer.DetailsJSON; // eslint-disable-line no-unused-expressions
 
+/**
+ * @typedef {{
+ *     type: string,
+ *     header: ({text: string}|undefined),
+ *     items: !Array<{type: string, text: (string|undefined)}>
+ * }}
+ */
+DetailsRenderer.ListDetailsJSON; // eslint-disable-line no-unused-expressions
+
 /** @typedef {{
  *     type: string,
- *     text: string,
- *     header: !DetailsRenderer.DetailsJSON,
+ *     header: ({text: string}|undefined),
  *     items: !Array<{title: string, value: string, snippet: (string|undefined), target: string}>
  * }}
  */

diff --git a/lighthouse-core/report/v2/renderer/dom.js b/lighthouse-core/report/v2/renderer/dom.js
@@ -22,6 +22,7 @@ class DOM {
    * @param {!Document} document
    */
   constructor(document) {
+    /** @private {!Document} */
     this._document = document;
   }
 
@@ -33,9 +34,7 @@ class DOM {
    *     set the attribute on the node.
    * @return {!Element}
    */
-  createElement(name, className, attrs) {
-    // TODO(all): adopt `attrs` default arg when https://codereview.chromium.org/2821773002/ lands
-    attrs = attrs || {};
+  createElement(name, className, attrs = {}) {
     const element = this._document.createElement(name);
     if (className) {
       element.className = className;
@@ -56,7 +55,7 @@ class DOM {
    * @throws {Error}
    */
   cloneTemplate(selector, context) {
-    const template = context.querySelector(selector);
+    const template = /** @type {HTMLTemplateElement} */ (context.querySelector(selector));
     if (!template) {
       throw new Error(`Template not found: template${selector}`);
     }
@@ -80,7 +79,7 @@ class DOM {
 
       // Append link if there are any.
       if (linkText && linkHref) {
-        const a = this.createElement('a');
+        const a = /** @type {!HTMLAnchorElement} */ (this.createElement('a'));
         a.rel = 'noopener';
         a.target = '_blank';
         a.textContent = linkText;
@@ -98,6 +97,21 @@ class DOM {
   document() {
     return this._document;
   }
+
+  /**
+   * Guaranteed context.querySelector. Always returns an element or throws if
+   * nothing matches query.
+   * @param {!DocumentFragment|!Element} context
+   * @param {string} query
+   * @return {!Element}
+   */
+  static find(context, query) {
+    const result = context.querySelector(query);
+    if (result === null) {
+      throw new Error(`query ${query} not found`);
+    }
+    return result;
+  }
 }
 
 if (typeof module !== 'undefined' && module.exports) {