Simplify [LegacyFactoryFunction] implementation

Author: ExE-Boss

README.md

@@ -409,21 +409,13 @@ However, it is not required! The wrapper classes will have a correct inheritance
 
 ### The `[LegacyFactoryFunction]` extended attribute
 
-For interfaces which have the `[LegacyFactoryFunction]` extended attribute, the implementation class file must contain the `legacyFactoryFunction` export, with the signature `(globalObject, legacyFactoryFunctionArgs, legacyFactoryFunctionName)`, which is used for:
+For interfaces which have the `[LegacyFactoryFunction]` extended attribute, the implementation class file must contain the `legacyFactoryFunction` export, with the signature `(globalObject, ...legacyFactoryFunctionArgs)`, which is used for:
 
 - Setting up initial state that will always be used, such as caches or default values
 - Keep a reference to the relevant `globalObject` for later consumption.
 - Processing constructor arguments `legacyFactoryFunctionArgs` passed to the legacy factory function constructor, if the legacy factory function takes arguments.
-- Switching on the `legacyFactoryFunctionName`, if the interface defines multiple legacy factory functions, eg.:
 
-  ```webidl
-  [LegacyFactoryFunction=OverloadedLegacyFactoryFunction(DOMString arg1),
-   LegacyFactoryFunction=OverloadedLegacyFactoryFunction(long arg1, long arg2),
-   LegacyFactoryFunction=SimpleLegacyFactoryFunction(optional DOMString src)]
-  interface SomeInterface {};
-  ```
-
-The `legacyFactoryFunction` export is called with a `this` value of a new uninitialized implementation instance, which may be ignored by returning a different object, similarly to how constructors with overridden return types are implemented.
+The `legacyFactoryFunction` export is called with a `this` value of a new uninitialized implementation instance, which may be ignored by returning a different object, similarly to how constructors with overridden return values are implemented.
 
 ### The init export
 

lib/constructs/interface.js

@@ -33,65 +33,6 @@ const defaultClassMethodDescriptor = {
   writable: true
 };
 
-class LegacyFactoryFunction {
-  constructor(ctx, I, idl) {
-    this.ctx = ctx;
-    this.interface = I;
-    this.idls = [idl];
-    this.name = idl.rhs.value;
-  }
-
-  generate() {
-    const requires = new utils.RequiresMap(this.ctx);
-
-    let str = "";
-
-    if (!this.name) {
-      throw new Error(`Internal error: this legacy factory function does not have a name (in interface ${this.interface.name})`);
-    }
-
-    const overloads = Overloads.getEffectiveOverloads("legacy factory function", this.name, 0, this.interface);
-    let minOp = overloads[0];
-    for (let i = 1; i < overloads.length; ++i) {
-      if (overloads[i].nameList.length < minOp.nameList.length) {
-        minOp = overloads[i];
-      }
-    }
-
-    const argNames = minOp.nameList;
-
-    const conversions = Parameters.generateOverloadConversions(
-      this.ctx, "legacy factory function", this.name, this.interface, `Failed to construct '${this.name}': `);
-    requires.merge(conversions.requires);
-
-    const setupArgs = [
-      "globalObject",
-      conversions.hasArgs ? "args" : "[]",
-      `"${this.name}"`
-    ];
-
-    str += `
-      if (new.target === undefined) {
-        throw new TypeError("Class constructor ${this.name} cannot be invoked without 'new'");
-      }
-
-      ${conversions.body}
-    `;
-
-    // This implements the WebIDL legacy factory function behavior, as well as support for overridding
-    // the return type, which is used by HTML's element legacy factory functions:
-    str += `
-      const thisArgument = exports.new(globalObject, new.target);
-      const result = Impl.legacyFactoryFunction.call(thisArgument, ${formatArgs(setupArgs)});
-      return utils.tryWrapperForImpl(utils.isObject(result) ? result : thisArgument);
-    `;
-
-    this.interface.addLegacyFactoryFunction(this.name, argNames, str);
-
-    return { requires };
-  }
-}
-
 class Interface {
   constructor(ctx, idl, opts) {
     this.ctx = ctx;
@@ -113,7 +54,7 @@ class Interface {
     this.attributes = new Map();
     this.staticAttributes = new Map();
     this.constants = new Map();
-    this.legacyFactoryFunctions = new Map();
+    this.legacyFactoryFunctions = [];
 
     this.indexedGetter = null;
     this.indexedSetter = null;
@@ -130,7 +71,6 @@ class Interface {
     this._outputStaticMethods = new Map();
     this._outputProperties = new Map();
     this._outputStaticProperties = new Map();
-    this._outputLegacyFactoryFunctions = new Map();
 
     const global = utils.getExtAttr(this.idl.extAttrs, "Global");
     this.isGlobal = Boolean(global);
@@ -242,10 +182,6 @@ class Interface {
     this._outputStaticMethods.set(propName, { type, args, body, descriptor });
   }
 
-  addLegacyFactoryFunction(name, args, body) {
-    this._outputLegacyFactoryFunctions.set(name, { args, body });
-  }
-
   // whence is either "instance" or "prototype"
   addProperty(whence, propName, str, {
     configurable = true,
@@ -460,19 +396,25 @@ class Interface {
       }
     }
 
+    let legacyFactoryFunctionName;
     for (const attr of this.idl.extAttrs) {
       if (attr.name === "LegacyFactoryFunction") {
-        if (attr.rhs.type !== "identifier" || !attr.arguments) {
+        if (!attr.rhs || attr.rhs.type !== "identifier" || !attr.arguments) {
           throw new Error(`[LegacyFactoryFunction] must take a named argument list`);
         }
 
         const name = attr.rhs.value;
-
-        if (!this.legacyFactoryFunctions.has(name)) {
-          this.legacyFactoryFunctions.set(name, new LegacyFactoryFunction(this.ctx, this, attr));
-        } else {
-          this.legacyFactoryFunctions.get(name).idls.push(attr);
+        if (legacyFactoryFunctionName === undefined) {
+          legacyFactoryFunctionName = name;
+        } else if (legacyFactoryFunctionName !== name) {
+          // This is currently valid, but not used anywhere, and there are plans to disallow it:
+          // https://github.com/jsdom/webidl2js/pull/213#issuecomment-621277733
+          throw new Error(
+            `Multiple [LegacyFactoryFunction] definitions with different names are not supported on ${this.name}`
+          );
         }
+
+        this.legacyFactoryFunctions.push(attr);
       }
     }
   }
@@ -1403,11 +1345,6 @@ class Interface {
       const data = member.generate();
       this.requires.merge(data.requires);
     }
-
-    for (const legacyFactoryFunction of this.legacyFactoryFunctions.values()) {
-      const data = legacyFactoryFunction.generate();
-      this.requires.merge(data.requires);
-    }
   }
 
   generateOffInstanceMethods() {
@@ -1572,29 +1509,63 @@ class Interface {
     }
   }
 
-  generateLegacyFactoryFunctions() {
-    for (const [name, { args, body }] of this._outputLegacyFactoryFunctions) {
-      this.str += `
-        {
-          function ${name}(${formatArgs(args)}) {
-            ${body}
-          }
+  generateLegacyFactoryFunction() {
+    const { legacyFactoryFunctions } = this;
+    if (legacyFactoryFunctions.length === 0) {
+      return;
+    }
 
-          Object.defineProperty(${name}, "prototype", {
-            configurable: false,
-            enumerable: false,
-            writable: false,
-            value: ${this.name}.prototype
-          })
+    const name = legacyFactoryFunctions[0].rhs.value;
 
-          Object.defineProperty(globalObject, "${name}", {
-            configurable: true,
-            writable: true,
-            value: ${name}
-          });
-        }
-      `;
+    if (!name) {
+      throw new Error(`Internal error: The legacy factory function does not have a name (in interface ${this.name})`);
+    }
+
+    const overloads = Overloads.getEffectiveOverloads("legacy factory function", name, 0, this);
+    let minOp = overloads[0];
+    for (let i = 1; i < overloads.length; ++i) {
+      if (overloads[i].nameList.length < minOp.nameList.length) {
+        minOp = overloads[i];
+      }
     }
+
+    const args = minOp.nameList;
+    const conversions = Parameters.generateOverloadConversions(
+      this.ctx, "legacy factory function", name, this, `Failed to construct '${name}': `);
+    this.requires.merge(conversions.requires);
+
+    const argsSpread = conversions.hasArgs ? "...args" : "";
+
+    this.str += `
+      function ${name}(${formatArgs(args)}) {
+        if (new.target === undefined) {
+          throw new TypeError("Class constructor ${name} cannot be invoked without 'new'");
+        }
+
+        ${conversions.body}
+    `;
+
+    // This implements the WebIDL legacy factory function behavior, as well as support for overridding
+    // the return type, which is used by HTML's element legacy factory functions:
+    this.str += `
+        const thisArgument = exports.new(globalObject, new.target);
+        const result = Impl.legacyFactoryFunction.call(thisArgument, globalObject, ${argsSpread});
+        return utils.tryWrapperForImpl(utils.isObject(result) ? result : thisArgument);
+      }
+
+      Object.defineProperty(${name}, "prototype", {
+        configurable: false,
+        enumerable: false,
+        writable: false,
+        value: ${this.name}.prototype
+      })
+
+      Object.defineProperty(globalObject, "${name}", {
+        configurable: true,
+        writable: true,
+        value: ${name}
+      });
+    `;
   }
 
   generateInstall() {
@@ -1662,7 +1633,7 @@ class Interface {
       }
     }
 
-    this.generateLegacyFactoryFunctions();
+    this.generateLegacyFactoryFunction();
 
     this.str += `
       };

lib/overloads.js

@@ -12,7 +12,7 @@ function getOperations(type, A, I) {
       return I.constructorOperations;
     }
     case "legacy factory function":
-      return I.legacyFactoryFunctions.get(A).idls;
+      return I.legacyFactoryFunctions;
   }
   throw new RangeError(`${type}s are not yet supported`);
 }