diff --git a/build-system/amp.extern.js b/build-system/amp.extern.js index 1a8f494b6d9d..7fd09eea9b60 100644 --- a/build-system/amp.extern.js +++ b/build-system/amp.extern.js @@ -131,3 +131,8 @@ twttr.widgets.createTweet; var FB; FB.init; + +// Validator +var amp; +amp.validator; +amp.validator.validateUrlAndLog = function(string, doc, filter) {} diff --git a/build-system/runner/dist/runner.jar b/build-system/runner/dist/runner.jar index 52b6bab9d074..5c24dda0b9da 100644 Binary files a/build-system/runner/dist/runner.jar and b/build-system/runner/dist/runner.jar differ diff --git a/build-system/runner/src/org/ampproject/AmpCodingConvention.java b/build-system/runner/src/org/ampproject/AmpCodingConvention.java index 7bf98898d0dc..0ca1a3e21167 100644 --- a/build-system/runner/src/org/ampproject/AmpCodingConvention.java +++ b/build-system/runner/src/org/ampproject/AmpCodingConvention.java @@ -17,6 +17,7 @@ package org.ampproject; import com.google.common.collect.ImmutableList; +import com.google.javascript.jscomp.ClosureCodingConvention.AssertFunctionByTypeName; import com.google.javascript.jscomp.CodingConvention; import com.google.javascript.jscomp.CodingConvention.AssertionFunctionSpec; import com.google.javascript.jscomp.CodingConventions; @@ -45,9 +46,8 @@ public AmpCodingConvention(CodingConvention convention) { return ImmutableList.of( new AssertionFunctionSpec("user.assert", JSType.TRUTHY), new AssertionFunctionSpec("dev.assert", JSType.TRUTHY), - new AssertionFunctionSpec("module$src$log.user.assert", JSType.TRUTHY), - new AssertionFunctionSpec("module$src$log.dev.assert", JSType.TRUTHY), - new AssertionFunctionSpec("Log$$module$src$log.prototype.assert", JSType.TRUTHY) + new AssertionFunctionSpec("Log$$module$src$log.prototype.assert", JSType.TRUTHY), + new AssertFunctionByTypeName("Log$$module$src$log.prototype.assertElement", "Element") ); } diff --git a/src/event-helper.js b/src/event-helper.js index 3b0403b4ebbf..1354b7331877 100644 --- a/src/event-helper.js +++ b/src/event-helper.js @@ -101,9 +101,10 @@ export function isLoaded(element) { * Returns a promise that will resolve or fail based on the element's 'load' * and 'error' events. Optionally this method takes a timeout, which will reject * the promise if the resource has not loaded by then. - * @param {!Element} element + * @param {T} element * @param {number=} opt_timeout - * @return {!Promise} + * @return {!Promise} + * @template T */ export function loadPromise(element, opt_timeout) { let unlistenLoad; diff --git a/src/json.js b/src/json.js index 1bb77a4513fa..026a295236a5 100644 --- a/src/json.js +++ b/src/json.js @@ -105,7 +105,7 @@ export function getValueForExpr(obj, expr) { * Parses the given `json` string without throwing an exception if not valid. * Returns `undefined` if parsing fails. * Returns the `Object` corresponding to the JSON string when parsing succeeds. - * @param {string} json JSON string to parse + * @param {*} json JSON string to parse * @param {function(!Error)=} opt_onFailed Optional function that will be called with * the error if parsing fails. * @return {?JSONValueDef|undefined} diff --git a/src/log.js b/src/log.js index d4ca7e615c24..9978094af7e3 100644 --- a/src/log.js +++ b/src/log.js @@ -249,7 +249,24 @@ export class Log { } return shouldBeTrueish; } + + /** + * Throws an error if the first argument isn't an Element + * + * Otherwise see `assert` for usage + * + * @param {*} shouldBeElement + * @param {string=} opt_message The assertion message + * @return {!Element} The value of shouldBeTrueish. + * @template T + */ /*eslint "google-camelcase/google-camelcase": 2*/ + assertElement(shouldBeElement, opt_message) { + const shouldBeTrueish = shouldBeElement && shouldBeElement.nodeType == 1; + this.assert(shouldBeTrueish, (opt_message || 'Element expected') + ': %s', + shouldBeElement); + return /** @type {!Element} */ (shouldBeElement); + } /** * Asserts and returns the enum value. If the enum doesn't contain such a value, @@ -261,6 +278,7 @@ export class Log { * @return T * @template T */ + /*eslint "google-camelcase/google-camelcase": 2*/ assertEnumValue(enumObj, s, opt_enumName) { for (const k in enumObj) { if (enumObj[k] == s) { diff --git a/src/performance.js b/src/performance.js index e0a4854f85a1..abe97203b3a0 100644 --- a/src/performance.js +++ b/src/performance.js @@ -18,8 +18,9 @@ import {getExistingServiceForWindow} from './service'; /** * @param {!Window} window - * @return {!Performance} + * @return {!./service/performance-impl.Performance} */ export function performanceFor(window) { - return getExistingServiceForWindow(window, 'performance'); + return /** @type {!./service/performance-impl.Performance}*/ ( + getExistingServiceForWindow(window, 'performance')); }; diff --git a/src/service/url-replacements-impl.js b/src/service/url-replacements-impl.js index 20a9798e23dc..852a8b243d35 100644 --- a/src/service/url-replacements-impl.js +++ b/src/service/url-replacements-impl.js @@ -50,7 +50,7 @@ export class UrlReplacements { /** @private {!RegExp|undefined} */ this.replacementExpr_ = undefined; - /** @private @const {!Object} */ + /** @private @const {!Object} */ this.replacements_ = this.win_.Object.create(null); /** @private @const {function():!Promise} */ @@ -426,7 +426,7 @@ export class UrlReplacements { * @template T */ getAccessValue_(getter, expr) { - return this.getAccessService_(this.win_).then(accessService => { + return this.getAccessService_().then(accessService => { if (!accessService) { // Access service is not installed. user().error(TAG, 'Access service is not installed to access: ', expr); @@ -441,8 +441,8 @@ export class UrlReplacements { * The data for the timing events is retrieved from performance.timing API. * If start and end events are both given, the result is the difference between the two. * If only start event is given, the result is the timing value at start event. - * @param {string} startEvent - * @param {string=} endEvent + * @param {*} startEvent + * @param {*=} endEvent * @return {!Promise} * @private */ @@ -472,14 +472,15 @@ export class UrlReplacements { : String(metric); }); } else { - return Promise.resolve(String(metric)); + return /** @type {!Promise<(string|undefined)>} */ ( + Promise.resolve(String(metric))); } } /** * Returns navigation information from the current browsing context. * @param {string} attribute - * @return {!Promise} + * @return {!Promise|string} * @private */ getNavigationData_(attribute) { @@ -497,7 +498,7 @@ export class UrlReplacements { * Sets the value resolver for the variable with the specified name. The * value resolver may optionally take an extra parameter. * @param {string} varName - * @param {function(*):*} resolver + * @param {function(*, *):*} resolver * @return {!UrlReplacements} * @private */ @@ -604,7 +605,7 @@ export class UrlReplacements { /** * Method exists to assist stubbing in tests. * @param {string} name - * @return {function(*):*} + * @return {function(*, *):*} */ getReplacement_(name) { return this.replacements_[name]; @@ -620,7 +621,7 @@ export class UrlReplacements { if (additionalKeys && additionalKeys.length > 0) { const allKeys = Object.keys(this.replacements_); additionalKeys.forEach(key => { - if (allKeys[key] === undefined) { + if (this.replacements_[key] === undefined) { allKeys.push(key); } }); diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index 3fb64475e145..4bdc20438d6c 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -116,7 +116,7 @@ export class Viewer { /** @private {boolean} */ this.overtakeHistory_ = false; - /** @private {string} */ + /** @private {!VisibilityState} */ this.visibilityState_ = VisibilityState.VISIBLE; /** @private {string} */ @@ -164,6 +164,15 @@ export class Viewer { /** @private {?time} */ this.firstVisibleTime_ = null; + /** @private {?Function} */ + this.messagingReadyResolver_ = null; + + /** @private {?Function} */ + this.viewerOriginResolver_ = null; + + /** @private {?Function} */ + this.trustedViewerResolver_ = null; + /** * This promise might be resolved right away if the current * document is already visible. See end of this constructor where we call @@ -188,8 +197,8 @@ export class Viewer { this.isRuntimeOn_ = !parseInt(this.params_['off'], 10); dev().fine(TAG_, '- runtimeOn:', this.isRuntimeOn_); - this.overtakeHistory_ = !!(parseInt(this.params_['history'], 10)) || - this.overtakeHistory_; + this.overtakeHistory_ = !!(parseInt(this.params_['history'], 10) || + this.overtakeHistory_); dev().fine(TAG_, '- history:', this.overtakeHistory_); this.setVisibilityState_(this.params_['visibilityState']); @@ -237,9 +246,6 @@ export class Viewer { // Wait for document to become visible. this.docState_.onVisibilityChanged(this.recheckVisibilityState_.bind(this)); - /** @private {function()|undefined} */ - this.messagingReadyResolver_ = undefined; - /** * This promise will resolve when communications channel has been * established or timeout in 20 seconds. The timeout is needed to avoid @@ -287,9 +293,6 @@ export class Viewer { this.isTrustedViewerOrigin_(this.win.location.ancestorOrigins[0])); trustedViewerPromise = Promise.resolve(trustedViewerResolved); } else { - - /** @private {!function(boolean)|undefined} */ - this.trustedViewerResolver_ = undefined; // Wait for comms channel to confirm the origin. trustedViewerResolved = undefined; trustedViewerPromise = new Promise(resolve => { @@ -300,9 +303,6 @@ export class Viewer { /** @const @private {!Promise} */ this.isTrustedViewer_ = trustedViewerPromise; - /** @private {!function(string)|undefined} */ - this.viewerOriginResolver_ = undefined; - /** @const @private {!Promise} */ this.viewerOrigin_ = new Promise(resolve => { if (!this.isEmbedded()) { @@ -775,7 +775,8 @@ export class Viewer { * @return {!Promise} */ requestFullOverlay() { - return this.sendMessageUnreliable_('requestFullOverlay', {}, true); + return /** @type {!Promise} */ ( + this.sendMessageUnreliable_('requestFullOverlay', {}, true)); } /** @@ -784,7 +785,8 @@ export class Viewer { * @return {!Promise} */ cancelFullOverlay() { - return this.sendMessageUnreliable_('cancelFullOverlay', {}, true); + return /** @type {!Promise} */ ( + this.sendMessageUnreliable_('cancelFullOverlay', {}, true)); } /** @@ -793,8 +795,8 @@ export class Viewer { * @return {!Promise} */ postPushHistory(stackIndex) { - return this.sendMessageUnreliable_( - 'pushHistory', {stackIndex}, true); + return /** @type {!Promise} */ (this.sendMessageUnreliable_( + 'pushHistory', {stackIndex}, true)); } /** @@ -803,8 +805,8 @@ export class Viewer { * @return {!Promise} */ postPopHistory(stackIndex) { - return this.sendMessageUnreliable_( - 'popHistory', {stackIndex}, true); + return /** @type {!Promise} */ (this.sendMessageUnreliable_( + 'popHistory', {stackIndex}, true)); } /** @@ -833,7 +835,7 @@ export class Viewer { // it should resolve in milli seconds. return timerFor(this.win).timeoutPromise(10000, cidPromise, 'base cid') .catch(error => { - dev().error(error); + dev().error(TAG_, error); return undefined; }); }); @@ -921,7 +923,8 @@ export class Viewer { return Promise.resolve(); } if (eventType == 'broadcast') { - this.broadcastObservable_.fire(data); + this.broadcastObservable_.fire( + /** @type {!JSONType|undefined} */ (data)); return Promise.resolve(); } dev().fine(TAG_, 'unknown message:', eventType); @@ -1014,7 +1017,7 @@ export class Viewer { * @param {string} eventType * @param {*} data * @param {boolean} awaitResponse - * @return {!Promise<*>|undefined} + * @return {?Promise<*>|undefined} * @private */ sendMessageUnreliable_(eventType, data, awaitResponse) { @@ -1090,7 +1093,7 @@ function parseParams_(str, allParams) { /** * Creates an error for the case where a channel cannot be established. - * @param {!Error|string=} opt_reason + * @param {*=} opt_reason * @return {!Error} */ function getChannelError(opt_reason) { diff --git a/src/service/viewport-impl.js b/src/service/viewport-impl.js index 2954f7b11354..c9e981812d46 100644 --- a/src/service/viewport-impl.js +++ b/src/service/viewport-impl.js @@ -119,7 +119,7 @@ export class Viewport { /** @private @const {!Observable} */ this.scrollObservable_ = new Observable(); - /** @private {?HTMLMetaElement|undefined} */ + /** @private {?Element|undefined} */ this.viewportMeta_ = undefined; /** @private {string|undefined} */ @@ -491,7 +491,7 @@ export class Viewport { } /** - * @return {?HTMLMetaElement} + * @return {?Element} * @private */ getViewportMeta_() { @@ -627,9 +627,7 @@ export class ViewportBindingDef { * independent fixed layer. * @return {boolean} */ - requiresFixedLayerTransfer() { - return false; - } + requiresFixedLayerTransfer() {} /** * Register a callback for scroll events. @@ -908,6 +906,9 @@ export class ViewportBindingNaturalIosEmbed_ { /** @private {?Element} */ this.scrollMoveEl_ = null; + /** @private {?Element} */ + this.endPosEl_ = null; + /** @private {!{x: number, y: number}} */ this.pos_ = {x: 0, y: 0}; @@ -936,7 +937,8 @@ export class ViewportBindingNaturalIosEmbed_ { /** @private */ setup_() { const documentElement = this.win.document.documentElement; - const documentBody = this.win.document.body; + const documentBody = /** @type {!Element} */ ( + this.win.document.body); // Embedded scrolling on iOS is rather complicated. IFrames cannot be sized // and be scrollable. Sizing iframe by scrolling height has a big negative @@ -1185,7 +1187,7 @@ export class ViewportBindingNaturalIosEmbed_ { * width=device-width,initial-scale=1,minimum-scale=1 * ``` * @param {string} content - * @return {!Object} + * @return {!Object} * @private Visible for testing only. */ export function parseViewportMeta(content) { @@ -1288,7 +1290,7 @@ function createViewport_(window) { * @return {!Viewport} */ export function installViewportService(window) { - return getService(window, 'viewport', () => { + return /** @type !Viewport} */ (getService(window, 'viewport', () => { return createViewport_(window); - }); + })); }; diff --git a/src/service/vsync-impl.js b/src/service/vsync-impl.js index 3fef0c7a8be8..30df68f6c09d 100644 --- a/src/service/vsync-impl.js +++ b/src/service/vsync-impl.js @@ -129,7 +129,7 @@ export class Vsync { */ run(task, opt_state) { this.tasks_.push(task); - this.states_.push(opt_state); + this.states_.push(opt_state || undefined); this.schedule_(); } @@ -160,9 +160,9 @@ export class Vsync { * @return {function(!VsyncStateDef=)} */ createTask(task) { - return opt_state => { + return /** @type {function(!VsyncStateDef=)} */ (opt_state => { this.run(task, opt_state); - }; + }); } /** @@ -259,9 +259,10 @@ export class Vsync { * @return {function(!VsyncStateDef=):boolean} */ createAnimTask(contextNode, task) { - return opt_state => { - return this.runAnim(contextNode, task, opt_state); - }; + return /** @type {function(!VsyncStateDef=):boolean} */ ( + opt_state => { + return this.runAnim(contextNode, task, opt_state); + }); } /** @@ -385,7 +386,7 @@ export class Vsync { * @return {!Vsync} */ export function installVsyncService(window) { - return getService(window, 'vsync', () => { + return /** @type {!Vsync} */ (getService(window, 'vsync', () => { return new Vsync(window, installViewerService(window)); - }); + })); }; diff --git a/src/service/xhr-impl.js b/src/service/xhr-impl.js index dd5b5e459138..a7fd189d5286 100644 --- a/src/service/xhr-impl.js +++ b/src/service/xhr-impl.js @@ -484,7 +484,8 @@ export class FetchResponse { user().assert(this.xhr_.responseXML, 'responseXML should exist. Make sure to return ' + 'Content-Type: text/html header.'); - return Promise.resolve(this.xhr_.responseXML); + return /** @type {!Promise} */ ( + Promise.resolve(dev().assert(this.xhr_.responseXML))); } /** diff --git a/src/shadow-embed.js b/src/shadow-embed.js index d75c2620b267..8a0d58ad095e 100644 --- a/src/shadow-embed.js +++ b/src/shadow-embed.js @@ -86,7 +86,10 @@ export function createShadowRoot(hostElement) { function createShadowRootPolyfill(hostElement) { const doc = hostElement.ownerDocument; const win = doc.defaultView; - const shadowRoot = doc.createElement('i-amp-shadow-root'); + const shadowRoot = /** @type {!ShadowRoot} */ ( + // Cast to ShadowRoot even though it is an Element + // TODO(@dvoytenko) Consider to switch to a type union instead. + /** @type {?} */ (doc.createElement('i-amp-shadow-root'))); shadowRoot.id = 'i-amp-sd-' + Math.floor(win.Math.random() * 10000); hostElement.appendChild(shadowRoot); hostElement.shadowRoot = hostElement.__AMP_SHADOW_ROOT = shadowRoot; @@ -96,10 +99,11 @@ function createShadowRootPolyfill(hostElement) { /** @type {!Element} */ shadowRoot.host = hostElement; - /** @type {function(string):?Element} */ + /** @type {function (this:ShadowRoot, string): ?HTMLElement} */ shadowRoot.getElementById = function(id) { const escapedId = escapeCssSelectorIdent(win, id); - return shadowRoot.querySelector(`#${escapedId}`); + return /** @type {HTMLElement|null} */ ( + shadowRoot.querySelector(`#${escapedId}`)); }; return shadowRoot; @@ -154,7 +158,7 @@ export function importShadowBody(shadowRoot, body) { } resultBody.style.position = 'relative'; shadowRoot.appendChild(resultBody); - return resultBody; + return dev().assertElement(resultBody); } @@ -171,12 +175,12 @@ export function importShadowBody(shadowRoot, body) { * as the first element in head and all style elements will be positioned * after. * @param {string=} opt_ext - * @return {!HTMLStyleElement} + * @return {!Element} */ export function installStylesForShadowRoot(shadowRoot, cssText, opt_isRuntimeCss, opt_ext) { return insertStyleElement( - shadowRoot.ownerDocument, + dev().assert(shadowRoot.ownerDocument), shadowRoot, transformShadowCss(shadowRoot, cssText), opt_isRuntimeCss || false, diff --git a/src/share-tracking-service.js b/src/share-tracking-service.js index 9465dfe59157..8f08dcebfdad 100644 --- a/src/share-tracking-service.js +++ b/src/share-tracking-service.js @@ -23,6 +23,7 @@ import {getElementServiceIfAvailable} from './element-service'; * @return {!Promise>} */ export function shareTrackingForOrNull(win) { - return getElementServiceIfAvailable(win, 'share-tracking', - 'amp-share-tracking'); + return /** @type {!Promise>} */ ( + getElementServiceIfAvailable(win, 'share-tracking', + 'amp-share-tracking')); } diff --git a/src/style-installer.js b/src/style-installer.js index 8a79a9f508ed..7b9c1c31b2d8 100644 --- a/src/style-installer.js +++ b/src/style-installer.js @@ -40,12 +40,12 @@ const bodyVisibleSentinel = '__AMP_BODY_VISIBLE'; * as the first element in head and all style elements will be positioned * after. * @param {string=} opt_ext - * @return {!HTMLStyleElement} + * @return {!Element} */ export function installStyles(doc, cssText, cb, opt_isRuntimeCss, opt_ext) { const style = insertStyleElement( doc, - dev().assert(doc.head), + dev().assertElement(doc.head), cssText, opt_isRuntimeCss || false, opt_ext || null); @@ -72,7 +72,7 @@ export function installStyles(doc, cssText, cb, opt_isRuntimeCss, opt_ext) { /** * Creates the properly configured style element. - * @param {!Document} doc + * @param {?Document} doc * @param {!Element|!ShadowRoot} cssRoot * @param {string} cssText * @param {boolean} isRuntimeCss @@ -110,7 +110,7 @@ export function insertStyleElement(doc, cssRoot, cssText, isRuntimeCss, ext) { */ export function makeBodyVisible(doc, opt_waitForExtensions) { const set = () => { - setStyles(dev().assert(doc.body), { + setStyles(dev().assertElement(doc.body), { opacity: 1, visibility: 'visible', animation: 'none', diff --git a/src/variant-service.js b/src/variant-service.js index 1ab0b4a25c6b..d9bb444349ba 100644 --- a/src/variant-service.js +++ b/src/variant-service.js @@ -23,5 +23,6 @@ import {getElementServiceIfAvailable} from './element-service'; * @return {!Promise>} */ export function variantForOrNull(win) { - return getElementServiceIfAvailable(win, 'variant', 'amp-experiment'); + return /** @type {!Promise>} */ ( + getElementServiceIfAvailable(win, 'variant', 'amp-experiment')); } diff --git a/test/functional/test-log.js b/test/functional/test-log.js index 0aadfa17867f..eb11c35acc61 100644 --- a/test/functional/test-log.js +++ b/test/functional/test-log.js @@ -414,6 +414,25 @@ describe('Logging', () => { expect(isUserErrorMessage(error.message)).to.be.false; expect(error.message).to.contain('test-other'); }); + + it('should pass for elements', () => { + log.assertElement(document.documentElement); + const element = document.createElement('element'); + const ret = log.assertElement(element); + expect(ret).to.equal(element); + }); + + it('should should identify non-elements', () => { + expect(() => { + log.assertElement(document); + }).to.throw(/Element expected: /); + expect(() => { + log.assertElement(null); + }).to.throw(/Element expected: null/); + expect(() => { + log.assertElement(null, 'custom error'); + }).to.throw(/custom error: null/); + }); });