JSON export/import in generate-bindings.pl

Source/bindings/scripts/IRToFromJSON.pm

+# Copyright (C) 2013 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+# Library to convert the Perl Intermediate Representation (IR) of IDLs
+# to and from JSON, for:
+# 1. Modularity between frontend parser and backend code generator;
+# 2. Porting to Python, so can connect Perl scripts and Python scripts.
+
+package IRToFromJSON;

[haraken, 2013/06/12 03:20:21]

I'd call this JSONSerializer.pm. IR sounds unclear.

[Nils Barth (inactive), 2013/06/12 03:57:43]

Done.

+
+use strict;
+use warnings;
+
+require Exporter;
+use vars qw(@ISA @EXPORT_OK);
+@ISA = qw(Exporter);
+@EXPORT_OK = qw(IRToJSON JSONToIR);

[haraken, 2013/06/12 03:20:21]

I'd remove the code for controlling exports. You can just expose everything just
like preprocessor.pm. This script is anyway a temporal one. Let's keep it
simple.

[Nils Barth (inactive), 2013/06/12 03:57:43]

Done.

+
+use Class::Struct;
+use JSON -convert_blessed_universally;  # IR contains blessed references
+use Data::Dumper;

[haraken, 2013/06/12 03:20:21]

Remove this.

[Nils Barth (inactive), 2013/06/12 03:57:43]

Oops (>.<)
Done.

+
+use IDLParser;

[haraken, 2013/06/12 03:20:21]

Remove this.

[Nils Barth (inactive), 2013/06/12 03:57:43]

Ok.
Needed so we have classes for object construction.
Ok to remove for now b/c this is only called from generate-bindings.pl, which
already uses IDLParser, but if we split generate-bindings.pl into Parser +
CodeGenerator pieces we'll need it again for the CodeGenerator piece.

+
+
+sub IRToJSON

[haraken, 2013/06/12 03:20:21]

SerializeJSON might be a better name.

[Nils Barth (inactive), 2013/06/12 03:57:43]

Done.

+{
+    my $document = shift;
+    my $json = JSON->new->utf8;
+    # Blessed references for objects, canonical to order (so can compare)

[dominicc (has gone to gerrit), 2013/06/11 22:56:30]

What does this mean?

"Blessed references for objects"--which objects? Blessed as?

"canonical to order"--what does it mean? I understand the words but not the
phrase. I think this can be stated more simply.

"so can compare"--cryptic. I doubt most readers know what you mean.

[Nils Barth (inactive), 2013/06/12 02:56:12]

Clarified.

+    $json = $json->allow_blessed->convert_blessed->canonical();
+    return $json->encode($document);
+}
+
+sub rawIRToCleanIR

[dominicc (has gone to gerrit), 2013/06/11 22:56:30]

How about a brief comment (one line) explaining what this does in a way that
makes it clear what "raw" and "clean" are?

For pedagogy it might be better to keep IRToJSON and JSONToIR together, and have
this function last.

[Nils Barth (inactive), 2013/06/12 02:56:12]

I've renamed "rawIR" and "cleanIR" to "jsonIR" and "perlIR" which should clarify
some.
I've added a (two-line) comment explaining that we're rebuilding objects;
I could make it one line, but it would be a bit terse.
Done.

+{
+    my $rawIR = shift;
+    if (ref $rawIR eq "ARRAY") {
+        my $cleanIR = [];
+        foreach my $element (@$rawIR) {
+            push(@$cleanIR, rawIRToCleanIR($element));
+        }
+        return $cleanIR;
+    }
+    if (ref $rawIR eq "HASH") {
+        if (!scalar %{$rawIR}) {
+            return {};
+        }
+        # overloadedIndex is not part of domFunction struct,
+        # (just a hash key, not an object member), so special case
+        my $overload = -1;
+        my $classAndKeyFirst = (keys %{$rawIR})[0];

[dominicc (has gone to gerrit), 2013/06/11 22:56:30]

I think this code can be made much simpler.

This variable name is misleading. It is really "classAndFirstKey".

However sometimes it does not contain a class. I think a few blank lines for
grouping and more intensional variable names would make this easier to
understand.

For example testing for '::' and putting all of the handling of class/object
together would make it clearer what is going on.

Some helper functions would also simplify the control flow:

if (... is X ...) {
  return do the thing for X();
} elsif (... is Y ...) {
  return do the thing for Y();
} ... etc.

Is it just a coincidence that all class names come before 'o' of
'overloadedIndex' in sort order? If so relying on that seems gross.

[Nils Barth (inactive), 2013/06/12 02:56:12]

Rewritten, with helper function.
It's still a bit ugly, due to special case, but should be clearer.
Yes, but we don't rely on that.

overloadedIndex only occurs in domFunction, where it's alphabetically after the
other keys (domFunction::key1 ...).
This is the order in the JSON, since we've specified that JSON is written in
alphabetical order.
However, key order in Perl is not guaranteed, hence test for this.

We could use a "sort(keys ...)" instead to ensure overloadedIndex never comes
first, but that adds the above assumption.
We don't rely on it, which is why there's a test:
if overloadedIndex comes first, skip over it.

We do rely on there being only one special case (overloadedIndex),
and on overloadedIndex not being the only key (it's added to an object with
other attributes, so it's never the only key), but we don't make any assumptions
about ordering.

+        if ($classAndKeyFirst eq "overloadedIndex") {

[dominicc (has gone to gerrit), 2013/06/11 22:56:30]

Why special-case overloadedIndex *only* if it is the first one?

[Nils Barth (inactive), 2013/06/12 02:56:12]

We're just looking for the class name here, so we just need to skip over to the
next key, which will have the class name (if any).
We also check for overloaded within the loop, since it's not an object member.

+            $overload = $rawIR->{$classAndKeyFirst};
+            $classAndKeyFirst = (keys %{$rawIR})[1];
+        }
+        (my $class, my $firstKey) = (split('::', $classAndKeyFirst));
+        my $keysValuesRef = {};
+        if ($class ne $classAndKeyFirst) {  # object

[dominicc (has gone to gerrit), 2013/06/11 22:56:30]

I'm having trouble understanding this. It looks like you're comparing the wrong
kinds of things here since "class" and "classAndKey..." look like different
kinds of things.

[Nils Barth (inactive), 2013/06/12 02:56:12]

This tests if the splitting achieved anything (i.e., is it actually A::B?).
It's clearer as a regex match (=~ /::/) so I've rewritten as such.

+            foreach my $classAndKey (keys %{$rawIR}) {
+                if ($classAndKey eq "overloadedIndex") {
+                    $overload = $rawIR->{$classAndKey};
+                } else {
+                    (my $newClass, my $key) = split('::', $classAndKey);
+                    $keysValuesRef->{$key} = rawIRToCleanIR($rawIR->{$classAndKey});
+                }
+            }
+            my $object = $class->new(%$keysValuesRef);  # Build object
+            if ($overload != -1) {
+                $object->{overloadedIndex} = $overload;
+            }
+            return $object;
+        } else {  # just hash
+            foreach my $key (keys %{$rawIR}) {
+                $keysValuesRef->{$key} = rawIRToCleanIR($rawIR->{$key});
+            }
+            return $keysValuesRef;
+        }
+    }
+    die "Unexpected reference type: " . ref $rawIR . "\n" if ref $rawIR;
+    return $rawIR;
+}
+
+sub JSONToIR

[haraken, 2013/06/12 03:20:21]

deserializeJSON might be a better name.

+{
+    my $jsonText = shift;
+    my $json = JSON->new->utf8;
+    my $rawIR = $json->decode($jsonText);
+    return rawIRToCleanIR($rawIR);
+}
+
+1;