Use class/object method definitions for operations

Fixes #97.
jsdom · Dec 25, 2017 · 112a4aa · 112a4aa
1 parent ba5edd9
commit 112a4aa
Show file tree

Hide file tree

Showing 9 changed files with 2,668 additions and 2,900 deletions.
diff --git a/README.md b/README.md
@@ -9,13 +9,13 @@ As a very minimal example, the Web IDL file
 ```webidl
 interface SomeInterface {
   unsigned long long add(unsigned long x, unsigned long y);
-}
+};
 ```
 
 combined with the JavaScript implementation class file
 
 ```js
-exports.implementation = class {
+exports.implementation = class SomeInterfaceImpl {
   add(x, y) {
     return x + y;
   }
@@ -30,38 +30,38 @@ const impl = require("./utils.js").implSymbol;
 
 const Impl = require("./SomeInterface-impl.js").implementation;
 
-function SomeInterface() {
-  throw new TypeError("Illegal constructor");
-}
-
-SomeInterface.prototype.add = function add(x, y) {
-  if (!exports.is(this)) {
-    throw new TypeError("Illegal invocation");
-  }
-  if (arguments.length < 2) {
-    throw new TypeError(
-      "Failed to execute 'add' on 'SomeInterface': 2 arguments required, but only " +
-        arguments.length +
-        " present."
-    );
+class SomeInterface {
+  constructor() {
+    throw new TypeError("Illegal constructor");
   }
 
-  const args = [];
-  args[0] = conversions["unsigned long"](arguments[0], {
-    context: "Failed to execute 'add' on 'SomeInterface': parameter 1"
-  });
-  args[1] = conversions["unsigned long"](arguments[1], {
-    context: "Failed to execute 'add' on 'SomeInterface': parameter 2"
-  });
-
-  this[impl].add(...args);
-};
+  add(x, y) {
+    if (!exports.is(this)) {
+      throw new TypeError("Illegal invocation");
+    }
+    if (arguments.length < 2) {
+      throw new TypeError(
+        "Failed to execute 'add' on 'SomeInterface': 2 arguments required, but only " +
+          arguments.length +
+          " present."
+      );
+    }
+
+    const args = [];
+    args[0] = conversions["unsigned long"](arguments[0], {
+      context: "Failed to execute 'add' on 'SomeInterface': parameter 1"
+    });
+    args[1] = conversions["unsigned long"](arguments[1], {
+      context: "Failed to execute 'add' on 'SomeInterface': parameter 2"
+    });
+
+    return this[impl].add(...args);
+  }
+}
 
-Object.defineProperty(SomeInterface.prototype, Symbol.toStringTag, {
-  value: "SomeInterface",
-  writable: false,
-  enumerable: false,
-  configurable: true
+Object.defineProperties(SomeInterface.prototype, {
+  add: { enumerable: true },
+  [Symbol.toStringTag]: { value: "SomeInterface", configurable: true }
 });
 
 exports.interface = SomeInterface;
@@ -82,6 +82,7 @@ The above is a simplification of the actual generated code, but should give you
   - Brand-checking on operation invocation
   - Argument-length checking for non-optional arguments
   - Argument conversion according to the rules specified in Web IDL for given argument types
+  - Enumerability of operations
   - @@toStringTag semantics to make `Object.prototype.toString.call()` behave correctly
 - After performing Web IDL-related processing, webidl2js delegates to the implementation class for the non-boilerplate parts of `add()`. This allows you to focus on writing the interesting parts of the implementation without worrying about types, brand-checking, parameter-processing, etc.
 - webidl2js attempts to generate informative error messages using what it knows.

diff --git a/lib/constructs/attribute.js b/lib/constructs/attribute.js
@@ -15,18 +15,17 @@ class Attribute {
   }
 
   generate() {
-    let str = "";
     const requires = new utils.RequiresMap(this.ctx);
 
     const configurable = !utils.getExtAttr(this.idl.extAttrs, "Unforgeable");
     const shouldReflect = utils.getExtAttr(this.idl.extAttrs, "Reflect");
     const sameObject = utils.getExtAttr(this.idl.extAttrs, "SameObject");
 
+    const onInstance = utils.isOnInstance(this.idl, this.interface.idl);
+
     let objName = `this`;
-    let definedOn = this.interface.name + (this.static ? "" : ".prototype");
-    if (utils.isOnInstance(this.idl, this.interface.idl)) { // we're in a setup method
+    if (onInstance) { // we're in a setup method
       objName = `obj`;
-      definedOn = `obj`;
     }
     let brandCheck = `
       if (!this || !module.exports.is(this)) {
@@ -39,6 +38,10 @@ class Attribute {
       getterBody = `return ${objName}[impl]["${this.idl.name}"];`;
     }
 
+    const addMethod = this.static ?
+      this.interface.addStaticMethod.bind(this.interface) :
+      this.interface.addMethod.bind(this.interface, onInstance ? "instance" : "prototype");
+
     if (this.static) {
       brandCheck = "";
       getterBody = `return Impl.implementation["${this.idl.name}"];`;
@@ -60,13 +63,11 @@ class Attribute {
       getterBody = `return utils.getSameObject(this, "${this.idl.name}", () => { ${getterBody} });`;
     }
 
-    str += `
-      Object.defineProperty(${definedOn}, "${this.idl.name}", {
-        get() {
-          ${brandCheck}
-          ${getterBody}
-        },
-    `;
+    addMethod(this.idl.name, [], `
+      ${brandCheck}
+      ${getterBody}
+    `, "get", { configurable });
+
     if (!this.idl.readonly) {
       let idlConversion;
       if (typeof this.idl.idlType.idlType === "string" && !this.idl.idlType.nullable &&
@@ -85,70 +86,39 @@ class Attribute {
         requires.merge(conv.requires);
         idlConversion = conv.body;
       }
-      str += `
-        set(V) {
-          ${brandCheck}
-          ${idlConversion}
-          ${setterBody}
-        },
-      `;
+
+      addMethod(this.idl.name, ["V"], `
+        ${brandCheck}
+        ${idlConversion}
+        ${setterBody}
+      `, "set", { configurable });
     } else if (utils.getExtAttr(this.idl.extAttrs, "PutForwards")) {
-      str += `
-        set(V) {
-          ${brandCheck}
-          this.${this.idl.name}.${utils.getExtAttr(this.idl.extAttrs, "PutForwards").rhs.value} = V;
-        },
-      `;
+      addMethod(this.idl.name, ["V"], `
+        ${brandCheck}
+        this.${this.idl.name}.${utils.getExtAttr(this.idl.extAttrs, "PutForwards").rhs.value} = V;
+      `, "set", { configurable });
     } else if (utils.getExtAttr(this.idl.extAttrs, "Replaceable")) {
-      str += `
-        set(V) {
-          ${brandCheck}
-          Object.defineProperty(this, "${this.idl.name}", {
-            configurable: true,
-            enumerable: true,
-            value: V,
-            writable: true
-          });
-        },
-      `;
+      addMethod(this.idl.name, ["V"], `
+        ${brandCheck}
+        Object.defineProperty(this, "${this.idl.name}", {
+          configurable: true,
+          enumerable: true,
+          value: V,
+          writable: true
+        });
+      `, "set", { configurable });
     }
-    str += `
-        enumerable: true,
-        configurable: ${JSON.stringify(configurable)}
-      });
-    `;
 
-    if (this.idl.stringifier) {
-      const functionExpression = `
-        function toString() {
-          if (!this || !module.exports.is(this)) {
-            throw new TypeError("Illegal invocation");
-          }
-          ${getterBody};
+    if (!this.static && this.idl.stringifier) {
+      addMethod("toString", [], `
+        if (!this || !module.exports.is(this)) {
+          throw new TypeError("Illegal invocation");
         }
-      `;
-
-      if (utils.getExtAttr(this.idl.extAttrs, "Unforgeable")) {
-        str += `
-          Object.defineProperty(${definedOn}, "toString", {
-            writable: false,
-            enumerable: true,
-            configurable: false,
-            value: ${functionExpression}
-          });
-        `;
-      } else {
-        str += `
-          ${definedOn}.toString = ${functionExpression};
-        `;
-      }
-      str += "\n";
+        ${getterBody};
+      `, "regular", { configurable, writable: configurable });
     }
 
-    return {
-      requires,
-      body: str
-    };
+    return { requires };
   }
 }
 

diff --git a/lib/constructs/constant.js b/lib/constructs/constant.js
@@ -12,20 +12,15 @@ class Constant {
   }
 
   generate() {
-    const body = `
-      Object.defineProperty(${this.interface.name}, "${this.idl.name}", {
-        value: ${utils.getDefault(this.idl.value)},
-        enumerable: true
-      });
-      Object.defineProperty(${this.interface.name}.prototype, "${this.idl.name}", {
-        value: ${utils.getDefault(this.idl.value)},
-        enumerable: true
-      });
-    `;
-    return {
-      requires: new utils.RequiresMap(this.ctx),
-      body
-    };
+    this.interface.addStaticProperty(this.idl.name, utils.getDefault(this.idl.value), {
+      configurable: false,
+      writable: false
+    });
+    this.interface.addProperty(this.interface.defaultWhence, this.idl.name, utils.getDefault(this.idl.value), {
+      configurable: false,
+      writable: false
+    });
+    return { requires: new utils.RequiresMap(this.ctx) };
   }
 }