LGTM
On Thu, Aug 28, 2008 at 10:01 AM, <[EMAIL PROTECTED]> wrote: > I'd like you to do a code review. To review this change, run > > gvn review --project https://google-caja.googlecode.com/svn/trunk > ihab/[EMAIL PROTECTED] > > Alternatively, to review the latest snapshot of this change > branch, run > > gvn --project https://google-caja.googlecode.com/svn/trunk review > ihab/fixmultidecl > > to review the following change: > > *ihab/[EMAIL PROTECTED] | ihab | 2008-08-28 08:58:22 -0800 (Thu, 28 Aug 2008) > > Description: > > Fix MultiDeclaration > Fix special case for MultiDeclaration to only fire at global scope. > > > > > Affected Paths: > M //trunk/src/com/google/caja/parser/quasiliteral/DefaultValijaRewriter.java > M //trunk/src/com/google/caja/parser/quasiliteral/Scope.java > M > //trunk/tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTest.java > M > //trunk/tests/com/google/caja/parser/quasiliteral/DefaultValijaRewriterTest.java > > > This is a semiautomated message from "gvn mail". See > <http://code.google.com/p/gvn/> to learn more. > > Index: src/com/google/caja/parser/quasiliteral/DefaultValijaRewriter.java > =================================================================== > --- src/com/google/caja/parser/quasiliteral/DefaultValijaRewriter.java > (//trunk/src/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) > +++ src/com/google/caja/parser/quasiliteral/DefaultValijaRewriter.java > (//changes/ihab/fixmultidecl/trunk/src/com/google/caja/parser/quasiliteral/[EMAIL > PROTECTED]) > @@ -1231,7 +1231,8 @@ > synopsis="Convert a MultiDeclaration into a comma expression", > reason="") > public ParseTreeNode fire(ParseTreeNode node, Scope scope, MessageQueue > mq) { > - if (node instanceof MultiDeclaration) { > + if (node instanceof MultiDeclaration > + && scope.isOuter()) { > Expression[] newChildren = new Expression[node.children().size()]; > for (int i = 0; i < newChildren.length; i++) { > ExpressionStmt result = (ExpressionStmt) > Index: src/com/google/caja/parser/quasiliteral/Scope.java > =================================================================== > --- src/com/google/caja/parser/quasiliteral/Scope.java > (//trunk/src/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) > +++ src/com/google/caja/parser/quasiliteral/Scope.java > (//changes/ihab/fixmultidecl/trunk/src/com/google/caja/parser/quasiliteral/[EMAIL > PROTECTED]) > @@ -270,6 +270,17 @@ > } > > /** > + * Determines whether this is an outer scope. A scope is outer if it is not > + * (transitively) contained in any function scopes. Any declarations in > this > + * scope are therefore visible throughout the program. > + */ > + public boolean isOuter() { > + if (type == ScopeType.FUNCTION_BODY) { return false; } > + if (parent == null) return true; > + return parent.isOuter(); > + } > + > + /** > * When a Scope is used for recursively processing a parse tree, steps > taken on nodes contained > * within the node of this Scope sometimes add statements (e.g., variable > declarations) that > * need to be rendered in the result before these nodes are rendered. These > statements are the > Index: tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTest.java > =================================================================== > --- tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTest.java > (//trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL PROTECTED]) > +++ tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTest.java > (//changes/ihab/fixmultidecl/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL > PROTECTED]) > @@ -227,6 +227,17 @@ > assertConsistent("for (var i = 0, j = 0; i < 10; i++) { j += 10; } j;"); > assertConsistent("for (var i = 0, j = 0; i < 10; i++, j += 10) { } j;"); > } > + > + public void testMultiDeclaration() throws Exception { > + assertConsistent("var a = 3, b = 4; a + b;"); > + assertConsistent("var a, b; a = 3; b = 4; a + b;"); > + assertConsistent( > + " function f() {" > + + " var a = 3, b = 4;" > + + " return a + b;" > + + " }" > + + "f();"); > + } > } > > > 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/ihab/fixmultidecl/trunk/tests/com/google/caja/parser/quasiliteral/[EMAIL > PROTECTED]) > @@ -94,11 +94,6 @@ > assertConsistent("str=''; for (var i in {x:1, y:true}) {str+=i;} str;"); > } > > - public void testMultiDeclaration() throws Exception { > - assertConsistent("var a = 3, b = 4; a + b;"); > - assertConsistent("var a, b; a = 3; b = 4; a + b;"); > - } > - > /** > * Tests that the container can get access to > * "virtual globals" defined in cajoled code. > > -- Mike Stay - [EMAIL PROTECTED] http://math.ucr.edu/~mike http://reperiendi.wordpress.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to http://groups.google.com/group/google-caja-discuss To unsubscribe, email [EMAIL PROTECTED] -~----------~----~----~----~------~----~------~--~---
