Repository: w3c/webidl2.js
Pull request #262 (2019-02-15 00:52:34)

refactor: self-containing classes
A trial to implement https://github.com/w3c/webidl2.js/issues/138#issuecomment-455918738.

marcoscaceres: Looking good. 
saschanaz: No real value here yet, the commit here is for future self-containing class instances get their own token references that include position information.

TIL new GitHub draft PR so I wanted to try it 😆, IMO good as this will constantly remind me that I have a WIP branch.
marcoscaceres: How does one do the Draft PR? That’s quite a useful feature. 
saschanaz: It's a fancy new feature: https://github.blog/2019-02-14-introducing-draft-pull-requests/
saschanaz: IMO mostly done, not feel great about `toJSON()` but it's needed for JSON compatibility.
saschanaz: Too much changes so I'll open smaller PRs soon.
marcoscaceres: Appreciate that. Maybe we can do one class at a time? Generally ok if it's around ~100 per pull request. 
saschanaz: @marcoscaceres This still have two pieces of features: name duplication checker and a feature for https://github.com/KhronosGroup/WebGL/issues/2216. Should I split them?
marcoscaceres: This is manageable now, so no need to split them. I'll try to review tonight or first thing tomorrow. 

Patch:
diff --git a/index.js b/index.js
index 2e783b9..abf94ff 100644
--- a/index.js
+++ b/index.js
@@ -1,4 +1,5 @@
 module.exports = Object.assign({},
   require("./lib/webidl2.js"),
-  require("./lib/writer.js")
+  require("./lib/writer.js"),
+  require("./lib/validator.js")
 );
diff --git a/lib/validator.js b/lib/validator.js
new file mode 100644
index 0000000..60adb08
--- /dev/null
+++ b/lib/validator.js
@@ -0,0 +1,159 @@
+"use strict";
+
+(() => {
+  function error(source, token, current, str) {
+    const consume_position = token.index;
+    const maxTokens = 5; // arbitrary but works well enough
+    const line =
+      !source[token.index].type === "eof" ? source[consume_position].line :
+      source.length > 1 ? source[consume_position - 1].line :
+      1;
+
+    const precedingLine = lastLine(
+      tokensToText(sliceTokens(-maxTokens), { precedes: true })
+    );
+
+    const procedingTokens = sliceTokens(maxTokens);
+    const procedingText = tokensToText(procedingTokens);
+    const procedingLine = procedingText.split("\n")[0];
+
+    const spaced = " ".repeat(precedingLine.length) + "^ " + str;
+    const context = precedingLine + procedingLine + "\n" + spaced;
+
+    const inside = current ? `, inside \`${current.partial ? "partial " : ""}${current.type} ${current.name}\`` : "";
+    return `Validation error at line ${line}${inside}:\n${context}`;
+
+    function sliceTokens(count) {
+      return count > 0 ?
+        source.slice(consume_position, consume_position + count) :
+        source.slice(Math.max(consume_position + count, 0), consume_position);
+    }
+
+    function tokensToText(inputs, { precedes } = {}) {
+      const text = inputs.map(t => t.trivia + t.value).join("");
+      const nextToken = source[consume_position];
+      if (nextToken.type === "eof") {
+        return text;
+      }
+      if (precedes) {
+        return text + nextToken.trivia;
+      }
+      return text.slice(nextToken.trivia.length);
+    }
+
+    function lastLine(text) {
+      const splitted = text.split("\n");
+      return splitted[splitted.length - 1];
+    }
+  }
+
+  function groupDefinitions(all) {
+    const unique = new Map();
+    const duplicates = new Set();
+    const partials = new Map();
+    for (const def of all) {
+      if (def.partial) {
+        const array = partials.get(def.name);
+        if (array) {
+          array.push(def);
+        } else {
+          partials.set(def.name, [def]);
+        }
+        continue;
+      }
+      if (!def.name) {
+        continue;
+      }
+      if (!unique.has(def.name)) {
+        unique.set(def.name, def);
+      } else {
+        duplicates.add(def);
+      }
+    }
+    return { all, unique, partials, duplicates };
+  }
+
+  function* checkDuplicatedNames({ unique, duplicates }) {
+    for (const dup of duplicates) {
+      const { name } = dup;
+      const message = `The name "${name}" of type "${unique.get(name).type}" was already seen`;
+      yield error(dup.source, dup.tokens.name, dup, message);
+    }
+  }
+
+  function* checkInterfaceMemberDuplication(defs) {
+    const interfaces = [...defs.unique.values()].filter(def => def.type === "interface");
+    const includesMap = getIncludesMap();
+
+    for (const i of interfaces) {
+      yield* forEachInterface(i);
+    }
+
+    function* forEachInterface(i) {
+      const opNames = getOperationNames(i);
+      const partials = defs.partials.get(i.name) || [];
+      const mixins = includesMap.get(i.name) || [];
+      for (const partial of partials) {
+        yield* forEachExtension(partial, opNames, i);
+      }
+      for (const mixin of mixins) {
+        yield* forEachExtension(mixin, opNames, i);
+      }
+    }
+
+    function* forEachExtension(ext, names, base) {
+      for (const op of ext.members.filter(mem => mem.type === "operation")) {
+        const name = (op.body && op.body.name) ? op.body.name.value : "";
+        if (name && names.has(name)) {
+          const message = `The operation "${name}" has already been defined in the base interface "${base.name}"`;
+          yield error(ext.source, op.body.tokens.name, ext, message);
+        }
+      }
+    }
+
+    function getOperationNames(i) {
+      const names = i.members
+        .filter(({type}) => type === "operation")
+        .map(op => (op.body && op.body.name) ? op.body.name.value : "");
+      return new Set(names);
+    }
+
+    function getIncludesMap() {
+      const map = new Map();
+      const includes = defs.all.filter(def => def.type === "includes");
+      for (const include of includes) {
+        const array = map.get(include.target);
+        const mixin = defs.unique.get(include.includes);
+        if (!mixin) {
+          continue;
+        }
+        if (array) {
+          array.push(mixin);
+        } else {
+          map.set(include.target, [mixin]);
+        }
+      }
+      return map;
+    }
+  }
+
+  function validate(ast) {
+    const defs = groupDefinitions(ast);
+    return [
+      ...checkDuplicatedNames(defs),
+      ...checkInterfaceMemberDuplication(defs)
+    ];
+  }
+
+  const obj = {
+    validate
+  };
+
+  if (typeof module !== 'undefined' && typeof module.exports !== 'undefined') {
+    module.exports = obj;
+  } else if (typeof define === 'function' && define.amd) {
+    define([], () => obj);
+  } else {
+    (self || window).WebIDL2Validator = obj;
+  }
+})();
diff --git a/lib/webidl2.js b/lib/webidl2.js
index 18a9f4d..f624579 100644
--- a/lib/webidl2.js
+++ b/lib/webidl2.js
@@ -189,7 +189,6 @@
 
   function parse(source) {
     source = source.slice();
-    const names = new Map();
     let current = null;
 
     const FLOAT = "float";
@@ -244,15 +243,6 @@
       }
     }
 
-    function sanitize_name(name, type) {
-      const unescaped = unescape(name);
-      if (names.has(unescaped)) {
-        error(`The name "${unescaped}" of type "${names.get(unescaped)}" was already seen`);
-      }
-      names.set(unescaped, type);
-      return unescaped;
-    }
-
     let consume_position = 0;
 
     function probe(type) {
@@ -445,6 +435,7 @@
         if (this.subtype.length) {
           return this.subtype;
         }
+        // Adding prefixes/postfixes for "unrestricted float", etc.
         const name = [
           this.tokens.prefix,
           this.tokens.base,
@@ -834,7 +825,6 @@
         const ret = new CallbackFunction({ tokens });
         tokens.name = consume(ID) || error("No name for callback");
         current = ret;
-        sanitize_name(ret.name, ret.type);
         tokens.assign = consume("=") || error("No assignment in callback");
         ret.idlType = return_type() || error("Missing return type");
         tokens.open = consume("(") || error("No arguments in callback");
@@ -1055,9 +1045,6 @@
         const { tokens } = instance;
         tokens.name = consume(ID) || error("No name for interface");
         current = instance;
-        if (!instance.partial) {
-          sanitize_name(instance.name, type);
-        }
         if (inheritable) {
           instance.inheritance = Inheritance.parse() || null;
         }
@@ -1241,7 +1228,6 @@
         }
         tokens.name = consume(ID) || error("No name for enum");
         current = new Enum({ tokens });
-        sanitize_name(current.name, "enum");
         tokens.open = consume("{") || error("Bodyless enum");
         current.values = list({
           parser: EnumValue.parse,
@@ -1297,7 +1283,6 @@
         ret.idlType = type_with_extended_attributes("typedef-type") || error("No type in typedef");
         tokens.name = consume(ID) || error("No name in typedef");
         current = ret;
-        sanitize_name(ret.name, "typedef");
         tokens.termination = consume(";") || error("Unterminated typedef");
         return ret;
       }


Test patch:
diff --git a/test/invalid.js b/test/invalid.js
index 2d04706..6622380 100644
--- a/test/invalid.js
+++ b/test/invalid.js
@@ -13,6 +13,8 @@ describe("Parses all of the invalid IDLs to check that they blow up correctly",
       const err = test.readText();
       if (test.error) {
         expect(test.error.message + "\n").toEqual(err);
+      } else if (test.validation) {
+        expect(test.validation.join("\n") + "\n").toEqual(err);
       } else {
         throw new Error("This test unexpectedly had no error");
       }
diff --git a/test/invalid/baseline/duplicate-escaped.txt b/test/invalid/baseline/duplicate-escaped.txt
index d02addb..78f78b8 100644
--- a/test/invalid/baseline/duplicate-escaped.txt
+++ b/test/invalid/baseline/duplicate-escaped.txt
@@ -1,3 +1,3 @@
-Syntax error at line 2, since `interface Iroha`:
+Validation error at line 2, inside `interface Iroha`:
 interface _Iroha {};
-                 ^ The name "Iroha" of type "interface" was already seen
+          ^ The name "Iroha" of type "interface" was already seen
diff --git a/test/invalid/baseline/duplicate.txt b/test/invalid/baseline/duplicate.txt
index 034d6e4..da4f5df 100644
--- a/test/invalid/baseline/duplicate.txt
+++ b/test/invalid/baseline/duplicate.txt
@@ -1,3 +1,3 @@
-Syntax error at line 3, since `interface Test`:
+Validation error at line 3, inside `interface Test`:
 interface Test {
-               ^ The name "Test" of type "typedef" was already seen
+          ^ The name "Test" of type "typedef" was already seen
diff --git a/test/invalid/baseline/overloads.txt b/test/invalid/baseline/overloads.txt
new file mode 100644
index 0000000..b51e8e1
--- /dev/null
+++ b/test/invalid/baseline/overloads.txt
@@ -0,0 +1,6 @@
+Validation error at line 7, inside `partial interface Base`:
+  void unique(short num)
+       ^ The operation "unique" has already been defined in the base interface "Base"
+Validation error at line 11, inside `interface mixin Extension`:
+  void unique(string str)
+       ^ The operation "unique" has already been defined in the base interface "Base"
diff --git a/test/invalid/idl/overloads.widl b/test/invalid/idl/overloads.widl
new file mode 100644
index 0000000..3d62e63
--- /dev/null
+++ b/test/invalid/idl/overloads.widl
@@ -0,0 +1,14 @@
+interface Base {
+  // Operations cannot be overloaded across partial interfaces and mixins
+  void unique();
+};
+
+partial interface Base {
+  void unique(short num);
+};
+
+interface mixin Extension {
+  void unique(string str);
+};
+Base includes Extension;
+Base includes Unknown;
diff --git a/test/util/acquire.js b/test/util/acquire.js
index 8126d92..c259828 100644
--- a/test/util/acquire.js
+++ b/test/util/acquire.js
@@ -8,5 +8,10 @@ for (const test of collect("syntax")) {
 }
 
 for (const test of collect("invalid", { expectError: true, raw: true })) {
-  fs.writeFileSync(test.baselinePath, `${test.error.message}\n`);
+  const content =
+    test.error ? test.error.message :
+    test.validation ? test.validation.join("\n") :
+    "";
+
+  fs.writeFileSync(test.baselinePath, `${content}\n`);
 }
diff --git a/test/util/collect.js b/test/util/collect.js
index f80f327..1b95177 100644
--- a/test/util/collect.js
+++ b/test/util/collect.js
@@ -1,6 +1,6 @@
 "use strict";
 
-const wp = require("../../lib/webidl2");
+const wp = require("../../index");
 const pth = require("path");
 const fs = require("fs");
 const jdp = require("jsondiffpatch");
@@ -20,7 +20,12 @@ function* collect(base, { expectError, raw } = {}) {
     try {
       const text = fs.readFileSync(path, "utf8");
       const ast = wp.parse(text);
-      yield new TestItem({ text, ast, path, raw });
+      const validation = wp.validate(ast);
+      if (validation) {
+        yield new TestItem({ text, ast, path, validation, raw });
+      } else {
+        yield new TestItem({ text, ast, path, raw });
+      }
     }
     catch (error) {
       if (expectError) {
@@ -35,11 +40,12 @@ function* collect(base, { expectError, raw } = {}) {
 
 
 class TestItem {
-  constructor({ text, ast, path, error, raw }) {
+  constructor({ text, ast, path, error, validation, raw }) {
     this.text = text;
     this.ast = ast;
     this.path = path;
     this.error = error;
+    this.validation = validation;
     const fileExtension = raw ? ".txt" : ".json";
     this.baselinePath = pth.join(
       pth.dirname(path),