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.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to 
http://groups.google.com/group/google-caja-discuss
To unsubscribe, email [EMAIL PROTECTED]
-~----------~----~----~----~------~----~------~--~---

Reply via email to