Bug 896270 - Replace optional arguments with nullable types in protocol.js, part 2. r=jimb

toolkit/devtools/server/actors/inspector.js

@@ -596,8 +596,8 @@ var NodeListActor = exports.NodeListActor = protocol.ActorClass({
     }
   }, {
     request: {
-      start: Arg(0, "number", { optional: true }),
-      end: Arg(1, "number", { optional: true })
+      start: Arg(0, "nullable:number"),
+      end: Arg(1, "nullable:number")
     },
     response: RetVal("disconnectedNodeArray")
   }),
@@ -664,7 +664,7 @@ let traversalMethod = {
     whatToShow: Option(1)
   },
   response: {
-    node: RetVal("domnode", {optional: true})
+    node: RetVal("nullable:domnode")
   }
 }
 
@@ -839,7 +839,7 @@ var WalkerActor = protocol.ActorClass({
     let doc = node ? nodeDocument(node.rawNode) : this.rootDoc;
     return this._ref(doc);
   }, {
-    request: { node: Arg(0, "domnode", {optional: true}) },
+    request: { node: Arg(0, "nullable:domnode") },
     response: { node: RetVal("domnode") },
   }),
 
@@ -854,7 +854,7 @@ var WalkerActor = protocol.ActorClass({
     let elt = node ? nodeDocument(node.rawNode).documentElement : this.rootDoc.documentElement;
     return this._ref(elt);
   }, {
-    request: { node: Arg(0, "domnode", {optional: true}) },
+    request: { node: Arg(0, "nullable:domnode") },
     response: { node: RetVal("domnode") },
   }),
 
@@ -1357,7 +1357,7 @@ var WalkerActor = protocol.ActorClass({
     }
   }, {
     request: {
-      node: Arg(0, "domnode", { optional: true }),
+      node: Arg(0, "nullable:domnode")
     },
     response: {}
   }),
@@ -1412,7 +1412,7 @@ var WalkerActor = protocol.ActorClass({
       node: Arg(0, "domnode")
     },
     response: {
-      nextSibling: RetVal("domnode", { optional: true })
+      nextSibling: RetVal("nullable:domnode")
     }
   }),
 
@@ -1425,7 +1425,7 @@ var WalkerActor = protocol.ActorClass({
     request: {
       node: Arg(0, "domnode"),
       parent: Arg(1, "domnode"),
-      sibling: Arg(2, "domnode", { optional: true })
+      sibling: Arg(2, "nullable:domnode")
     },
     response: {}
   }),

toolkit/devtools/server/protocol.js

@@ -107,6 +107,17 @@ types.getType = function(type) {
   throw Error("Unknown type: " + type);
 }
 
+/**
+ * Don't allow undefined when writing primitive types to packets.  If
+ * you want to allow undefined, use a nullable type.
+ */
+function identityWrite(v) {
+  if (v === undefined) {
+    throw Error("undefined passed where a value is required");
+  }
+  return v;
+}
+
 /**
  * Add a type to the type system.
  *
@@ -138,8 +149,8 @@ types.addType = function(name, typeObject={}, options={}) {
   let type = object.merge({
     name: name,
     primitive: !(typeObject.read || typeObject.write),
-    read: v => v,
-    write: v => v
+    read: identityWrite,
+    write: identityWrite
   }, typeObject);
 
   registeredTypes.set(name, type);
@@ -384,32 +395,19 @@ types.JSON = types.addType("json");
  *    The argument index to place at this position.
  * @param type type
  *    The argument should be marshalled as this type.
- * @param object options
- *    Argument options:
- *      optional: true if the argument can be undefined or null.
  * @constructor
  */
 let Arg = Class({
-  initialize: function(index, type, options={}) {
+  initialize: function(index, type) {
     this.index = index;
     this.type = types.getType(type);
-    this.optional = !!options.optional;
   },
 
   write: function(arg, ctx) {
-    if (arg === undefined || arg === null) {
-      if (!this.optional) throw Error("Required argument " + this.name + " not specified.");
-      return undefined;
-    }
     return this.type.write(arg, ctx);
   },
 
   read: function(v, ctx, outArgs) {
-    if (v === undefined || v === null) {
-      if (!this.optional) throw Error("Required argument " + this.name + " not specified.");
-      outArgs[this.index] = v;
-      return;
-    }
     outArgs[this.index] = this.type.read(v, ctx);
   }
 });
@@ -466,34 +464,18 @@ exports.Option = Option;
  *
  * @param type type
  *    The return value should be marshalled as this type.
- * @param object options
- *    Argument options:
- *      optional: true if the argument can be undefined or null.
  */
 let RetVal = Class({
-  initialize: function(type, options={}) {
+  initialize: function(type) {
     this.type = types.getType(type);
-    this.optional = !!options.optional;
   },
 
   write: function(v, ctx) {
-    if (v !== undefined && v != null) {
-      return this.type.write(v, ctx);
-    }
-    if (!this.optional) {
-      throw Error("Return value not specified.");
-    }
-    return v;
+    return this.type.write(v, ctx);
   },
 
   read: function(v, ctx) {
-    if (v !== undefined && v != null) {
-      return this.type.read(v, ctx);
-    }
-    if (!this.optional) {
-      throw Error("Return value not specified.");
-    }
-    return v;
+    return this.type.read(v, ctx);
   }
 });
 

toolkit/devtools/server/tests/unit/test_protocol_simple.js

@@ -81,7 +81,7 @@ let RootActor = protocol.ActorClass({
   }, {
     request: {
       a: Arg(0),
-      b: Arg(1, "number", { optional: true })
+      b: Arg(1, "nullable:number")
     },
     response: {
       value: RetVal("number")
@@ -188,6 +188,11 @@ function run_test()
       trace.expectReceive({"value":1,"from":"<actorid>"});
       do_check_eq(ret, 1);
     }).then(() => {
+      // Missing argument should throw an exception
+      check_except(() => {
+        rootClient.simpleArgs(5);
+      });
+
       return rootClient.simpleArgs(5, 10)
     }).then(ret => {
       trace.expectSend({"type":"simpleArgs","firstArg":5,"secondArg":10,"to":"<actorid>"});