Fix all type errors in v0.js and turn on type checking on travis (amp…

…project#4908)

build-system/amp.extern.js

@@ -104,6 +104,7 @@ var AmpElement;
 var AccessService = function() {};
 /** @constructor */
 var UserNotificationManager = function() {};
+UserNotificationManager.prototype.get;
 /** @constructor */
 var Cid = function() {};
 /** @constructor */
@@ -136,3 +137,48 @@ FB.init;
 var amp;
 amp.validator;
 amp.validator.validateUrlAndLog = function(string, doc, filter) {}
+
+// Temporary Access types (delete when amp-access is compiled
+// for type checking).
+var Activity;
+Activity.prototype.getTotalEngagedTime = function() {};
+var AccessService;
+AccessService.prototype.getAccessReaderId = function() {};
+AccessService.prototype.getAuthdataField = function(field) {};
+// Same for amp-analytics
+/**
+ * The "get CID" parameters.
+ * - createCookieIfNotPresent: Whether CID is allowed to create a cookie when.
+ *   Default value is `false`.
+ * @typedef {{
+ *   scope: string,
+ *   createCookieIfNotPresent: (boolean|undefined),
+ * }}
+ */
+var GetCidDef;
+var Cid;
+/**
+ * @param {string|!GetCidDef} externalCidScope Name of the fallback cookie
+ *     for the case where this doc is not served by an AMP proxy. GetCidDef
+ *     structure can also instruct CID to create a cookie if one doesn't yet
+ *     exist in a non-proxy case.
+ * @param {!Promise} consent Promise for when the user has given consent
+ *     (if deemed necessary by the publisher) for use of the client
+ *     identifier.
+ * @param {!Promise=} opt_persistenceConsent Dedicated promise for when
+ *     it is OK to persist a new tracking identifier. This could be
+ *     supplied ONLY by the code that supplies the actual consent
+ *     cookie.
+ *     If this is given, the consent param should be a resolved promise
+ *     because this call should be only made in order to get consent.
+ *     The consent promise passed to other calls should then itself
+ *     depend on the opt_persistenceConsent promise (and the actual
+ *     consent, of course).
+ * @return {!Promise<?string>} A client identifier that should be used
+ *      within the current source origin and externalCidScope. Might be
+ *      null if no identifier was found or could be made.
+ *      This promise may take a long time to resolve if consent isn't
+ *      given.
+ */
+Cid.prototype.get = function(
+    externalCidScope, consent, opt_persistenceConsent) {}

build-system/runner/src/org/ampproject/AmpCodingConvention.java

@@ -47,7 +47,9 @@ public AmpCodingConvention(CodingConvention convention) {
         new AssertionFunctionSpec("user.assert", JSType.TRUTHY),
         new AssertionFunctionSpec("dev.assert", JSType.TRUTHY),
         new AssertionFunctionSpec("Log$$module$src$log.prototype.assert", JSType.TRUTHY),
-        new AssertFunctionByTypeName("Log$$module$src$log.prototype.assertElement", "Element")
+        new AssertFunctionByTypeName("Log$$module$src$log.prototype.assertElement", "Element"),
+        new AssertFunctionByTypeName("Log$$module$src$log.prototype.assertString", "string"),
+        new AssertFunctionByTypeName("Log$$module$src$log.prototype.assertNumber", "number")
     );
   }
 

build-system/runner/src/org/ampproject/AmpCommandLineRunner.java

@@ -48,8 +48,8 @@ public class AmpCommandLineRunner extends CommandLineRunner {
    */
   ImmutableSet<String> suffixTypes = ImmutableSet.of(
       "dev.fine");
-  
-  
+
+
   ImmutableMap<String, Node> assignmentReplacements = ImmutableMap.of(
       "IS_DEV",
       IR.falseNode());
@@ -98,6 +98,7 @@ protected AmpCommandLineRunner(String[] args) {
   protected CompilerOptions createTypeCheckingOptions() {
     CompilerOptions options = super.createOptions();
     options.setCheckTypes(true);
+    options.setInferTypes(true);
     return options;
   }
 

build-system/tasks/compile.js

@@ -224,6 +224,7 @@ function compile(entryModuleFilename, outputDir,
           '3p/environment.js',
           'src/document-state.js',
         ],
+        jscomp_error: [],
       }
     };
 
@@ -232,7 +233,8 @@ function compile(entryModuleFilename, outputDir,
       // Don't modify compilation_level to a lower level since
       // it won't do strict type checking if its whitespace only.
       compilerOptions.compilerFlags.define.push('TYPECHECK_ONLY=true');
-      compilerOptions.compilerFlags.jscomp_error = 'checkTypes';
+      compilerOptions.compilerFlags.jscomp_error.push(
+          'checkTypes', 'accessControls', 'const', 'constantProperty');
     }
     if (argv.pseudo_names) {
       compilerOptions.compilerFlags.define.push('PSEUDO_NAMES=true');

build-system/tasks/presubmit-checks.js

@@ -340,13 +340,15 @@ var forbiddenTerms = {
   'getAccessReaderId': {
     message: requiresReviewPrivacy,
     whitelist: [
+      'build-system/amp.extern.js',
       'extensions/amp-access/0.1/amp-access.js',
       'src/service/url-replacements-impl.js',
     ]
   },
   'getAuthdataField': {
     message: requiresReviewPrivacy,
     whitelist: [
+      'build-system/amp.extern.js',
       'extensions/amp-access/0.1/amp-access.js',
       'src/service/url-replacements-impl.js',
     ]

builtins/amp-video.js

@@ -20,6 +20,7 @@ import {isLayoutSizeDefined} from '../src/layout';
 import {loadPromise} from '../src/event-helper';
 import {registerElement} from '../src/custom-element';
 import {getMode} from '../src/mode';
+import {dev} from '../src/log';
 
 /**
  * @param {!Window} win Destination window for the new element.
@@ -36,7 +37,7 @@ export function installVideo(win) {
 
     /** @override */
     buildCallback() {
-      /** @private @const {!HTMLVideoElement} */
+      /** @private @const {!Element} */
       this.video_ = this.element.ownerDocument.createElement('video');
 
       const posterAttr = this.element.getAttribute('poster');
@@ -82,7 +83,8 @@ export function installVideo(win) {
           return;
         }
         if (child.getAttribute && child.getAttribute('src')) {
-          assertHttpsUrl(child.getAttribute('src'), child);
+          assertHttpsUrl(child.getAttribute('src'),
+              dev().assertElement(child));
         }
         this.video_.appendChild(child);
       });

extensions/amp-apester-media/0.1/test/test-amp-apester-media.js

@@ -72,6 +72,7 @@ describe('amp-apester-media', () => {
         media.setAttribute(key, attributes[key]);
 
       }
+      media.setAttribute('width', '600');
       media.setAttribute('height', '390');
       //todo test width?
       if (opt_responsive) {

gulpfile.js

@@ -138,11 +138,7 @@ function compile(watch, shouldMinify, opt_preventRemoveAndMakeDir,
     preventRemoveAndMakeDir: opt_preventRemoveAndMakeDir,
     externs: ['ads/ads.extern.js',],
   });
-  // The main binary does not yet compile successfully with type checking
-  // turned on. Skip for now.
-  if (opt_checkTypes && !argv.more) {
-    return;
-  }
+
   // For compilation with babel we start with the amp-babel entry point,
   // but then rename to the amp.js which we've been using all along.
   compileJs('./src/', 'amp-babel.js', './dist', {
@@ -163,6 +159,10 @@ function compile(watch, shouldMinify, opt_preventRemoveAndMakeDir,
         's.animation="none";' +
         's.WebkitAnimation="none;"},1000);throw e};'
   });
+  if (opt_checkTypes) {
+    // We don't rerun type check for the shadow entry point for now.
+    return;
+  }
   // Entry point for shadow runtime.
   compileJs('./src/', 'amp-shadow-babel.js', './dist', {
     toName: 'amp-shadow.js',
@@ -344,19 +344,19 @@ function checkTypes() {
     checkTypes: true,
     preventRemoveAndMakeDir: true,
   });
-  buildSw({
+  // Temporarily turned off due to unknown type warnings.
+  /*buildSw({
     minify: true,
     checkTypes: true,
     preventRemoveAndMakeDir: true,
-  });
+  });*/
   buildExperiments({
     minify: true,
     checkTypes: true,
     preventRemoveAndMakeDir: true,
   });
   compile(false, true, /* opt_preventRemoveAndMakeDir*/ true,
       /* check types */ true);
-  // These are not turned on on Travis.
 }
 
 /**

src/access-service.js

@@ -26,7 +26,8 @@ import {
  * @return {!Promise<!AccessService>}
  */
 export function accessServiceFor(win) {
-  return getElementService(win, 'access', 'amp-access');
+  return /** @type {!Promise<!AccessService>} */ (
+        getElementService(win, 'access', 'amp-access'));
 }
 
 /**
@@ -36,5 +37,6 @@ export function accessServiceFor(win) {
  * @return {!Promise<?AccessService>}
  */
 export function accessServiceForOrNull(win) {
-  return getElementServiceIfAvailable(win, 'access', 'amp-access');
+  return /** @type {!Promise<?AccessService>} */ (
+      getElementServiceIfAvailable(win, 'access', 'amp-access'));
 }

src/activity.js

@@ -21,5 +21,6 @@ import {getElementService} from './element-service';
  * @return {!Promise<!Activity>}
  */
 export function activityFor(win) {
-  return getElementService(win, 'activity', 'amp-analytics');
+  return /** @type {!Promise<!Activity>} */ (
+      getElementService(win, 'activity', 'amp-analytics'));
 };

src/animation.js

@@ -73,11 +73,13 @@ export class Animation {
    * Sets the default curve for the animation. Each segment is allowed to have
    * its own curve, but this curve will be used if a segment doesn't specify
    * its own.
-   * @param {!./curve.CurveDef|string} curve
+   * @param {!./curve.CurveDef|string|undefined} curve
    * @return {!Animation}
    */
   setCurve(curve) {
-    this.curve_ = getCurve(curve);
+    if (curve) {
+      this.curve_ = getCurve(curve);
+    }
     return this;
   }
 
@@ -127,7 +129,8 @@ export class Animation {
  * semantics of a Promise and signal when the animation completed or failed.
  * Additionally, it exposes the method "halt" which allows to stop/reset the
  * animation.
- * @implements {IThenable}
+ * // TODO(@cramforce) Actually fully implement.
+ * implements {IThenable}
  */
 class AnimationPlayer {