core(tsc): more type checking of top-level audits (#5089)

GoogleChrome · May 2, 2018 · ed14c8e · ed14c8e
1 parent 9ddae1e
commit ed14c8e
Show file tree

Hide file tree

Showing 17 changed files with 156 additions and 48 deletions.
diff --git a/lighthouse-core/audits/multi-check-audit.js b/lighthouse-core/audits/multi-check-audit.js
@@ -13,18 +13,18 @@ const Audit = require('./audit');
 
 class MultiCheckAudit extends Audit {
   /**
-   * @param {!Artifacts} artifacts
-   * @return {!AuditResult}
+   * @param {LH.Artifacts} artifacts
+   * @return {Promise<LH.Audit.Product>}
    */
   static audit(artifacts) {
-    return Promise.resolve(this.audit_(artifacts)).then(result => this.createAuditResult(result));
+    return Promise.resolve(this.audit_(artifacts)).then(result => this.createAuditProduct(result));
   }
 
   /**
-   * @param {!{failures: !Array<!string>, themeColor: ?string, manifestValues: ?Object, }} result
-   * @return {!AuditResult}
+   * @param {{failures: Array<string>, warnings?: Array<string>, manifestValues?: LH.Artifacts.ManifestValues}} result
+   * @return {LH.Audit.Product}
    */
-  static createAuditResult(result) {
+  static createAuditProduct(result) {
     const extendedInfo = {
       value: result,
     };
@@ -51,12 +51,17 @@ class MultiCheckAudit extends Audit {
     };
   }
 
+  /* eslint-disable no-unused-vars */
+
   /**
-   * @param {!Artifacts} artifacts
+   * @param {LH.Artifacts} artifacts
+   * @return {Promise<{failures: Array<string>, warnings?: Array<string>, manifestValues?: LH.Artifacts.ManifestValues}>}
    */
-  static audit_() {
+  static audit_(artifacts) {
     throw new Error('audit_ unimplemented');
   }
+
+  /* eslint-enable no-unused-vars */
 }
 
 module.exports = MultiCheckAudit;
diff --git a/lighthouse-core/audits/redirects-http.js b/lighthouse-core/audits/redirects-http.js
@@ -9,7 +9,7 @@ const Audit = require('./audit');
 
 class RedirectsHTTP extends Audit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -23,13 +23,12 @@ class RedirectsHTTP extends Audit {
   }
 
   /**
-   * @param {!Artifacts} artifacts
-   * @return {!AuditResult}
+   * @param {LH.Artifacts} artifacts
+   * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
     return {
       rawValue: artifacts.HTTPRedirect.value,
-      debugString: artifacts.HTTPRedirect.debugString,
     };
   }
 }

diff --git a/lighthouse-core/audits/screenshot-thumbnails.js b/lighthouse-core/audits/screenshot-thumbnails.js
@@ -12,9 +12,11 @@ const jpeg = require('jpeg-js');
 const NUMBER_OF_THUMBNAILS = 10;
 const THUMBNAIL_WIDTH = 120;
 
+/** @typedef {LH.Artifacts.Speedline['frames'][0]} SpeedlineFrame */
+
 class ScreenshotThumbnails extends Audit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -30,8 +32,8 @@ class ScreenshotThumbnails extends Audit {
    * Scales down an image to THUMBNAIL_WIDTH using nearest neighbor for speed, maintains aspect
    * ratio of the original thumbnail.
    *
-   * @param {{width: number, height: number, data: !Array<number>}} imageData
-   * @return {{width: number, height: number, data: !Array<number>}}
+   * @param {ReturnType<SpeedlineFrame['getParsedImage']>} imageData
+   * @return {{width: number, height: number, data: Uint8Array}}
    */
   static scaleImageToThumbnail(imageData) {
     const scaledWidth = THUMBNAIL_WIDTH;
@@ -63,11 +65,13 @@ class ScreenshotThumbnails extends Audit {
   }
 
   /**
-   * @param {!Artifacts} artifacts
-   * @return {!AuditResult}
+   * @param {LH.Artifacts} artifacts
+   * @param {LH.Audit.Context} context
+   * @return {Promise<LH.Audit.Product>}
    */
   static async audit(artifacts, context) {
     const trace = artifacts.traces[Audit.DEFAULT_PASS];
+    /** @type {Map<SpeedlineFrame, string>} */
     const cachedThumbnails = new Map();
 
     const speedline = await artifacts.requestSpeedline(trace);
@@ -99,6 +103,8 @@ class ScreenshotThumbnails extends Audit {
     for (let i = 1; i <= NUMBER_OF_THUMBNAILS; i++) {
       const targetTimestamp = speedline.beginning + timelineEnd * i / NUMBER_OF_THUMBNAILS;
 
+      /** @type {SpeedlineFrame} */
+      // @ts-ignore - there will always be at least one frame by this point. TODO: use nonnullable assertion in TS2.9
       let frameForTimestamp = null;
       if (i === NUMBER_OF_THUMBNAILS) {
         frameForTimestamp = analyzedFrames[analyzedFrames.length - 1];

diff --git a/lighthouse-core/audits/service-worker.js b/lighthouse-core/audits/service-worker.js
@@ -10,7 +10,7 @@ const Audit = require('./audit');
 
 class ServiceWorker extends Audit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -25,8 +25,8 @@ class ServiceWorker extends Audit {
   }
 
   /**
-   * @param {!Artifacts} artifacts
-   * @return {!AuditResult}
+   * @param {LH.Artifacts} artifacts
+   * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
     // Find active service worker for this URL. Match against

diff --git a/lighthouse-core/audits/speed-index.js b/lighthouse-core/audits/speed-index.js
@@ -10,7 +10,7 @@ const Util = require('../report/html/renderer/util');
 
 class SpeedIndex extends Audit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -39,9 +39,9 @@ class SpeedIndex extends Audit {
   /**
    * Audits the page to give a score for the Speed Index.
    * @see https://github.com/GoogleChrome/lighthouse/issues/197
-   * @param {Artifacts} artifacts The artifacts from the gather phase.
+   * @param {LH.Artifacts} artifacts The artifacts from the gather phase.
    * @param {LH.Audit.Context} context
-   * @return {Promise<AuditResult>}
+   * @return {Promise<LH.Audit.Product>}
    */
   static async audit(artifacts, context) {
     const trace = artifacts.traces[Audit.DEFAULT_PASS];

diff --git a/lighthouse-core/audits/splash-screen.js b/lighthouse-core/audits/splash-screen.js
@@ -22,7 +22,7 @@ const MultiCheckAudit = require('./multi-check-audit');
 
 class SplashScreen extends MultiCheckAudit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -36,8 +36,12 @@ class SplashScreen extends MultiCheckAudit {
     };
   }
 
+  /**
+   * @param {LH.Artifacts.ManifestValues} manifestValues
+   * @param {Array<string>} failures
+   */
   static assessManifest(manifestValues, failures) {
-    if (manifestValues.isParseFailure) {
+    if (manifestValues.isParseFailure && manifestValues.parseFailureReason) {
       failures.push(manifestValues.parseFailureReason);
       return;
     }
@@ -58,8 +62,12 @@ class SplashScreen extends MultiCheckAudit {
       });
   }
 
-
+  /**
+   * @param {LH.Artifacts} artifacts
+   * @return {Promise<{failures: Array<string>, manifestValues: LH.Artifacts.ManifestValues}>}
+   */
   static audit_(artifacts) {
+    /** @type {Array<string>} */
     const failures = [];
 
     return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => {

diff --git a/lighthouse-core/audits/themed-omnibox.js b/lighthouse-core/audits/themed-omnibox.js
@@ -20,7 +20,7 @@ const validColor = require('../lib/web-inspector').Color.parse;
 
 class ThemedOmnibox extends MultiCheckAudit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -33,6 +33,10 @@ class ThemedOmnibox extends MultiCheckAudit {
     };
   }
 
+  /**
+   * @param {LH.Artifacts['ThemeColor']} themeColorMeta
+   * @param {Array<string>} failures
+   */
   static assessMetaThemecolor(themeColorMeta, failures) {
     if (themeColorMeta === null) {
       failures.push('No `<meta name="theme-color">` tag found');
@@ -41,19 +45,28 @@ class ThemedOmnibox extends MultiCheckAudit {
     }
   }
 
+  /**
+   * @param {LH.Artifacts.ManifestValues} manifestValues
+   * @param {Array<string>} failures
+   */
   static assessManifest(manifestValues, failures) {
-    if (manifestValues.isParseFailure) {
+    if (manifestValues.isParseFailure && manifestValues.parseFailureReason) {
       failures.push(manifestValues.parseFailureReason);
       return;
     }
 
     const themeColorCheck = manifestValues.allChecks.find(i => i.id === 'hasThemeColor');
-    if (!themeColorCheck.passing) {
+    if (themeColorCheck && !themeColorCheck.passing) {
       failures.push(themeColorCheck.failureText);
     }
   }
 
+  /**
+   * @param {LH.Artifacts} artifacts
+   * @return {Promise<{failures: Array<string>, manifestValues: LH.Artifacts.ManifestValues, themeColor: ?string}>}
+   */
   static audit_(artifacts) {
+    /** @type {Array<string>} */
     const failures = [];
 
     return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => {

diff --git a/lighthouse-core/audits/time-to-first-byte.js b/lighthouse-core/audits/time-to-first-byte.js
@@ -12,7 +12,7 @@ const TTFB_THRESHOLD = 600;
 
 class TTFBMetric extends Audit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -25,15 +25,18 @@ class TTFBMetric extends Audit {
     };
   }
 
+  /**
+   * @param {LH.WebInspector.NetworkRequest} record
+   */
   static caclulateTTFB(record) {
     const timing = record._timing;
 
     return timing.receiveHeadersEnd - timing.sendEnd;
   }
 
   /**
-   * @param {!Artifacts} artifacts
-   * @return {!AuditResult}
+   * @param {LH.Artifacts} artifacts
+   * @return {Promise<LH.Audit.Product>}
    */
   static audit(artifacts) {
     const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
@@ -44,6 +47,9 @@ class TTFBMetric extends Audit {
 
         const finalUrl = artifacts.URL.finalUrl;
         const finalUrlRequest = networkRecords.find(record => record._url === finalUrl);
+        if (!finalUrlRequest) {
+          throw new Error(`finalUrl '${finalUrl} not found in network records.`);
+        }
         const ttfb = TTFBMetric.caclulateTTFB(finalUrlRequest);
         const passed = ttfb < TTFB_THRESHOLD;
 

diff --git a/lighthouse-core/audits/viewport.js b/lighthouse-core/audits/viewport.js
@@ -10,7 +10,7 @@ const Parser = require('metaviewport-parser');
 
 class Viewport extends Audit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -25,8 +25,8 @@ class Viewport extends Audit {
   }
 
   /**
-   * @param {!Artifacts} artifacts
-   * @return {!AuditResult}
+   * @param {LH.Artifacts} artifacts
+   * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
     if (artifacts.Viewport === null) {

diff --git a/lighthouse-core/audits/webapp-install-banner.js b/lighthouse-core/audits/webapp-install-banner.js
@@ -30,7 +30,7 @@ const SWAudit = require('./service-worker');
 
 class WebappInstallBanner extends MultiCheckAudit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -44,11 +44,16 @@ class WebappInstallBanner extends MultiCheckAudit {
     };
   }
 
+  /**
+   * @param {LH.Artifacts.ManifestValues} manifestValues
+   * @return {Array<string>}
+   */
   static assessManifest(manifestValues) {
-    if (manifestValues.isParseFailure) {
+    if (manifestValues.isParseFailure && manifestValues.parseFailureReason) {
       return [manifestValues.parseFailureReason];
     }
 
+    /** @type {Array<string>} */
     const failures = [];
     const bannerCheckIds = [
       'hasName',
@@ -68,7 +73,10 @@ class WebappInstallBanner extends MultiCheckAudit {
     return failures;
   }
 
-
+  /**
+   * @param {LH.Artifacts} artifacts
+   * @return {Array<string>}
+   */
   static assessServiceWorker(artifacts) {
     const failures = [];
     const hasServiceWorker = SWAudit.audit(artifacts).rawValue;
@@ -79,6 +87,10 @@ class WebappInstallBanner extends MultiCheckAudit {
     return failures;
   }
 
+  /**
+   * @param {LH.Artifacts} artifacts
+   * @return {{failures: Array<string>, warnings: Array<string>}}
+   */
   static assessOfflineStartUrl(artifacts) {
     const failures = [];
     const warnings = [];
@@ -98,8 +110,14 @@ class WebappInstallBanner extends MultiCheckAudit {
     return {failures, warnings};
   }
 
+  /**
+   * @param {LH.Artifacts} artifacts
+   * @return {Promise<{failures: Array<string>, warnings: Array<string>, manifestValues: LH.Artifacts.ManifestValues}>}
+   */
   static audit_(artifacts) {
+    /** @type {Array<string>} */
     let offlineFailures = [];
+    /** @type {Array<string>} */
     let offlineWarnings = [];
 
     return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => {

diff --git a/lighthouse-core/audits/without-javascript.js b/lighthouse-core/audits/without-javascript.js
@@ -9,7 +9,7 @@ const Audit = require('./audit');
 
 class WithoutJavaScript extends Audit {
   /**
-   * @return {!AuditMeta}
+   * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
@@ -24,8 +24,8 @@ class WithoutJavaScript extends Audit {
   }
 
   /**
-   * @param {!Artifacts} artifacts
-   * @return {!AuditResult}
+   * @param {LH.Artifacts} artifacts
+   * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
     const artifact = artifacts.HTMLWithoutJavaScript;