Repository: w3c/webidl2.js
Pull request #189 (2018-06-05 06:18:37)

BREAKING CHANGE: move nullable field from const type to .idlType
It's weird that `cnt.nullable` is `true` and then `cnt.idlType.nullable` is `false`.

Actually more weird thing is that the const type is allowed to be nullable (https://github.com/heycam/webidl/issues/448).

Also some refactorings in writer.js.

marcoscaceres: Are we jumping the shark here wrt heycam/webidl#448 ?
saschanaz: This PR is about moving the nullable feature into `idlType`, not removing it. Previously even a nullable const member type has `.idlType.nullable === false`, which is fairly unexpected.

And yeah, sorry for the misleading title 😞.
saschanaz: I hope that could be an answer. This PR still conforms to the standard but in more consistent way.

Patch:
diff --git a/lib/webidl2.js b/lib/webidl2.js
index 10c210b..12dfe74 100644
--- a/lib/webidl2.js
+++ b/lib/webidl2.js
@@ -503,14 +503,14 @@
 
     function const_() {
       if (!consume("const")) return;
-      const ret = { type: "const", nullable: false };
+      const ret = { type: "const" };
       let typ = primitive_type();
       if (!typ) {
         typ = consume(ID) || error("No type for const");
         typ = typ.value;
       }
       ret.idlType = Object.assign({ type: "const-type" }, EMPTY_IDLTYPE, { idlType: typ });
-      type_suffix(ret);
+      type_suffix(ret.idlType);
       const name = consume(ID) || error("No name for const");
       ret.name = name.value;
       consume("=") || error("No value assignment for const");
diff --git a/lib/writer.js b/lib/writer.js
index aeebe5b..a34c88c 100644
--- a/lib/writer.js
+++ b/lib/writer.js
@@ -37,7 +37,7 @@
         if (it.rhs.type === "identifier-list") ret += `=(${it.rhs.value.join(",")})`;
         else ret += `=${it.rhs.value}`;
       }
-      if (it.arguments) ret += `(${it.arguments.length ? it.arguments.map(argument).join(",") : ""})`;
+      if (it.arguments) ret += `(${it.arguments.map(argument).join(",")})`;
       return ret;
     };
     function extended_attributes(eats) {
@@ -48,7 +48,7 @@
     function operation(it) {
       let ret = extended_attributes(it.extAttrs);
       if (it.stringifier && !it.idlType) return "stringifier;";
-      for (const mod of ["getter",  "setter", "deleter", "stringifier", "static"]) {
+      for (const mod of ["getter", "setter", "deleter", "stringifier", "static"]) {
         if (it[mod]) ret += mod + " ";
       }
       ret += type(it.idlType) + " ";
@@ -110,7 +110,7 @@
     };
     function const_(it) {
       const ret = extended_attributes(it.extAttrs);
-      return `${ret}const ${type(it.idlType)}${it.nullable ? "?" : ""} ${it.name} = ${const_value(it.value)};`;
+      return `${ret}const ${type(it.idlType)} ${it.name} = ${const_value(it.value)};`;
     };
     function typedef(it) {
       let ret = extended_attributes(it.extAttrs);
@@ -127,27 +127,17 @@
     };
     function callback(it) {
       const ret = extended_attributes(it.extAttrs);
-      return `${ret}callback ${it.name} = ${type(it.idlType)}(${it.arguments.map(argument).join(",")});`;
+      const args = it.arguments.map(argument).join(",");
+      return `${ret}callback ${it.name} = ${type(it.idlType)}(${args});`;
     };
     function enum_(it) {
-      let ret = extended_attributes(it.extAttrs);
-      ret += `enum ${it.name} {`;
-      for (const v of it.values) {
-        ret += `"${v.value}",`;
-      }
-      return ret + "};";
-    };
-    function iterable(it) {
-      return `iterable<${it.idlType.map(type).join(", ")}>;`;
-    };
-    function legacyiterable(it) {
-      return `legacyiterable<${it.idlType.map(type).join(", ")}>;`;
-    };
-    function maplike(it) {
-      return `${it.readonly ? "readonly " : ""}maplike<${it.idlType.map(type).join(", ")}>;`;
+      const ext = extended_attributes(it.extAttrs);
+      const values = it.values.map(v => `"${v.value}",`).join("");
+      return `${ext}enum ${it.name} {${values}};`;
     };
-    function setlike(it) {
-      return `${it.readonly ? "readonly " : ""}setlike<${type(it.idlType[0])}>;`;
+    function iterable_like(it) {
+      const readonly = it.readonly ? "readonly " : "";
+      return `${readonly}${it.type}<${it.idlType.map(type).join(", ")}>;`;
     };
     function callbackInterface(it) {
       return `callback${interface_(it)}`;
@@ -167,10 +157,10 @@
       includes,
       callback,
       enum: enum_,
-      iterable,
-      legacyiterable,
-      maplike,
-      setlike,
+      iterable: iterable_like,
+      legacyiterable: iterable_like,
+      maplike: iterable_like,
+      setlike: iterable_like,
       "callback interface": callbackInterface
     };
     function dispatch(it) {


Test patch:
diff --git a/test/syntax/json/constants.json b/test/syntax/json/constants.json
index bc8e7d0..9446673 100644
--- a/test/syntax/json/constants.json
+++ b/test/syntax/json/constants.json
@@ -6,7 +6,6 @@
         "members": [
             {
                 "type": "const",
-                "nullable": false,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
@@ -24,7 +23,6 @@
             },
             {
                 "type": "const",
-                "nullable": false,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
@@ -42,7 +40,6 @@
             },
             {
                 "type": "const",
-                "nullable": false,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
@@ -60,7 +57,6 @@
             },
             {
                 "type": "const",
-                "nullable": false,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
@@ -78,7 +74,6 @@
             },
             {
                 "type": "const",
-                "nullable": false,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
@@ -96,7 +91,6 @@
             },
             {
                 "type": "const",
-                "nullable": false,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
@@ -114,7 +108,6 @@
             },
             {
                 "type": "const",
-                "nullable": false,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
@@ -132,7 +125,6 @@
             },
             {
                 "type": "const",
-                "nullable": false,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
diff --git a/test/syntax/json/nullable.json b/test/syntax/json/nullable.json
index 43c231f..6745a26 100644
--- a/test/syntax/json/nullable.json
+++ b/test/syntax/json/nullable.json
@@ -6,11 +6,10 @@
         "members": [
             {
                 "type": "const",
-                "nullable": true,
                 "idlType": {
                     "type": "const-type",
                     "generic": null,
-                    "nullable": false,
+                    "nullable": true,
                     "union": false,
                     "idlType": "boolean",
                     "extAttrs": []