From 8606adc8181b95cc88889fa1a9026431e2dead94 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Fri, 17 Apr 2020 16:53:04 -0400 Subject: [PATCH] Ensure promise-typed operations and attributes return promises Closes #79. This removes support for overloading promise operations and non-promise operations, since that would require a more ambitious approach and doesn't seem to appear on the web. --- lib/constructs/attribute.js | 10 ++ lib/constructs/operation.js | 21 +++ test/__snapshots__/test.js.snap | 228 ++++++++++++++++++++++++++++++++ test/cases/CEReactions.webidl | 3 + test/cases/Overloads.webidl | 2 +- test/cases/PromiseTypes.webidl | 9 ++ 6 files changed, 272 insertions(+), 1 deletion(-) diff --git a/lib/constructs/attribute.js b/lib/constructs/attribute.js index b42da313..174df9a1 100644 --- a/lib/constructs/attribute.js +++ b/lib/constructs/attribute.js @@ -23,6 +23,10 @@ class Attribute { const onInstance = utils.isOnInstance(this.idl, this.interface.idl); + const async = this.idl.idlType.generic === "Promise"; + const promiseHandlingBefore = async ? `try {` : ``; + const promiseHandlingAfter = async ? `} catch (e) { return Promise.reject(e); }` : ``; + let brandCheck = ` if (!exports.is(esValue)) { throw new TypeError("Illegal invocation"); @@ -72,12 +76,18 @@ class Attribute { } addMethod(this.idl.name, [], ` + ${promiseHandlingBefore} const esValue = this !== null && this !== undefined ? this : globalObject; ${brandCheck} ${getterBody} + ${promiseHandlingAfter} `, "get", { configurable }); if (!this.idl.readonly) { + if (async) { + throw new Error(`Illegal promise-typed attribute "${this.idl.name}" in interface "${this.interface.idl.name}"`); + } + let idlConversion; if (typeof this.idl.idlType.idlType === "string" && !this.idl.idlType.nullable && this.ctx.enumerations.has(this.idl.idlType.idlType)) { diff --git a/lib/constructs/operation.js b/lib/constructs/operation.js index 53fb33c8..b163d137 100644 --- a/lib/constructs/operation.js +++ b/lib/constructs/operation.js @@ -27,6 +27,22 @@ class Operation { return firstOverloadOnInstance; } + isAsync() { + // As of the time of this writing, the current spec does not disallow such overloads, but the intention is to do so: + // https://github.com/heycam/webidl/pull/776 + const firstAsync = this.idls[0].idlType.generic === "Promise"; + for (const overload of this.idls.slice(1)) { + const isAsync = overload.idlType.generic === "Promise"; + if (isAsync !== firstAsync) { + throw new Error( + `Overloading between Promise and non-Promise return types is not allowed: operation ` + + `"${this.name}" on ${this.interface.name}` + ); + } + } + return firstAsync; + } + fixUpArgsExtAttrs() { for (const idl of this.idls) { for (const arg of idl.arguments) { @@ -50,6 +66,9 @@ class Operation { } const onInstance = this.isOnInstance(); + const async = this.isAsync(); + const promiseHandlingBefore = async ? `try {` : ``; + const promiseHandlingAfter = async ? `} catch (e) { return Promise.reject(e); }` : ``; const type = this.static ? "static operation" : "regular operation"; const overloads = Overloads.getEffectiveOverloads(type, this.name, 0, this.interface); @@ -100,6 +119,8 @@ class Operation { } str += invocation; + str = promiseHandlingBefore + str + promiseHandlingAfter; + if (this.static) { this.interface.addStaticMethod(this.name, argNames, str); } else { diff --git a/test/__snapshots__/test.js.snap b/test/__snapshots__/test.js.snap index 7f3f3c24..0099357e 100644 --- a/test/__snapshots__/test.js.snap +++ b/test/__snapshots__/test.js.snap @@ -446,6 +446,24 @@ exports.install = (globalObject, globalNames) => { } } + promiseOperation() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + CEReactions.preSteps(globalObject); + try { + return utils.tryWrapperForImpl(esValue[implSymbol].promiseOperation()); + } finally { + CEReactions.postSteps(globalObject); + } + } catch (e) { + return Promise.reject(e); + } + } + get attr() { const esValue = this !== null && this !== undefined ? this : globalObject; @@ -479,10 +497,31 @@ exports.install = (globalObject, globalNames) => { CEReactions.postSteps(globalObject); } } + + get promiseAttribute() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + CEReactions.preSteps(globalObject); + try { + return utils.tryWrapperForImpl(esValue[implSymbol][\\"promiseAttribute\\"]); + } finally { + CEReactions.postSteps(globalObject); + } + } catch (e) { + return Promise.reject(e); + } + } } Object.defineProperties(CEReactions.prototype, { method: { enumerable: true }, + promiseOperation: { enumerable: true }, attr: { enumerable: true }, + promiseAttribute: { enumerable: true }, [Symbol.toStringTag]: { value: \\"CEReactions\\", configurable: true } }); if (globalObject[ctorRegistrySymbol] === undefined) { @@ -3655,12 +3694,92 @@ exports.install = (globalObject, globalNames) => { } return esValue[implSymbol].promiseConsumer(...args); } + + promiseOperation() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol].promiseOperation()); + } catch (e) { + return Promise.reject(e); + } + } + + unforgeablePromiseOperation() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol].unforgeablePromiseOperation()); + } catch (e) { + return Promise.reject(e); + } + } + + get promiseAttribute() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol][\\"promiseAttribute\\"]); + } catch (e) { + return Promise.reject(e); + } + } + + get unforgeablePromiseAttribute() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol][\\"unforgeablePromiseAttribute\\"]); + } catch (e) { + return Promise.reject(e); + } + } + + static staticPromiseOperation() { + try { + return utils.tryWrapperForImpl(Impl.implementation.staticPromiseOperation()); + } catch (e) { + return Promise.reject(e); + } + } + + static get staticPromiseAttribute() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + + return Impl.implementation[\\"staticPromiseAttribute\\"]; + } catch (e) { + return Promise.reject(e); + } + } } Object.defineProperties(PromiseTypes.prototype, { voidPromiseConsumer: { enumerable: true }, promiseConsumer: { enumerable: true }, + promiseOperation: { enumerable: true }, + unforgeablePromiseOperation: { enumerable: true }, + promiseAttribute: { enumerable: true }, + unforgeablePromiseAttribute: { enumerable: true }, [Symbol.toStringTag]: { value: \\"PromiseTypes\\", configurable: true } }); + Object.defineProperties(PromiseTypes, { + staticPromiseOperation: { enumerable: true }, + staticPromiseAttribute: { enumerable: true } + }); if (globalObject[ctorRegistrySymbol] === undefined) { globalObject[ctorRegistrySymbol] = Object.create(null); } @@ -9166,6 +9285,19 @@ exports.install = (globalObject, globalNames) => { return esValue[implSymbol].method(); } + promiseOperation() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol].promiseOperation()); + } catch (e) { + return Promise.reject(e); + } + } + get attr() { const esValue = this !== null && this !== undefined ? this : globalObject; @@ -9189,10 +9321,26 @@ exports.install = (globalObject, globalNames) => { esValue[implSymbol][\\"attr\\"] = V; } + + get promiseAttribute() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol][\\"promiseAttribute\\"]); + } catch (e) { + return Promise.reject(e); + } + } } Object.defineProperties(CEReactions.prototype, { method: { enumerable: true }, + promiseOperation: { enumerable: true }, attr: { enumerable: true }, + promiseAttribute: { enumerable: true }, [Symbol.toStringTag]: { value: \\"CEReactions\\", configurable: true } }); if (globalObject[ctorRegistrySymbol] === undefined) { @@ -12349,12 +12497,92 @@ exports.install = (globalObject, globalNames) => { } return esValue[implSymbol].promiseConsumer(...args); } + + promiseOperation() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol].promiseOperation()); + } catch (e) { + return Promise.reject(e); + } + } + + unforgeablePromiseOperation() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol].unforgeablePromiseOperation()); + } catch (e) { + return Promise.reject(e); + } + } + + get promiseAttribute() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol][\\"promiseAttribute\\"]); + } catch (e) { + return Promise.reject(e); + } + } + + get unforgeablePromiseAttribute() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return utils.tryWrapperForImpl(esValue[implSymbol][\\"unforgeablePromiseAttribute\\"]); + } catch (e) { + return Promise.reject(e); + } + } + + static staticPromiseOperation() { + try { + return utils.tryWrapperForImpl(Impl.implementation.staticPromiseOperation()); + } catch (e) { + return Promise.reject(e); + } + } + + static get staticPromiseAttribute() { + try { + const esValue = this !== null && this !== undefined ? this : globalObject; + + return Impl.implementation[\\"staticPromiseAttribute\\"]; + } catch (e) { + return Promise.reject(e); + } + } } Object.defineProperties(PromiseTypes.prototype, { voidPromiseConsumer: { enumerable: true }, promiseConsumer: { enumerable: true }, + promiseOperation: { enumerable: true }, + unforgeablePromiseOperation: { enumerable: true }, + promiseAttribute: { enumerable: true }, + unforgeablePromiseAttribute: { enumerable: true }, [Symbol.toStringTag]: { value: \\"PromiseTypes\\", configurable: true } }); + Object.defineProperties(PromiseTypes, { + staticPromiseOperation: { enumerable: true }, + staticPromiseAttribute: { enumerable: true } + }); if (globalObject[ctorRegistrySymbol] === undefined) { globalObject[ctorRegistrySymbol] = Object.create(null); } diff --git a/test/cases/CEReactions.webidl b/test/cases/CEReactions.webidl index 655fc097..3d686a2e 100644 --- a/test/cases/CEReactions.webidl +++ b/test/cases/CEReactions.webidl @@ -6,4 +6,7 @@ interface CEReactions { getter DOMString (DOMString name); [CEReactions] setter void (DOMString name, DOMString value); [CEReactions] deleter void (DOMString name); + + [CEReactions] Promise promiseOperation(); + [CEReactions] readonly attribute Promise promiseAttribute; }; diff --git a/test/cases/Overloads.webidl b/test/cases/Overloads.webidl index f4d53363..175789ee 100644 --- a/test/cases/Overloads.webidl +++ b/test/cases/Overloads.webidl @@ -6,7 +6,7 @@ interface Overloads { DOMString compatible(DOMString arg1); byte compatible(DOMString arg1, DOMString arg2); - Promise compatible(DOMString arg1, DOMString arg2, optional long arg3 = 0); + URL compatible(DOMString arg1, DOMString arg2, optional long arg3 = 0); DOMString incompatible1(DOMString arg1); byte incompatible1(long arg1); diff --git a/test/cases/PromiseTypes.webidl b/test/cases/PromiseTypes.webidl index fb79c67e..7b206782 100644 --- a/test/cases/PromiseTypes.webidl +++ b/test/cases/PromiseTypes.webidl @@ -2,4 +2,13 @@ interface PromiseTypes { void voidPromiseConsumer(Promise p); void promiseConsumer(Promise p); + + Promise promiseOperation(); + readonly attribute Promise promiseAttribute; + + [Unforgeable] Promise unforgeablePromiseOperation(); + [Unforgeable] readonly attribute Promise unforgeablePromiseAttribute; + + static Promise staticPromiseOperation(); + static readonly attribute Promise staticPromiseAttribute; };