Add a MapRule

pkg/serialization/lib/src/serialization_rule.dart

Index: pkg/serialization/lib/src/serialization_rule.dart
diff --git a/pkg/serialization/lib/src/serialization_rule.dart b/pkg/serialization/lib/src/serialization_rule.dart
index b040796668570f0e04db697e77eca4f28682fd89..17b832b9c1fa266babf5a46d5635790d51ffa823 100644
--- a/pkg/serialization/lib/src/serialization_rule.dart
+++ b/pkg/serialization/lib/src/serialization_rule.dart
@@ -28,7 +28,7 @@ abstract class SerializationRule {
    * be identified within it by number.
    */
   void set number(x) {
-    if (_number != null) throw
+    if (_number != null && _number != x) throw
         new SerializationException("Rule numbers cannot be changed, once set");
     _number = x;
   }
@@ -48,23 +48,32 @@ abstract class SerializationRule {
   extractState(object, void f(value));
 
   /**
+   * Allows rules to tell us how they expect to store their state. If this
+   * isn't specified we can also just look at the data to tell.
+   */
+  bool get storesStateAsLists => false;
+  bool get storesStateAsMaps => false;
+  bool get storesStateAsPrimitives => false;
+
+  /**
    * Given the variables representing the state of an object, flatten it
    * by turning object pointers into Reference objects where needed. This
    * destructively modifies the state object.
    *
    * This has a default implementation which assumes that object is indexable,
    * so either conforms to Map or List. Subclasses may override to do something
-   * different.
+   * different, including returning a new state object to be used in place
+   * of the original.
    */
   // This has to be a separate operation from extracting, because we extract
   // as we are traversing the objects, so we don't yet have the objects to
   // generate references for them. It might be possible to avoid that by
   // doing a depth-first rather than breadth-first traversal, but I'm not
   // sure it's worth it.
-  void flatten(state, Writer writer) {
+  flatten(state, Writer writer) {
     keysAndValues(state).forEach((key, value) {
       var reference = writer._referenceFor(value);
-      state[key] = (reference == null) ? value : reference;
+      state[key] = reference;
     });
   }
 
@@ -131,7 +140,7 @@ class ListRule extends SerializationRule {
 
   appliesTo(object, Writer w) => object is List;
 
-  state(List list) => new List.from(list);
+  bool get storesStateAsLists => true;
 
   List extractState(List list, f) {
     var result = new List();
@@ -180,6 +189,82 @@ class ListRuleEssential extends ListRule {
 }
 
 /**
+ * This rule handles things that implement Map. It will recreate them as
+ * whatever the default implemenation of Map is on the target platform. If a
+ * map has string keys it will attempt to retain it as a map for JSON formats,
+ * otherwise it will store it as a list of references to keys and values.
+ */
+class MapRule extends SerializationRule {
+
+  appliesTo(object, Writer w) => object is Map;
+
+  bool get storesStateAsMaps => true;
+
+  extractState(Map map, f) {
+    // Note that we make a copy here because flattening may be destructive.
+    var newMap = new Map.from(map);
+    newMap.forEach((key, value) {
+      f(key);
+      f(value);
+    });
+    return newMap;
+  }
+
+  /**
+   * Change the keys and values of [state] into references in [writer].
+   * If [state] is a map whose keys are all strings then we leave the keys
+   * as is so that JSON formats will be more readable. If the keys are
+   * arbitrary then we need to turn them into references.
+   */
+  flatten(Map state, Writer writer) {
+    bool keysAreAllStrings = state.keys.every((x) => x is String);

[Jennifer Messerly, 2013/01/25 01:19:59]

out of curiousity, why do we check all keys? Could we check each key individual
and preserve strings when possible?

[Alan Knight, 2013/01/26 01:33:38]

I don't think that would work, at least not with the current state of the JSON
support. A lot of the point of retaining things in Map form is to make readable
JSON. But the JSON support requires any Maps in its arguments to have string
keys, and will fail otherwise. So if we see any non-string keys we need to turn
the whole thing into a List instead.

+    if (keysAreAllStrings) {
+      keysAndValues(state).forEach(

[Jennifer Messerly, 2013/01/25 01:19:59]

I would have expected a return in both branches. Could we "return state;" here?

[Alan Knight, 2013/01/26 01:33:38]

Sure. The normal case for flatten() is that it destructively modifies the
existing state, and returning null is interpreted as just meaning to use the old
one. But consistently returning it in both cases probably makes the intention
clearer, so doing that.

+          (key, value) => state[key] = writer._referenceFor(value));
+    } else {
+      var newState = [];
+      keysAndValues(state).forEach((key, value) {
+        newState.add(writer._referenceFor(key));
+        newState.add(writer._referenceFor(value));
+      });
+      return newState;
+    }
+  }
+
+  inflateEssential(state, Reader r) => new Map();
+
+  // For a map, we consider all of its state non-essential and add it
+  // after creation.
+  inflateNonEssential(state, Map newMap, Reader r) {
+    if (state is List) {
+      inflateNonEssentialFromList(state, newMap, r);
+    } else {
+      inflateNonEssentialFromMap(state, newMap, r);
+    }
+  }
+
+  void inflateNonEssentialFromList(List state, Map newMap, Reader r) {
+    var key;
+    for (var each in state) {
+      if (key == null) {
+        key = each;
+      } else {
+        newMap[r.inflateReference(key)] = r.inflateReference(each);
+        key = null;
+      }
+    }
+  }
+
+  void inflateNonEssentialFromMap(Map state, Map newMap, Reader r) {
+    state.forEach((key, value) {
+      newMap[r.inflateReference(key)] = r.inflateReference(value);
+    });
+  }
+
+  bool get hasVariableLengthEntries => true;
+}
+
+/**
  * This rule handles primitive types, defined as those that we can normally
  * represent directly in the output format. We hard-code that to mean
  * num, String, and bool.
@@ -189,10 +274,12 @@ class PrimitiveRule extends SerializationRule {
     return isPrimitive(object);
   }
   extractState(object, Function f) => object;
-  void flatten(object, Writer writer) {}
+  flatten(object, Writer writer) {}
   inflateEssential(state, Reader r) => state;
   inflateNonEssential(object, _, Reader r) {}
 
+  bool get storesStateAsPrimitives => true;
+
   /**
    * Indicate whether we should save pointers to this object as references
    * or store the object directly. For primitives this depends on the format,
@@ -274,7 +361,7 @@ class NamedObjectRule extends SerializationRule {
   // reference to that. But that requires adding the Writer as a parameter to
   // extractState, and I'm reluctant to add yet another parameter until
   // proven necessary.
-  void flatten(state, Writer writer) {
+  flatten(state, Writer writer) {
     state[0] = nameFor(state.first, writer);
   }
 
@@ -445,8 +532,8 @@ class _LazyList extends Iterable implements List {
   sort([f]) => _throw();
   clear() => _throw();
   removeAt(x) => _throw();
-  removeLast() => _throw();
   remove(x) => _throw();
+  removeLast() => _throw();
   removeAll(x) => _throw();
   retainAll(x) => _throw();
   removeMatching(x) => _throw();
@@ -455,6 +542,6 @@ class _LazyList extends Iterable implements List {
   setRange(x, y, z, [a]) => _throw();
   removeRange(x, y) => _throw();
   insertRange(x, y, [z]) => _throw();
+  get reversed => _throw();
   void set length(x) => _throw();
-  List get reversed => _throw();
 }