I'd like you to do a code review. To review this change, run gvn review --project https://google-caja.googlecode.com/svn mikesamuel/[EMAIL PROTECTED]
Alternatively, to review the latest snapshot of this change branch, run gvn --project https://google-caja.googlecode.com/svn review mikesamuel/structural-assert-consistent to review the following change: *mikesamuel/[EMAIL PROTECTED] | mikesamuel | 2008-10-15 20:20:37 -0800 (Wed, 15 Oct 2008) Description: Issue 824: toString shouldn't be used in tests to structural equivalence Doing an assertConsistent comparing two pieces of code that should produce [1, 2], we need two strings to compare. Currently, many of our test cases compare these using xxx.toString() or ''+xxx. However, this will just [1, 2] as consistent with [[1, 2]] and [[1], [2]] etc. Once JSON.stringify is available to cajoled code, we should use that. Affected Paths: M //trunk/tests/com/google/caja/CajitaTest.java M //trunk/tests/com/google/caja/parser/quasiliteral/CajitaRewriterTest.java M //trunk/tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTestCase.java M //trunk/tests/com/google/caja/parser/quasiliteral/DefaultValijaRewriterTest.java M //trunk/tests/com/google/caja/parser/quasiliteral/RewriterTestCase.java A //trunk/tests/com/google/caja/util/RhinoAsserts.java A //trunk/tests/com/google/caja/util/RhinoAssertsTest.java M //trunk/tests/com/google/caja/util/RhinoTestBed.java This is a semiautomated message from "gvn mail". See <http://code.google.com/p/gvn/> to learn more. Index: tests/com/google/caja/CajitaTest.java =================================================================== --- tests/com/google/caja/CajitaTest.java (^/trunk/tests/com/google/caja/[EMAIL PROTECTED]) +++ tests/com/google/caja/CajitaTest.java (^/changes/mikesamuel/structural-assert-consistent/trunk/tests/com/google/caja/[EMAIL PROTECTED]) @@ -31,7 +31,7 @@ public class CajitaTest extends CajaTestCase { + "___.asSimpleFunc(o.f);" + "assertEquals(42, ___.construct(o.f, [42]).x);"); } - + public void testGrantGeneric() throws Exception { runTest( " function A() {} function B() {} function C() {}" Index: tests/com/google/caja/parser/quasiliteral/CajitaRewriterTest.java =================================================================== --- tests/com/google/caja/parser/quasiliteral/CajitaRewriterTest.java (^/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) +++ tests/com/google/caja/parser/quasiliteral/CajitaRewriterTest.java (^/changes/mikesamuel/structural-assert-consistent/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) @@ -103,7 +103,7 @@ public class CajitaRewriterTest extends CommonJsRe assertConsistent( "var z={toString:function(){return 'blah';}};" + "try {" + - " ''+z;" + + " '' + z;" + "} catch (e) {" + " throw new Error('PlusPlus error: ' + e);" + "}"); @@ -138,7 +138,7 @@ public class CajitaRewriterTest extends CommonJsRe } public void testInitializeMap() throws Exception { - assertConsistent("var zerubabel={bobble:2, apple:1}; zerubabel.apple;"); + assertConsistent("var zerubabel = {bobble:2, apple:1}; zerubabel.apple;"); } public void testValueOf() throws Exception { @@ -231,17 +231,19 @@ public class CajitaRewriterTest extends CommonJsRe public void testReflectiveMethodInvocation() throws Exception { assertConsistent( - "(function (first, second){return 'a'+first+'b'+second;}).call([],8,9);"); + "(function (first, second) { return 'a' + first + 'b' + second; })" + + ".call([], 8, 9);"); assertConsistent( - "var a=[]; [].push.call(a, 5, 6); a.join(',');"); + "var a = []; [].push.call(a, 5, 6); a;"); assertConsistent( - "(function (a,b){return 'a'+a+'b'+b;}).apply([],[8,9]);"); + "(function (a, b) { return 'a' + a + 'b' + b; }).apply([], [8, 9]);"); assertConsistent( - "var a=[]; [].push.apply(a, [5, 6]); a.join(',');"); + "var a = []; [].push.apply(a, [5, 6]); a;"); assertConsistent( - "[].sort.apply([6,5]).join('');"); + "[].sort.apply([6, 5]);"); assertConsistent( - "(function (first, second){return 'a'+first+'b'+second;}).bind([],8)(9);"); + "(function (first, second) { return 'a' + first + 'b' + second; })" + + ".bind([], 8)(9);"); } /** @@ -515,7 +517,7 @@ public class CajitaRewriterTest extends CommonJsRe " ___.readPub(g, 3);" + " }" + "}"); - assertConsistent( + rewriteAndExecute( "var handled = false;" + "try {" + " throw null;" + @@ -524,7 +526,7 @@ public class CajitaRewriterTest extends CommonJsRe " handled = true;" + "}" + "assertTrue(handled);"); // Control reached and left the catch block. - assertConsistent( + rewriteAndExecute( "var handled = false;" + "try {" + " throw undefined;" + @@ -533,7 +535,7 @@ public class CajitaRewriterTest extends CommonJsRe " handled = true;" + "}" + "assertTrue(handled);"); - assertConsistent( + rewriteAndExecute( "var handled = false;" + "try {" + " throw true;" + @@ -542,7 +544,7 @@ public class CajitaRewriterTest extends CommonJsRe " handled = true;" + "}" + "assertTrue(handled);"); - assertConsistent( + rewriteAndExecute( "var handled = false;" + "try {" + " throw 37639105;" + @@ -551,7 +553,7 @@ public class CajitaRewriterTest extends CommonJsRe " handled = true;" + "}" + "assertTrue(handled);"); - assertConsistent( + rewriteAndExecute( "var handled = false;" + "try {" + " throw 'panic';" + @@ -560,7 +562,7 @@ public class CajitaRewriterTest extends CommonJsRe " handled = true;" + "}" + "assertTrue(handled);"); - assertConsistent( + rewriteAndExecute( "var handled = false;" + "try {" + " throw new Error('hello');" + @@ -1015,7 +1017,7 @@ public class CajitaRewriterTest extends CommonJsRe assertConsistent("var x = 3; x *= 2;"); assertConsistent("var x = 1; x += 7;"); assertConsistent("var x = 1; x /= '2';"); - assertConsistent("var o = { x: 'a' }; o.x += 'b';"); + assertConsistent("var o = { x: 'a' }; o.x += 'b'; o;"); EnumSet<Operator> ops = EnumSet.of( Operator.ASSIGN_MUL, @@ -1071,12 +1073,12 @@ public class CajitaRewriterTest extends CommonJsRe "var x = 2;" + "var arr = [--x, x, x--, x, ++x, x, x++, x];" + "assertEquals('1,1,1,0,1,1,1,2', arr.join(','));" + - "arr.join(',');"); + "arr;"); assertConsistent( "var x = '2';" + "var arr = [--x, x, x--, x, ++x, x, x++, x];" + "assertEquals('1,1,1,0,1,1,1,2', arr.join(','));" + - "arr.join(',');"); + "arr;"); } public void testSetIncrDecrOnLocals() throws Exception { @@ -1091,7 +1093,7 @@ public class CajitaRewriterTest extends CommonJsRe " var x = 2;" + " var arr = [--x, x, x--, x, ++x, x, x++, x];" + " assertEquals('1,1,1,0,1,1,1,2', arr.join(','));" + - " return arr.join(',');" + + " return arr;" + "})();"); } @@ -1114,7 +1116,7 @@ public class CajitaRewriterTest extends CommonJsRe " var o = { x: 2 };" + " var arr = [--o.x, o.x, o.x--, o.x, ++o.x, o.x, o.x++, o.x];" + " assertEquals('1,1,1,0,1,1,1,2', arr.join(','));" + - " return arr.join(',');" + + " return arr;" + "})();"); } @@ -1127,7 +1129,7 @@ public class CajitaRewriterTest extends CommonJsRe " assertEquals(2, j);" + " assertEquals(1, arrs[0]);" + " assertEquals(4, arrs[1]);" + - " return arrs.join(',');" + + " return arrs;" + "})();"); assertConsistent( "(function () {" + @@ -1142,7 +1144,7 @@ public class CajitaRewriterTest extends CommonJsRe " }" + " };" + " })();" + - " foo()[foo()] -= foo();" + + " return foo()[foo()] -= foo();" + "})();" ); } @@ -1188,7 +1190,7 @@ public class CajitaRewriterTest extends CommonJsRe "var o = { a: 1 };" + "delete o[alert];" + "assertEquals(undefined, o.a);" + - "o.a;"); + "o;"); } public void testDeleteFails() throws Exception { @@ -1467,7 +1469,7 @@ public class CajitaRewriterTest extends CommonJsRe assertConsistent("[ (({}) instanceof Object)," + " ((new Date) instanceof Date)," + " (({}) instanceof Date)" + - "].toString();"); + "];"); assertConsistent("function foo() {}; (new foo) instanceof foo;"); assertConsistent("function foo() {}; !(({}) instanceof foo);"); } @@ -1598,19 +1600,19 @@ public class CajitaRewriterTest extends CommonJsRe "var arr = [1, 2, 3], k = -1;" + "(function () {" + " var a = arr[++k], b = arr[++k], c = arr[++k];" + - " return [a, b, c].join(',');" + + " return [a, b, c];" + "})();"); // Check exceptions on read of uninitialized variables. assertConsistent( "(function () {" + " var a = [];" + " for (var i = 0, j = 10; i < j; ++i) { a.push(i); }" + - " return a.join(',');" + + " return a;" + "})();"); assertConsistent( "var a = [];" + "for (var i = 0, j = 10; i < j; ++i) { a.push(i); }" + - "a.join(',');"); + "a;"); } public void testRecurseParseTreeNodeContainer() throws Exception { @@ -1791,9 +1793,11 @@ public class CajitaRewriterTest extends CommonJsRe } public void testAssertConsistent() throws Exception { + // Since we test structurally, this works. + assertConsistent("({})"); try { - // A value that cannot be consistent across invocations. - assertConsistent("({})"); + // But this won't. + assertConsistent("typeof (new RegExp('foo'))"); } catch (AssertionFailedError e) { // Pass return; Index: tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTestCase.java =================================================================== --- tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTestCase.java (^/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) +++ tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTestCase.java (^/changes/mikesamuel/structural-assert-consistent/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) @@ -226,34 +226,35 @@ public abstract class CommonJsRewriterTestCase ext "var x = [33];" + "x.foo = [].push;" + "x.foo.call(x, 44);" + - "x.toString();"); + "x;"); assertConsistent( "var x = [33];" + "x.foo = [].push;" + "x.foo.apply(x, [6,7,8]);" + - "x.toString();"); + "x;"); assertConsistent( "var x = [33];" + "x.foo = [].push;" + "x.foo.bind(x)(6,7,8);" + - "x.toString();"); + "x;"); assertConsistent( "var x = [33];" + "x.foo = [].push;" + "x.foo.bind(x,6)(7,8);" + - "x.toString();"); + "x;"); assertConsistent( "[].push.length;"); assertConsistent( "var x = {blue:'green'};" + "x.foo = [].push;" + "x.foo.call(x, 44);" + - "x.toString();"); + "delete x.foo;" + + "x;"); assertConsistent( "var x = {blue:'green'};" + "x.foo = [].push;" + "x.foo.call(x, 44);" + - "cajita.getOwnPropertyNames(x).sort().toString();"); + "cajita.getOwnPropertyNames(x).sort();"); } public void testTypeofConsistent() throws Exception { @@ -266,7 +267,7 @@ public abstract class CommonJsRewriterTestCase ext " (typeof (function () {}))," + " (typeof { x: 4.0 }.x)," + " (typeof { 2: NaN }[1 + 1])" + - "].toString();"); + "];"); rewriteAndExecute("assertEquals(typeof new RegExp('.*'), 'object');"); } } Index: tests/com/google/caja/parser/quasiliteral/DefaultValijaRewriterTest.java =================================================================== --- tests/com/google/caja/parser/quasiliteral/DefaultValijaRewriterTest.java (^/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) +++ tests/com/google/caja/parser/quasiliteral/DefaultValijaRewriterTest.java (^/changes/mikesamuel/structural-assert-consistent/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) @@ -50,9 +50,9 @@ public class DefaultValijaRewriterTest extends Com } public void testProtoCall() throws Exception { - assertConsistent("'' + Array.prototype.sort.call([3, 1, 2]);"); - assertConsistent("'' + [3, 1, 2].sort();"); - assertConsistent("'' + [3, 1, 2].sort.call([4, 2, 7]);"); + assertConsistent("Array.prototype.sort.call([3, 1, 2]);"); + assertConsistent("[3, 1, 2].sort();"); + assertConsistent("[3, 1, 2].sort.call([4, 2, 7]);"); assertConsistent("String.prototype.indexOf.call('foo', 'o');"); assertConsistent("'foo'.indexOf('o');"); @@ -101,12 +101,12 @@ public class DefaultValijaRewriterTest extends Com } public void testArray() throws Exception { - assertConsistent("[3, 2, 1].sort().toString();"); - assertConsistent("[3, 2, 1].sort.call([4, 2, 7]).toString();"); + assertConsistent("[3, 2, 1].sort();"); + assertConsistent("[3, 2, 1].sort.call([4, 2, 7]);"); } public void testObject() throws Exception { - assertConsistent("({ x: 1, y: 2 }).toString();"); + assertConsistent("({ x: 1, y: 2 });"); } public void testIndexOf() throws Exception { @@ -158,7 +158,7 @@ public class DefaultValijaRewriterTest extends Com + "assertEquals('number', typeof time);"); } - public void testMultiDeclaration() throws Exception { + public void testMultiDeclarationDefinitionOrder() throws Exception { rewriteAndExecute("var a, b, c;"); rewriteAndExecute( "" @@ -168,8 +168,9 @@ public class DefaultValijaRewriterTest extends Com public void testDelete() throws Exception { assertConsistent( - "(function () { var a = { x: 1 }; delete a.x; return a.x; })();"); - assertConsistent("var a = { x: 1 }; delete a.x; a.x;"); + "(function () { var a = { x: 1 }; delete a.x; return typeof a.x; })();" + ); + assertConsistent("var a = { x: 1 }; delete a.x; typeof a.x;"); } public void testIn() throws Exception { @@ -180,7 +181,7 @@ public class DefaultValijaRewriterTest extends Com "})();"); assertConsistent( "var a = { x: 1 };\n" + - "'' + ('x' in a) + ('y' in a);"); + "[('x' in a), ('y' in a)];"); } public void testForIn2() throws Exception { @@ -270,7 +271,7 @@ public class DefaultValijaRewriterTest extends Com } public void testStatic() throws Exception { - assertConsistent("'' + Array.slice([3, 4, 5, 6], 1);"); + assertConsistent("Array.slice([3, 4, 5, 6], 1);"); } public void testConcatArgs() throws Exception { @@ -285,20 +286,20 @@ public class DefaultValijaRewriterTest extends Com "var x = [33];" + "x.foo = [].push;" + "x.foo(44);" + - "x.toString();"); + "x;"); assertConsistent( "var x = {blue:'green'};" + "x.foo = [].push;" + "x.foo(44);" + - "cajita.getOwnPropertyNames(x).sort().toString();"); + "cajita.getOwnPropertyNames(x).sort();"); assertConsistent( "var x = [33];" + "Array.prototype.push.apply(x, [3,4,5]);" + - "x.toString();"); + "x;"); assertConsistent( "var x = {blue:'green'};" + "Array.prototype.push.apply(x, [3,4,5]);" + - "cajita.getOwnPropertyNames(x).sort().toString();"); + "cajita.getOwnPropertyNames(x).sort();"); } @Override Index: tests/com/google/caja/parser/quasiliteral/RewriterTestCase.java =================================================================== --- tests/com/google/caja/parser/quasiliteral/RewriterTestCase.java (^/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) +++ tests/com/google/caja/parser/quasiliteral/RewriterTestCase.java (^/changes/mikesamuel/structural-assert-consistent/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) @@ -28,6 +28,7 @@ import com.google.caja.parser.js.Reference; import com.google.caja.parser.js.Statement; import com.google.caja.parser.js.SyntheticNodes; import com.google.caja.util.CajaTestCase; +import com.google.caja.util.RhinoAsserts; import com.google.caja.util.TestUtil; import com.google.caja.reporting.MessageLevel; import com.google.caja.reporting.Message; @@ -216,12 +217,20 @@ public abstract class RewriterTestCase extends Caj throws IOException, ParseException { Object plainResult = executePlain(caja); Object rewrittenResult = rewriteAndExecute(caja); + + String plainRepr = RhinoAsserts.structuralForm(plainResult); + String rewrittenRepr = RhinoAsserts.structuralForm(rewrittenResult); + if ("undefined".equals(plainRepr)) { + // This usually indicates an error, such as failing to return a value. + fail("Consistency check returned undefined"); + } + System.err.println( "Results: " - + "plain=<" + plainResult + "> " - + "rewritten=<" + rewrittenResult + "> " + + "plain=<" + plainRepr + "> " + + "rewritten=<" + rewrittenRepr + "> " + "for " + getName()); - assertEquals(message, plainResult, rewrittenResult); + assertEquals(message, plainRepr, rewrittenRepr); } protected final <T extends ParseTreeNode> T syntheticTree(T node) { Index: tests/com/google/caja/util/RhinoAsserts.java =================================================================== --- tests/com/google/caja/util/RhinoAsserts.java (added) +++ tests/com/google/caja/util/RhinoAsserts.java (^/changes/mikesamuel/structural-assert-consistent/trunk/tests/com/google/caja/util/[EMAIL PROTECTED]) @@ -0,0 +1,301 @@ +// Copyright (C) 2008 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.caja.util; + +import com.google.caja.lexer.escaping.Escaping; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; + +import org.mozilla.javascript.NativeArray; +import org.mozilla.javascript.Scriptable; +import org.mozilla.javascript.Undefined; + +/** + * Utilities for dealing with values produced by the Rhino JS interpreter. + * + * @author [EMAIL PROTECTED] + */ +public final class RhinoAsserts { + + /** + * Coerces a JavaScript object graph to a canonical form that can be compared + * to another object graph for equivalence across JavaScript execution + * contexts. + * <p> + * Properties ending in two-underscores are not considered significant, and + * non-element properties on Arrays are not considered significant. + * <p> + * No attempt is made to serialize functions to a structural form, and since + * functions can't be compared, neither can instances of user defined classes. + * + * @param o the result of a JavaScript computation. + * @return a string that would be recognized by SpiderMonkey's eval. + * This form differs from JSON in that it uses SpiderMonkey's [EMAIL PROTECTED] uneval} + * result format to deal with objects that have multiple in-bound edges. + */ + public static String structuralForm(Object o) { + final Map<Object, VisitationRecord> visited + = new IdentityHashMap<Object, VisitationRecord>(); + + // Walk the object graph and figure out which objects have multiple in-bound + // edges, and assign a number to them. + walk(o, new JsObjVisitor() { + @Override + void visitArray(Scriptable s, Object[] values) { + if (visit(s)) { + for (Object value : values) { walk(value, this); } + } + } + @Override + void visitObject(Scriptable s, List<Pair<String, Object>> props) { + if (visit(s)) { + for (Pair<String, Object> prop : props) { + walk(prop.b, this); + } + } + } + + int counter = 0; + private boolean visit(Object o) { + VisitationRecord r = visited.get(o); + if (r == null) { + visited.put(o, r = new VisitationRecord()); + return true; + } else { + if (r.key == 0) { r.key = ++counter; } + return false; + } + } + }); + + // Now that we know which object's need to have a numeric label attached + // we can serialize them to a String. + final StringBuilder sb = new StringBuilder(); + walk(o, new JsObjVisitor() { + // Primitives are rendered as in JavaScript. + @Override + void visitString(String s) { + sb.append('"'); + Escaping.escapeJsString(s, true, false, sb); + sb.append('"'); + } + @Override + void visitNumber(Number n) { + sb.append(n); + } + @Override + void visitBoolean(Boolean b) { + sb.append(b); + } + @Override + void visitNull() { sb.append("null"); } + @Override + void visitUndefined() { sb.append("undefined"); } + // We serialize Arrays using the abbreviated syntax, and ignore + // non-integral keys. + @Override + void visitArray(Scriptable s, Object[] values) { + if (checkDupePrefix(s)) { + sb.append('['); + for (int i = 0, n = values.length; i < n; ++i) { + if (i != 0) { sb.append(", "); } + walk(values[i], this); + } + sb.append(']'); + } + } + // We serialize Objects using the abbreviates {...} syntax ignoring + // properties that end in __. + @Override + void visitObject(Scriptable s, List<Pair<String, Object>> props) { + if (checkDupePrefix(s)) { + sb.append('{'); + boolean sawOne = false; + for (Pair<String, Object> prop : props) { + if (sawOne) { + sb.append(", "); + } else { + sawOne = true; + } + sb.append('"'); + Escaping.escapeJsString(prop.a, true, false, sb); + sb.append("\": "); + walk(prop.b, this); + } + sb.append('}'); + } + } + + /** + * If this is the first mention of an object with multiple in-bound edges, + * attach a numeric label like [EMAIL PROTECTED] #1=}. + * If this is the second or subsequent use, emit a reference like + * [EMAIL PROTECTED] #1#}, and return false so that the renderer knows not to render + * the object. + * @return true if o should be rendered. + */ + private boolean checkDupePrefix(Object o) { + VisitationRecord r = visited.get(o); + if (r.key == 0) { return true; } + sb.append('#').append(r.key); + if (r.written) { + sb.append('#'); + return false; + } + r.written = true; + sb.append('='); + return true; + } + }); + + return sb.toString(); + } + + /** + * Tracks whether a particular Object has been seen before. + * This is used to generate a SpiderMonkey-style representation. + * Spidermonkey's [EMAIL PROTECTED] uneval} will generate a string like + * [EMAIL PROTECTED] #1=[#1#]} for an Array that contains only itself + */ + private static class VisitationRecord { + /** + * A key, unique within the context of a particular object graph, that + * can be used to identify an object that has multiple in-bound links in + * that graph. + * <p>A value of zero indicates that the corresponding object only has one + * in-bound link. + */ + int key; + /** True if the object has already been written out. */ + boolean written; + } + + /** A visitor over a JavaScript object graph. */ + private static abstract class JsObjVisitor { + /** @param s unused in this default implementation. */ + void visitString(String s) {} + /** @param n unused in this default implementation. */ + void visitNumber(Number n) {} + /** @param b unused in this default implementation. */ + void visitBoolean(Boolean b) {} + void visitNull() {} + void visitUndefined() {} + /** + * @param s the JS array. + * @param values elements of the array + */ + void visitArray(Scriptable s, Object[] values) {} + /** + * @param s the JSON object. + * @param props name to value map of cajita mentionable properties. + */ + void visitObject(Scriptable s, List<Pair<String, Object>> props) {} + } + + /** + * Dispatches a node in a JavaScript object graph to a [EMAIL PROTECTED] JsObjVisitor}. + */ + private static void walk(Object o, JsObjVisitor visitor) { + if (o == null) { + visitor.visitNull(); + } else if (o instanceof String) { + visitor.visitString((String) o); + } else if (o instanceof Number) { + visitor.visitNumber((Number) o); + } else if (o instanceof Boolean) { + visitor.visitBoolean((Boolean) o); + } else if (o instanceof Undefined) { + visitor.visitUndefined(); + } else if (o instanceof NativeArray) { + Scriptable s = ((Scriptable) o); + Scriptable globalScope = s.getParentScope(); + + Object lengthVal = s.get("length", globalScope); + int length = ((Number) lengthVal).intValue(); + Object[] elements = new Object[length]; + for (int i = 0; i < length; ++i) { + elements[i] = s.get(i, globalScope); + } + visitor.visitArray(s, elements); + } else if (o instanceof Scriptable && isBaseObject((Scriptable) o)) { + Scriptable s = ((Scriptable) o); + Scriptable globalScope = s.getParentScope(); + + Object[] ids = s.getIds(); + // Ensure a consistent key ordering, placing array indices first. + Arrays.sort(ids, new Comparator<Object>() { + public int compare(Object a, Object b) { + if (a instanceof Number) { + if (b instanceof Number) { + double d = ((Number) a).doubleValue(), + e = ((Number) b).doubleValue(); + return ( + Double.isNaN(d) + ? (Double.isNaN(e) + ? 0 + : 1) + : (d < e + ? -1 + : (d == e ? 0 : 1))); + } else { + return -1; + } + } else if (b instanceof Number) { + return 1; + } else { + return a.toString().compareTo(b.toString()); + } + } + }); + List<Pair<String, Object>> props = new ArrayList<Pair<String, Object>>(); + for (Object id : ids) { + if (id instanceof Number) { + Number n = (Number) id; + int i = n.intValue(); + if (i == n.doubleValue() && i >= 0) { + props.add(Pair.pair("" + i, s.get(i, globalScope))); + continue; + } + } + String k = id.toString(); + if (!k.endsWith("__")) { + props.add(Pair.pair(k, s.get(k, globalScope))); + } + } + visitor.visitObject(s, props); + } else { + String typeHint = ""; + if (o instanceof Scriptable) { + typeHint = " : " + ((Scriptable) o).getClassName(); + } + throw new IllegalArgumentException( + "Cannot compare structure of " + o + typeHint); + } + } + + private static boolean isBaseObject(Scriptable s) { + // A direct instance of Object is the only Object whose prototype's + // prototype is null. + Scriptable proto = s.getPrototype(); + return proto != null && proto.getPrototype() == null; + } + + private RhinoAsserts() { /* uninstantiable */ } +} Index: tests/com/google/caja/util/RhinoAssertsTest.java =================================================================== --- tests/com/google/caja/util/RhinoAssertsTest.java (added) +++ tests/com/google/caja/util/RhinoAssertsTest.java (^/changes/mikesamuel/structural-assert-consistent/trunk/tests/com/google/caja/util/[EMAIL PROTECTED]) @@ -0,0 +1,110 @@ +// Copyright (C) 2008 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.caja.util; + +import java.io.IOException; + +import junit.framework.TestCase; + +public class RhinoAssertsTest extends TestCase { + public void testString() throws Exception { + assertStructuralForm("\"\"", "''"); + assertStructuralForm("\"foo\"", "'foo'"); + assertStructuralForm("\"foo\\nbar\"", "'foo\\nbar'"); + } + + public void testNumbers() throws Exception { + assertStructuralForm("0.0", "0"); + assertStructuralForm("-1.0", "-1"); + assertStructuralForm("1.0", "+1"); + assertStructuralForm("0.1", ".1"); + assertStructuralForm("3.141592654", "3.141592654"); + assertStructuralForm("NaN", "NaN"); + assertStructuralForm("-Infinity", "-Infinity"); + } + + public void testBooleans() throws Exception { + assertStructuralForm("true", "true"); + assertStructuralForm("false", "false"); + } + + public void testNulls() throws Exception { + assertStructuralForm("null", "null"); + assertStructuralForm("undefined", "undefined"); + } + + public void testArrays() throws Exception { + assertStructuralForm("[]", "[]"); + assertStructuralForm("[1.0, 2.0, 3.0]", "[1, 2, 3]"); + assertStructuralForm("[[]]", "[[]]"); + assertStructuralForm("[1.0, 2.0, [3.0], null]", "[1, 2, [3], null]"); + } + + public void testObjects() throws Exception { + assertStructuralForm("{}", "({})"); + assertStructuralForm("{\"a\": {\"b\": null}}", "({ a: { b: null } })"); + assertStructuralForm("{\"a\": 4.0, \"b\": null}", "({ b: null, a: 4 })"); + assertStructuralForm( + "{\"0\": \"hi\", \"2\": \"there\", \"1.5\": null, \"length\": 4.0}", + "({ length: 4, 2: 'there', 0: 'hi', 1.5: null })"); + } + + public void testObjectGraphCycles() throws Exception { + assertStructuralForm( + "#1=[#1#]", + "(function () { var a = []; a.push(a); return a; })()"); + assertStructuralForm( + "#1=[#1#, [3.0]]", + "(function () { var a = []; a.push(a, [3]); return a; })()"); + assertStructuralForm( + "{\"x\": #1=[#1#, [3.0]]}", + "(function () { var a = []; a.push(a, [3]); return { x: a }; })()"); + assertStructuralForm( + "#1=[#2={\"x\": #1#, \"y\": #2#}, [], #1#, \"foo\", \"foo\"]", + "" + + "(function () {" + + " var a = []," + + " b = { x: a };" + + " a.push(b, [], a, 'foo', 'foo');" + + " b.y = b;" + + " return a;" + + "})()"); + } + + public void testBuiltinObjs() throws Exception { + assertIndeterminateStructuralForm("(function () {})"); + assertIndeterminateStructuralForm("(new Date(0))"); + assertIndeterminateStructuralForm( + "(function () { function Array() { } return new Array(); })()"); + assertIndeterminateStructuralForm("java.lang.System.err"); + } + + private void assertStructuralForm(String structuralForm, String js) + throws IOException { + Object result = RhinoTestBed.runJs(new RhinoTestBed.Input(js, getName())); + assertEquals(structuralForm, RhinoAsserts.structuralForm(result)); + } + + private void assertIndeterminateStructuralForm(String js) throws IOException { + Object result = RhinoTestBed.runJs(new RhinoTestBed.Input(js, getName())); + try { + RhinoAsserts.structuralForm(result); + } catch (IllegalArgumentException ex) { + // Pass + return; + } + fail("`" + js + "` should not have a structural form"); + } +} Index: tests/com/google/caja/util/RhinoTestBed.java =================================================================== --- tests/com/google/caja/util/RhinoTestBed.java (^/trunk/tests/com/google/caja/util/[EMAIL PROTECTED]) +++ tests/com/google/caja/util/RhinoTestBed.java (^/changes/mikesamuel/structural-assert-consistent/trunk/tests/com/google/caja/util/[EMAIL PROTECTED]) @@ -48,7 +48,6 @@ public class RhinoTestBed { /** * Runs the javascript from the given inputs in order, and returns the * result. - * If dumpJsFile is not null, also put all the javascript in that file. */ public static Object runJs(Input... inputs) throws IOException { Context context = ContextFactory.getGlobal().enterContext(); --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to http://groups.google.com/group/google-caja-discuss To unsubscribe, email [EMAIL PROTECTED] -~----------~----~----~----~------~----~------~--~---
