Fix mixin generation

Fixes #49.
Fixes #67.

Author: TimothyGu

README.md

@@ -297,7 +297,7 @@ webidl2js is implementing an ever-growing subset of the Web IDL specification. S
 - `typedef`s
 - Partial interfaces and dictionaries
 - Basic types (via [webidl-conversions](https://github.com/jsdom/webidl-conversions))
-- Mixins, i.e. `implements` (with a [notable bug](https://github.com/jsdom/webidl2js/issues/49))
+- Mixins, i.e. `implements`
 - Overload resolution (although [tricky cases are not easy on the implementation class](https://github.com/jsdom/webidl2js/issues/29))
 - Variadic arguments
 - `[Clamp]`

lib/constructs/interface.js

@@ -26,6 +26,9 @@ class Interface {
     this.idl = idl;
     this.name = idl.name;
     this.factory = Boolean(utils.getExtAttr(this.idl.extAttrs, "WebIDL2JSFactory"));
+    for (const member of this.idl.members) {
+      member.definingInterface = this.name;
+    }
 
     this.str = null;
     this.opts = opts;
@@ -50,13 +53,13 @@ class Interface {
   }
 
   _analyzeMembers() {
-    let definingInterface = null;
+    let memberInherited = false;
     for (const member of this.inheritedMembers()) {
-      if (member[0] === inherited) {
-        definingInterface = member[1];
+      if (member === inherited) {
+        memberInherited = true;
         continue;
       }
-      if (!definingInterface) {
+      if (!memberInherited) {
         let key;
         switch (member.type) {
           case "operation":
@@ -92,16 +95,16 @@ class Interface {
           case "iterable":
             if (this.iterable) {
               throw new Error(`Iterable interface ${this.name} inherits from another iterable interface ` +
-                              `${definingInterface}`);
+                              `${member.definingInterface}`);
             }
             break;
         }
       }
       if (member.type === "operation") {
         if (member.getter) {
           let msg = `Invalid getter ${member.name ? `"${member.name}" ` : ""}on interface ${this.name}`;
-          if (definingInterface) {
-            msg += ` (defined in ${definingInterface})`;
+          if (member.definingInterface !== this.name) {
+            msg += ` (defined in ${member.definingInterface})`;
           }
           msg += ": ";
           if (member.arguments.length < 1 ||
@@ -124,8 +127,8 @@ class Interface {
         }
         if (member.setter) {
           let msg = `Invalid setter ${member.name ? `"${member.name}" ` : ""}on interface ${this.name}`;
-          if (definingInterface) {
-            msg += ` (defined in ${definingInterface})`;
+          if (member.definingInterface !== this.name) {
+            msg += ` (defined in ${member.definingInterface})`;
           }
           msg += ": ";
 
@@ -149,8 +152,8 @@ class Interface {
         }
         if (member.deleter) {
           let msg = `Invalid deleter ${member.name ? `"${member.name}" ` : ""}on interface ${this.name}`;
-          if (definingInterface) {
-            msg += ` (defined in ${definingInterface})`;
+          if (member.definingInterface !== this.name) {
+            msg += ` (defined in ${member.definingInterface})`;
           }
           msg += ": ";
 
@@ -168,8 +171,12 @@ class Interface {
           }
         }
       }
-      if (!definingInterface && member.stringifier) {
-        const msg = `Invalid stringifier ${member.name ? `"${member.name}" ` : ""}on interface ${this.name}: `;
+      if (!memberInherited && member.stringifier) {
+        let msg = `Invalid stringifier ${member.name ? `"${member.name}" ` : ""}on interface ${this.name}`;
+        if (member.definingInterface !== this.name) {
+          msg += ` (defined in ${member.definingInterface})`;
+        }
+        msg += ": ";
         if (member.type === "operation") {
           if (!member.idlType) {
             member.idlType = { idlType: "DOMString" };
@@ -224,16 +231,16 @@ class Interface {
         forbiddenMembers.add(n);
       }
     }
-    definingInterface = null;
+    memberInherited = false;
     for (const member of this.inheritedMembers()) {
-      if (member[0] === inherited) {
-        definingInterface = member[1];
+      if (member === inherited) {
+        memberInherited = true;
         continue;
       }
       if (forbiddenMembers.has(member.name) || member.name && member.name[0] === "_") {
         let msg = `${member.name} is forbidden in interface ${this.name}`;
-        if (definingInterface) {
-          msg += ` (defined in ${definingInterface})`;
+        if (member.definingInterface !== this.name) {
+          msg += ` (defined in ${member.definingInterface})`;
         }
         throw new Error(msg);
       }
@@ -253,6 +260,23 @@ class Interface {
   }
 
   implements(source) {
+    const iface = this.ctx.interfaces.get(source);
+    if (!iface) {
+      if (this.ctx.options.suppressErrors) {
+        return;
+      }
+      throw new Error(`${source} interface not found (used as a mixin for ${this.name})`);
+    }
+
+    // TODO: are these actually allowed by the spec?
+    if (!this.ctx.options.suppressErrors) {
+      if (iface.idl.inheritance) {
+        throw new Error(`${source} is used as a mixin for ${this.name}, but it inherits from another interface`);
+      }
+      if (iface.mixins.length > 0) {
+        throw new Error(`${source} is used as a mixin for ${this.name}, but it mixes in other interfaces`);
+      }
+    }
     this.mixins.push(source);
   }
 
@@ -374,11 +398,12 @@ class Interface {
 
   * inheritedMembers() {
     yield* this.idl.members;
-    for (const iface of [...this.mixins, this.idl.inheritance]) {
-      if (this.ctx.interfaces.has(iface)) {
-        yield [inherited, iface];
-        yield* this.ctx.interfaces.get(iface).inheritedMembers();
-      }
+    for (const mixin of this.mixins) {
+      yield* this.ctx.interfaces.get(mixin).idl.members;
+    }
+    if (this.idl.inheritance && this.ctx.interfaces.has(this.idl.inheritance)) {
+      yield inherited;
+      yield* this.ctx.interfaces.get(this.idl.inheritance).inheritedMembers();
     }
   }
 
@@ -389,11 +414,8 @@ class Interface {
       this.requires.add(this.idl.inheritance);
     }
 
-    if (this.mixins.length !== 0) {
-      this.requires.addRaw("mixin", "utils.mixin");
-      for (const mixin of this.mixins) {
-        this.requires.add(mixin);
-      }
+    for (const mixin of this.mixins) {
+      this.requires.add(mixin);
     }
 
     this.str = `
@@ -406,7 +428,6 @@ class Interface {
   generateMixins() {
     for (const mixin of this.mixins) {
       this.str += `
-        mixin(${this.name}.prototype, ${mixin}.interface.prototype);
         ${mixin}.mixedInto.push(${this.name});
       `;
     }
@@ -870,7 +891,7 @@ class Interface {
     if (this.supportsNamedProperties && !utils.isGlobal(this.idl)) {
       const unforgeable = new Set();
       for (const m of this.inheritedMembers()) {
-        if (m[0] === inherited) {
+        if (m === inherited) {
           continue;
         }
         if ((m.type === "attribute" || m.type === "operation") && !m.static &&

lib/output/utils.js

@@ -20,17 +20,6 @@ function getCopyToBytes(bufferSource) {
   return Buffer.from(getReferenceToBytes(bufferSource));
 }
 
-function mixin(target, source) {
-  const keys = Object.getOwnPropertyNames(source);
-  for (let i = 0; i < keys.length; ++i) {
-    if (keys[i] in target) {
-      continue;
-    }
-
-    Object.defineProperty(target, keys[i], Object.getOwnPropertyDescriptor(source, keys[i]));
-  }
-}
-
 const wrapperSymbol = Symbol("wrapper");
 const implSymbol = Symbol("impl");
 const sameObjectCaches = Symbol("SameObject caches");
@@ -100,7 +89,6 @@ module.exports = exports = {
   isObject,
   getReferenceToBytes,
   getCopyToBytes,
-  mixin,
   wrapperSymbol,
   implSymbol,
   getSameObject,