Fixes issue #1458; corrects some cases of bad JS output from JS code generation. Also optimizes parenthesis output where it's not really needed.
Patch by: sgross
Review by: me
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1511 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/js/JsConstructExpressionVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsConstructExpressionVisitor.java
new file mode 100644
index 0000000..a244456
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/js/JsConstructExpressionVisitor.java
@@ -0,0 +1,123 @@
+/*
+ * Copyright 2007 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.gwt.dev.js;
+
+import com.google.gwt.dev.js.ast.JsArrayAccess;
+import com.google.gwt.dev.js.ast.JsArrayLiteral;
+import com.google.gwt.dev.js.ast.JsContext;
+import com.google.gwt.dev.js.ast.JsExpression;
+import com.google.gwt.dev.js.ast.JsFunction;
+import com.google.gwt.dev.js.ast.JsInvocation;
+import com.google.gwt.dev.js.ast.JsNameRef;
+import com.google.gwt.dev.js.ast.JsNew;
+import com.google.gwt.dev.js.ast.JsObjectLiteral;
+import com.google.gwt.dev.js.ast.JsVisitable;
+import com.google.gwt.dev.js.ast.JsVisitor;
+
+/**
+ * Searches for method invocations in constructor expressions that would not
+ * normally be surrounded by parentheses.
+ */
+public class JsConstructExpressionVisitor extends JsVisitor {
+
+ private static final int PRECEDENCE_NEW = JsPrecedenceVisitor.exec(new JsNew());
+
+ public static boolean exec(JsExpression expression) {
+ if (JsPrecedenceVisitor.exec(expression) < PRECEDENCE_NEW) {
+ return true;
+ }
+ JsConstructExpressionVisitor visitor = new JsConstructExpressionVisitor();
+ visitor.accept(expression);
+ return visitor.containsInvocation;
+ }
+
+ private boolean containsInvocation = false;
+
+ private JsConstructExpressionVisitor() {
+ }
+
+ /**
+ * We only look at the array expression since the index has its own scope.
+ */
+ @Override
+ public boolean visit(JsArrayAccess x, JsContext<JsExpression> ctx) {
+ accept(x.getArrayExpr());
+ return false;
+ }
+
+ /**
+ * Array literals have their own scoping.
+ */
+ @Override
+ public boolean visit(JsArrayLiteral x, JsContext<JsExpression> ctx) {
+ return false;
+ }
+
+ /**
+ * Functions have their own scoping.
+ */
+ @Override
+ public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsInvocation x, JsContext<JsExpression> ctx) {
+ containsInvocation = true;
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsNameRef x, JsContext<JsExpression> ctx) {
+ if (!x.isLeaf()) {
+ accept(x.getQualifier());
+ }
+ return false;
+ }
+
+ /**
+ * New constructs bind to the nearest set of parentheses.
+ */
+ @Override
+ public boolean visit(JsNew x, JsContext<JsExpression> ctx) {
+ return false;
+ }
+
+ /**
+ * Object literals have their own scope.
+ */
+ @Override
+ public boolean visit(JsObjectLiteral x, JsContext<JsExpression> ctx) {
+ return false;
+ }
+
+ /**
+ * We only look at nodes that would not normally be surrounded by parentheses.
+ */
+ protected <T extends JsVisitable<T>> T doAccept(T node) {
+ if (node instanceof JsExpression) {
+ JsExpression expression = (JsExpression) node;
+ int precedence = JsPrecedenceVisitor.exec(expression);
+ // Only visit expressions that won't automatically be surrounded by
+ // parentheses
+ if (precedence < PRECEDENCE_NEW) {
+ return node;
+ }
+ }
+ return super.doAccept(node);
+ }
+
+}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsFirstExpressionVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsFirstExpressionVisitor.java
new file mode 100644
index 0000000..683fcc6
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/js/JsFirstExpressionVisitor.java
@@ -0,0 +1,134 @@
+/*
+ * Copyright 2007 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.gwt.dev.js;
+
+import com.google.gwt.dev.js.ast.JsArrayAccess;
+import com.google.gwt.dev.js.ast.JsArrayLiteral;
+import com.google.gwt.dev.js.ast.JsBinaryOperation;
+import com.google.gwt.dev.js.ast.JsConditional;
+import com.google.gwt.dev.js.ast.JsContext;
+import com.google.gwt.dev.js.ast.JsExprStmt;
+import com.google.gwt.dev.js.ast.JsExpression;
+import com.google.gwt.dev.js.ast.JsFunction;
+import com.google.gwt.dev.js.ast.JsInvocation;
+import com.google.gwt.dev.js.ast.JsNameRef;
+import com.google.gwt.dev.js.ast.JsNew;
+import com.google.gwt.dev.js.ast.JsObjectLiteral;
+import com.google.gwt.dev.js.ast.JsPostfixOperation;
+import com.google.gwt.dev.js.ast.JsPrefixOperation;
+import com.google.gwt.dev.js.ast.JsRegExp;
+import com.google.gwt.dev.js.ast.JsVisitor;
+
+/**
+ * Determines if an expression statement needs to be surrounded by parentheses.
+ *
+ * The statement or the left-most expression needs to be surrounded by
+ * parentheses if the left-most expression is an object literal or a function
+ * object. Function declarations do not need parentheses.
+ *
+ * For example the following require parentheses:<br>
+ * <ul>
+ * <li>{ key : 'value'}</li>
+ * <li>{ key : 'value'}.key</li>
+ * <li>function () {return 1;}()</li>
+ * <li>function () {return 1;}.prototype</li>
+ * </ul>
+ *
+ * The following do not require parentheses:<br>
+ * <ul>
+ * <li>var x = { key : 'value'}</li>
+ * <li>"string" + { key : 'value'}.key</li>
+ * <li>function func() {}</li>
+ * <li>function() {}</li>
+ * </ul>
+ */
+public class JsFirstExpressionVisitor extends JsVisitor {
+
+ public static boolean exec(JsExprStmt statement) {
+ JsFirstExpressionVisitor visitor = new JsFirstExpressionVisitor();
+ JsExpression expression = statement.getExpression();
+ // Pure function declarations do not need parentheses
+ if (expression instanceof JsFunction) {
+ return false;
+ }
+ visitor.accept(statement.getExpression());
+ return visitor.needsParentheses;
+ }
+
+ private boolean needsParentheses = false;
+
+ private JsFirstExpressionVisitor() {
+ }
+
+ public boolean visit(JsArrayAccess x, JsContext<JsExpression> ctx) {
+ accept(x.getArrayExpr());
+ return false;
+ }
+
+ public boolean visit(JsArrayLiteral x, JsContext<JsExpression> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsBinaryOperation x, JsContext<JsExpression> ctx) {
+ accept(x.getArg1());
+ return false;
+ }
+
+ public boolean visit(JsConditional x, JsContext<JsExpression> ctx) {
+ accept(x.getTestExpression());
+ return false;
+ }
+
+ public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
+ needsParentheses = true;
+ return false;
+ }
+
+ public boolean visit(JsInvocation x, JsContext<JsExpression> ctx) {
+ accept(x.getQualifier());
+ return false;
+ }
+
+ public boolean visit(JsNameRef x, JsContext<JsExpression> ctx) {
+ if (!x.isLeaf()) {
+ accept(x.getQualifier());
+ }
+ return false;
+ }
+
+ public boolean visit(JsNew x, JsContext<JsExpression> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsObjectLiteral x, JsContext<JsExpression> ctx) {
+ needsParentheses = true;
+ return false;
+ }
+
+ public boolean visit(JsPostfixOperation x, JsContext<JsExpression> ctx) {
+ accept(x.getArg());
+ return false;
+ }
+
+ public boolean visit(JsPrefixOperation x, JsContext<JsExpression> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsRegExp x, JsContext<JsExpression> ctx) {
+ return false;
+ }
+
+}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
index 719bcc3..09770df 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
@@ -67,10 +67,12 @@
* Precedence indices from "JavaScript - The Definitive Guide" 4th Edition (page
* 57)
*
- * Precedence 16 is for indivisible primaries that either don't have children,
+ * Precedence 17 is for indivisible primaries that either don't have children,
* or provide their own delimiters.
*
- * Precedence 15 is for really important things that have their own AST classes.
+ * Precedence 16 is for really important things that have their own AST classes.
+ *
+ * Precedence 15 is for the new construct.
*
* Precedence 14 is for unary operators.
*
@@ -100,13 +102,13 @@
@Override
public boolean visit(JsArrayAccess x, JsContext<JsExpression> ctx) {
- answer = 15;
+ answer = 16;
return false;
}
@Override
public boolean visit(JsArrayLiteral x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@@ -123,7 +125,7 @@
@Override
public boolean visit(JsBooleanLiteral x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@@ -160,7 +162,7 @@
@Override
public boolean visit(JsDecimalLiteral x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@@ -196,7 +198,7 @@
@Override
public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@@ -207,13 +209,13 @@
@Override
public boolean visit(JsIntegralLiteral x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@Override
public boolean visit(JsInvocation x, JsContext<JsExpression> ctx) {
- answer = 15;
+ answer = 16;
return false;
}
@@ -225,9 +227,9 @@
@Override
public boolean visit(JsNameRef x, JsContext<JsExpression> ctx) {
if (x.isLeaf()) {
- answer = 16; // primary
+ answer = 17; // primary
} else {
- answer = 15; // property access
+ answer = 16; // property access
}
return false;
}
@@ -240,13 +242,13 @@
@Override
public boolean visit(JsNullLiteral x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@Override
public boolean visit(JsObjectLiteral x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@@ -275,13 +277,13 @@
@Override
public boolean visit(JsPropertyInitializer x,
JsContext<JsPropertyInitializer> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@Override
public boolean visit(JsRegExp x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@@ -292,7 +294,7 @@
@Override
public boolean visit(JsStringLiteral x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
@@ -303,7 +305,7 @@
@Override
public boolean visit(JsThisRef x, JsContext<JsExpression> ctx) {
- answer = 16; // primary
+ answer = 17; // primary
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
index c884737..88c9686 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
@@ -47,6 +47,7 @@
import com.google.gwt.dev.js.ast.JsNew;
import com.google.gwt.dev.js.ast.JsNullLiteral;
import com.google.gwt.dev.js.ast.JsObjectLiteral;
+import com.google.gwt.dev.js.ast.JsOperator;
import com.google.gwt.dev.js.ast.JsParameter;
import com.google.gwt.dev.js.ast.JsPostfixOperation;
import com.google.gwt.dev.js.ast.JsPrefixOperation;
@@ -69,6 +70,7 @@
import com.google.gwt.dev.util.TextOutput;
import java.util.Iterator;
+import java.util.regex.Pattern;
/**
* Produces text output from a JavaScript AST.
@@ -108,6 +110,14 @@
*/
private static final int JSBLOCK_LINES_TO_PRINT = 3;
+ /**
+ * A variable name is valid if it contains only letters, numbers, _, $ and
+ * does not begin with a number. There are actually other valid variable
+ * names, such as ones that contain escaped Unicode characters, but we
+ * surround those names with quotes in property initializers to be safe.
+ */
+ private static final Pattern VALID_NAME_PATTERN = Pattern.compile("[a-zA-Z_$][\\w$]*");
+
protected boolean needSemi = true;
private final TextOutput p;
@@ -148,8 +158,7 @@
JsExpression arg1 = x.getArg1();
_parenPush(x, arg1, !op.isLeftAssociative());
accept(arg1);
- boolean needSpace = op.isKeyword() || arg1 instanceof JsPostfixOperation;
- if (needSpace) {
+ if (op.isKeyword()) {
_parenPopOrSpace(x, arg1, !op.isLeftAssociative());
} else {
_parenPop(x, arg1, !op.isLeftAssociative());
@@ -157,8 +166,7 @@
}
p.print(op.getSymbol());
JsExpression arg2 = x.getArg2();
- needSpace = op.isKeyword() || arg2 instanceof JsPrefixOperation;
- if (needSpace) {
+ if (_spaceCalc(op, arg2)) {
_parenPushOrSpace(x, arg2, op.isLeftAssociative());
} else {
_spaceOpt();
@@ -251,16 +259,16 @@
// right associative
{
JsExpression testExpression = x.getTestExpression();
- _parenPush(x, testExpression, true);
+ _parenPush(x, testExpression, false);
accept(testExpression);
- _parenPop(x, testExpression, true);
+ _parenPop(x, testExpression, false);
}
_questionMark();
{
JsExpression thenExpression = x.getThenExpression();
- _parenPush(x, thenExpression, true);
+ _parenPush(x, thenExpression, false);
accept(thenExpression);
- _parenPop(x, thenExpression, true);
+ _parenPop(x, thenExpression, false);
}
_colon();
{
@@ -350,8 +358,15 @@
@Override
public boolean visit(JsExprStmt x, JsContext<JsStatement> ctx) {
- final JsExpression expr = x.getExpression();
+ boolean surroundWithParentheses = JsFirstExpressionVisitor.exec(x);
+ if (surroundWithParentheses) {
+ _lparen();
+ }
+ JsExpression expr = x.getExpression();
accept(expr);
+ if (surroundWithParentheses) {
+ _rparen();
+ }
return false;
}
@@ -552,9 +567,14 @@
_space();
JsExpression ctorExpr = x.getConstructorExpression();
- _parenPush(x, ctorExpr, true);
+ boolean needsParens = JsConstructExpressionVisitor.exec(ctorExpr);
+ if (needsParens) {
+ _lparen();
+ }
accept(ctorExpr);
- _parenPop(x, ctorExpr, true);
+ if (needsParens) {
+ _rparen();
+ }
_lparen();
boolean sep = false;
@@ -584,9 +604,14 @@
sep = _sepCommaOptSpace(sep);
JsPropertyInitializer propInit = (JsPropertyInitializer) element;
JsExpression labelExpr = propInit.getLabelExpr();
- _parenPushIfConditional(labelExpr);
- accept(labelExpr);
- _parenPopIfConditional(labelExpr);
+ // labels can be either string, integral, or decimal literals
+ if (labelExpr instanceof JsStringLiteral
+ && VALID_NAME_PATTERN.matcher(
+ ((JsStringLiteral) labelExpr).getValue()).matches()) {
+ p.print(((JsStringLiteral) labelExpr).getValue());
+ } else {
+ accept(labelExpr);
+ }
_colon();
JsExpression valueExpr = propInit.getValueExpr();
_parenPushIfConditional(valueExpr);
@@ -608,9 +633,9 @@
JsUnaryOperator op = x.getOperator();
JsExpression arg = x.getArg();
// unary operators always associate correctly (I think)
- _parenPush(x, arg, true);
+ _parenPush(x, arg, false);
accept(arg);
- _parenPop(x, arg, true);
+ _parenPop(x, arg, false);
p.print(op.getSymbol());
return false;
}
@@ -619,14 +644,14 @@
public boolean visit(JsPrefixOperation x, JsContext<JsExpression> ctx) {
JsUnaryOperator op = x.getOperator();
p.print(op.getSymbol());
- if (op.isKeyword()) {
+ JsExpression arg = x.getArg();
+ if (_spaceCalc(op, arg)) {
_space();
}
- JsExpression arg = x.getArg();
// unary operators always associate correctly (I think)
- _parenPush(x, arg, true);
+ _parenPush(x, arg, false);
accept(arg);
- _parenPop(x, arg, true);
+ _parenPop(x, arg, false);
return false;
}
@@ -637,7 +662,8 @@
}
@Override
- public boolean visit(JsPropertyInitializer x, JsContext<JsPropertyInitializer> ctx) {
+ public boolean visit(JsPropertyInitializer x,
+ JsContext<JsPropertyInitializer> ctx) {
// Since there are separators, we actually print the property init
// in visit(JsObjectLiteral).
//
@@ -1070,6 +1096,19 @@
p.print(' ');
}
+ private boolean _spaceCalc(JsOperator op, JsExpression arg) {
+ if (op.isKeyword()) {
+ return true;
+ }
+ if (arg instanceof JsPrefixOperation) {
+ JsOperator op2 = ((JsPrefixOperation) arg).getOperator();
+ return (op == JsBinaryOperator.SUB || op == JsUnaryOperator.NEG)
+ && (op2 == JsUnaryOperator.DEC || op2 == JsUnaryOperator.NEG)
+ || (op == JsBinaryOperator.ADD && op2 == JsUnaryOperator.INC);
+ }
+ return false;
+ }
+
private void _spaceOpt() {
p.printOpt(' ');
}
diff --git a/dev/core/test/com/google/gwt/dev/js/ComparingVisitor.java b/dev/core/test/com/google/gwt/dev/js/ComparingVisitor.java
new file mode 100644
index 0000000..573d436
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/js/ComparingVisitor.java
@@ -0,0 +1,390 @@
+/*
+ * Copyright 2007 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.gwt.dev.js;
+
+import com.google.gwt.dev.js.FlatteningVisitor.TreeNode;
+import com.google.gwt.dev.js.ast.JsArrayAccess;
+import com.google.gwt.dev.js.ast.JsArrayLiteral;
+import com.google.gwt.dev.js.ast.JsBinaryOperation;
+import com.google.gwt.dev.js.ast.JsBlock;
+import com.google.gwt.dev.js.ast.JsBooleanLiteral;
+import com.google.gwt.dev.js.ast.JsBreak;
+import com.google.gwt.dev.js.ast.JsCase;
+import com.google.gwt.dev.js.ast.JsCatch;
+import com.google.gwt.dev.js.ast.JsConditional;
+import com.google.gwt.dev.js.ast.JsContext;
+import com.google.gwt.dev.js.ast.JsContinue;
+import com.google.gwt.dev.js.ast.JsDebugger;
+import com.google.gwt.dev.js.ast.JsDecimalLiteral;
+import com.google.gwt.dev.js.ast.JsDefault;
+import com.google.gwt.dev.js.ast.JsDoWhile;
+import com.google.gwt.dev.js.ast.JsEmpty;
+import com.google.gwt.dev.js.ast.JsExprStmt;
+import com.google.gwt.dev.js.ast.JsExpression;
+import com.google.gwt.dev.js.ast.JsFor;
+import com.google.gwt.dev.js.ast.JsForIn;
+import com.google.gwt.dev.js.ast.JsFunction;
+import com.google.gwt.dev.js.ast.JsIf;
+import com.google.gwt.dev.js.ast.JsIntegralLiteral;
+import com.google.gwt.dev.js.ast.JsInvocation;
+import com.google.gwt.dev.js.ast.JsLabel;
+import com.google.gwt.dev.js.ast.JsName;
+import com.google.gwt.dev.js.ast.JsNameRef;
+import com.google.gwt.dev.js.ast.JsNew;
+import com.google.gwt.dev.js.ast.JsNullLiteral;
+import com.google.gwt.dev.js.ast.JsObjectLiteral;
+import com.google.gwt.dev.js.ast.JsParameter;
+import com.google.gwt.dev.js.ast.JsPostfixOperation;
+import com.google.gwt.dev.js.ast.JsPrefixOperation;
+import com.google.gwt.dev.js.ast.JsProgram;
+import com.google.gwt.dev.js.ast.JsPropertyInitializer;
+import com.google.gwt.dev.js.ast.JsRegExp;
+import com.google.gwt.dev.js.ast.JsReturn;
+import com.google.gwt.dev.js.ast.JsStatement;
+import com.google.gwt.dev.js.ast.JsStringLiteral;
+import com.google.gwt.dev.js.ast.JsSwitch;
+import com.google.gwt.dev.js.ast.JsSwitchMember;
+import com.google.gwt.dev.js.ast.JsThisRef;
+import com.google.gwt.dev.js.ast.JsThrow;
+import com.google.gwt.dev.js.ast.JsTry;
+import com.google.gwt.dev.js.ast.JsVars;
+import com.google.gwt.dev.js.ast.JsVisitable;
+import com.google.gwt.dev.js.ast.JsVisitor;
+import com.google.gwt.dev.js.ast.JsWhile;
+import com.google.gwt.dev.js.ast.JsVars.JsVar;
+
+import junit.framework.Assert;
+import junit.framework.TestCase;
+
+import java.util.List;
+
+public class ComparingVisitor extends JsVisitor {
+
+ public static void exec(List<JsStatement> expected, List<JsStatement> actual) {
+ TreeNode expectedTree = FlatteningVisitor.exec(expected);
+ TreeNode actualTree = FlatteningVisitor.exec(actual);
+ compare(expectedTree, actualTree);
+ }
+
+ private static void compare(JsVisitable<?> expected, JsVisitable<?> actual) {
+ if (expected == actual) {
+ return;
+ }
+ Assert.assertNotNull(expected);
+ Assert.assertNotNull(actual);
+ ComparingVisitor visitor = new ComparingVisitor(expected);
+ visitor.accept(actual);
+ }
+
+ private static void compare(TreeNode expected, TreeNode actual) {
+ compare(expected.node, actual.node);
+ List<TreeNode> expectedChildren = expected.children;
+ List<TreeNode> actualChildren = actual.children;
+ Assert.assertEquals(expectedChildren.size(), actualChildren.size());
+ for (int i = 0; i < expectedChildren.size(); i++) {
+ compare(expectedChildren.get(i), actualChildren.get(i));
+ }
+ }
+
+ private final JsVisitable<?> other;
+
+ private ComparingVisitor(JsVisitable<?> other) {
+ this.other = other;
+ }
+
+ @Override
+ public boolean visit(JsArrayAccess x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsArrayAccess);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsArrayLiteral x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsArrayLiteral);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsBinaryOperation x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsBinaryOperation);
+ Assert.assertEquals(((JsBinaryOperation) other).getOperator().getSymbol(),
+ x.getOperator().getSymbol());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsBlock x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsBlock);
+ Assert.assertEquals(((JsBlock) other).isGlobalBlock(), x.isGlobalBlock());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsBooleanLiteral x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsBooleanLiteral);
+ Assert.assertEquals(((JsBooleanLiteral) other).getValue(), x.getValue());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsBreak x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsBreak);
+ Assert.assertEquals(((JsBreak) other).getLabel().getIdent(),
+ x.getLabel().getIdent());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsCase x, JsContext<JsSwitchMember> ctx) {
+ Assert.assertTrue(other instanceof JsCase);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsCatch x, JsContext<JsCatch> ctx) {
+ Assert.assertTrue(other instanceof JsCatch);
+ Assert.assertEquals(((JsCatch) other).getParameter().getName().getIdent(),
+ x.getParameter().getName().getIdent());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsConditional x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsConditional);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsContinue x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsContinue);
+ Assert.assertEquals(((JsContinue) other).getLabel().getIdent(),
+ x.getLabel().getIdent());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsDebugger x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsDebugger);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsDecimalLiteral x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsDecimalLiteral);
+ Assert.assertEquals(((JsDecimalLiteral) other).getValue(), x.getValue());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsDefault x, JsContext<JsSwitchMember> ctx) {
+ Assert.assertTrue(other instanceof JsDefault);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsDoWhile x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsDoWhile);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsEmpty x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsEmpty);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsExprStmt x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsExprStmt);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsFor x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsFor);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsForIn x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsForIn);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsFunction);
+ JsFunction otherFunc = (JsFunction) other;
+ JsName otherName = otherFunc.getName();
+ JsName name = x.getName();
+ if (name != otherName) {
+ Assert.assertEquals(otherName.getIdent(), name.getIdent());
+ }
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsIf x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsIf);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsIntegralLiteral x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsIntegralLiteral);
+ Assert.assertEquals(((JsIntegralLiteral) other).getValue(), x.getValue());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsInvocation x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsInvocation);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsLabel x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsLabel);
+ Assert.assertEquals(((JsLabel) other).getName().getIdent(),
+ x.getName().getIdent());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsNameRef x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsNameRef);
+ Assert.assertEquals(((JsNameRef) other).getIdent(), x.getIdent());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsNew x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsNew);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsNullLiteral x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsNullLiteral);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsObjectLiteral x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsObjectLiteral);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsParameter x, JsContext<JsParameter> ctx) {
+ Assert.assertTrue(other instanceof JsParameter);
+ Assert.assertEquals(((JsParameter) other).getName().getIdent(),
+ x.getName().getIdent());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsPostfixOperation x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsPostfixOperation);
+ Assert.assertEquals(((JsPostfixOperation) other).getOperator().getSymbol(),
+ x.getOperator().getSymbol());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsPrefixOperation x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsPrefixOperation);
+ Assert.assertEquals(((JsPrefixOperation) other).getOperator().getSymbol(),
+ x.getOperator().getSymbol());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsProgram x, JsContext<JsProgram> ctx) {
+ Assert.assertTrue(other instanceof JsProgram);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsPropertyInitializer x,
+ JsContext<JsPropertyInitializer> ctx) {
+ Assert.assertTrue(other instanceof JsPropertyInitializer);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsRegExp x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsRegExp);
+ Assert.assertEquals(((JsRegExp) other).getFlags(), x.getFlags());
+ Assert.assertEquals(((JsRegExp) other).getPattern(), x.getPattern());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsReturn x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsReturn);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsStringLiteral x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsStringLiteral);
+ Assert.assertEquals(((JsStringLiteral) other).getValue(), x.getValue());
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsSwitch x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsSwitch);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsThisRef x, JsContext<JsExpression> ctx) {
+ Assert.assertTrue(other instanceof JsThisRef);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsThrow x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsThrow);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsTry x, JsContext<JsStatement> ctx) {
+ Assert.assertTrue(other instanceof JsTry);
+ return false;
+ }
+
+ @Override
+ public boolean visit(JsVar x, JsContext<JsVar> ctx) {
+ TestCase.assertTrue(other instanceof JsVar);
+ TestCase.assertEquals(((JsVar) other).getName().getIdent(),
+ x.getName().getIdent());
+ return false;
+ }
+
+ public boolean visit(JsVars x, JsContext<JsStatement> ctx) {
+ TestCase.assertTrue(other instanceof JsVars);
+ return false;
+ }
+
+ public boolean visit(JsWhile x, JsContext<JsStatement> ctx) {
+ TestCase.assertTrue(other instanceof JsWhile);
+ return false;
+ }
+}
diff --git a/dev/core/test/com/google/gwt/dev/js/FlatteningVisitor.java b/dev/core/test/com/google/gwt/dev/js/FlatteningVisitor.java
new file mode 100644
index 0000000..1dc0dbf
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/js/FlatteningVisitor.java
@@ -0,0 +1,63 @@
+/*
+ * Copyright 2007 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.gwt.dev.js;
+
+import com.google.gwt.dev.js.ast.JsStatement;
+import com.google.gwt.dev.js.ast.JsVisitable;
+import com.google.gwt.dev.js.ast.JsVisitor;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+class FlatteningVisitor extends JsVisitor {
+
+ public static TreeNode exec(List<JsStatement> statements) {
+ FlatteningVisitor visitor = new FlatteningVisitor();
+ visitor.acceptList(statements);
+ return visitor.root;
+ }
+
+ public static class TreeNode {
+ public final JsVisitable<?> node;
+ public final List<TreeNode> children = new ArrayList<TreeNode>();
+
+ public TreeNode(JsVisitable<?> node) {
+ this.node = node;
+ }
+ }
+
+ private TreeNode root;
+
+ private FlatteningVisitor() {
+ root = new TreeNode(null);
+ }
+
+ protected <T extends JsVisitable<T>> T doAccept(T node) {
+ TreeNode oldRoot = root;
+ root = new TreeNode(node);
+ oldRoot.children.add(root);
+ super.doAccept(node);
+ root = oldRoot;
+ return node;
+ }
+
+ protected <T extends JsVisitable<T>> void doAcceptList(List<T> collection) {
+ for (Iterator<T> it = collection.iterator(); it.hasNext();) {
+ doAccept(it.next());
+ }
+ }
+}
diff --git a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java
new file mode 100644
index 0000000..6c05c22
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java
@@ -0,0 +1,113 @@
+/*
+ * Copyright 2007 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.gwt.dev.js;
+
+import com.google.gwt.dev.js.ast.JsProgram;
+import com.google.gwt.dev.js.ast.JsStatement;
+import com.google.gwt.dev.js.ast.JsVisitor;
+import com.google.gwt.dev.util.DefaultTextOutput;
+import com.google.gwt.dev.util.TextOutput;
+
+import junit.framework.TestCase;
+
+import java.io.StringReader;
+import java.util.List;
+
+public class JsToStringGenerationVisitorAccuracyTest extends TestCase {
+
+ private JsParser parser = new JsParser();
+
+ public void testAdditionPositive() throws Exception {
+ // x plus positive 3
+ doTest("x + +3");
+ }
+
+ public void testArithmetic() throws Exception {
+ doTest("a + (b * (c - d)) / (e / f) % x");
+ }
+
+ public void testArrayDeclarationArrayAccess() throws Exception {
+ doTest("[1,2,3,4][2]");
+ }
+
+ public void testComplexConstruction() throws Exception {
+ doTest("(new (new (a(({a : 'b', c : 'd'}),[1,2,3,x,y,z]))())())()");
+ }
+
+ public void testConstructionInvocation() throws Exception {
+ doTest("(new a())()");
+ }
+
+ public void testDecrement() throws Exception {
+ doTest("(x--)-(-(--y))");
+ }
+
+ public void testFunctionDeclarationInvocation() throws Exception {
+ doTest("(function () {})()");
+ }
+
+ public void testInvocationConstruction() throws Exception {
+ doTest("new ((a.b.c()).d.e)(1,2,3)");
+ }
+
+ public void testNestedConstruction() throws Exception {
+ doTest("new (new (new MyClass()))");
+ }
+
+ public void testObjectDeclarationArrayAccess() throws Exception {
+ doTest("({ a : 'b'})['a']");
+ }
+
+ public void testObjectDeclarationMemberAccess() throws Exception {
+ doTest("({ a : 'b'}).a");
+ }
+
+ public void testObjectLiteral() throws Exception {
+ // declaring an object requires parentheses
+ doTest("({ 'property' : 'value'})");
+ }
+
+ public void testObjectLiteralDeclaration() throws Exception {
+ // quotes are necessary around some property variables
+ doTest("var x = {'abc\\'' : 'value'}");
+ doTest("var x = {\"a.1\" : 'value'}");
+ doTest("var x = {\"\\'\\\"\" : 'value'}");
+ }
+
+ public void testUnaryOperations() throws Exception {
+ // spaces or parentheses are necessary to separate negation and decrement
+ doTest("var x = -(-(--y))");
+ }
+
+ private void doTest(String js) throws Exception {
+ List<JsStatement> expected = parser.parse(new JsProgram().getScope(),
+ new StringReader(js), 0);
+ List<JsStatement> actual = parse(expected, true);
+ ComparingVisitor.exec(expected, actual);
+
+ actual = parse(expected, false);
+ ComparingVisitor.exec(expected, actual);
+ }
+
+ private List<JsStatement> parse(List<JsStatement> expected, boolean compact)
+ throws Exception {
+ TextOutput text = new DefaultTextOutput(compact);
+ JsVisitor generator = new JsToStringGenerationVisitor(text);
+ generator.acceptList(expected);
+ return parser.parse(new JsProgram().getScope(), new StringReader(
+ text.toString()), 0);
+ }
+}
diff --git a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorConcisenessTest.java b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorConcisenessTest.java
new file mode 100644
index 0000000..e77f01b
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorConcisenessTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2007 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.gwt.dev.js;
+
+import com.google.gwt.dev.js.ast.JsProgram;
+import com.google.gwt.dev.js.ast.JsStatement;
+import com.google.gwt.dev.js.ast.JsVisitor;
+import com.google.gwt.dev.util.DefaultTextOutput;
+import com.google.gwt.dev.util.TextOutput;
+
+import junit.framework.TestCase;
+
+import java.io.StringReader;
+import java.util.List;
+
+public class JsToStringGenerationVisitorConcisenessTest extends TestCase {
+
+ private JsParser parser = new JsParser();
+
+ public void testComplexDecrement() throws Exception {
+ String output = parse("var x = -(-(-(--y)))");
+ assertEquals("var x=- - - --y", output);
+ }
+
+ public void testComplexIncrement() throws Exception {
+ String output = parse("var x = (y++) + (-(--z))");
+ assertEquals("var x=y+++- --z", output);
+ }
+
+ public void testConstruction() throws Exception {
+ String output = parse("new a.b(c,d)");
+ assertEquals("new a.b(c,d)", output);
+ }
+
+ public void testIncrement() throws Exception {
+ // y++-x is valid
+ assertEquals("var x=y++-z", parse("var x = (y++) - z"));
+ }
+
+ public void testObjectLiteralDeclarationConcise() throws Exception {
+ // quotes are not necessary around many property variables in object
+ // literals
+ assertEquals("var x={1:'b'}", parse("var x = {1 : 'b'}"));
+ assertEquals("var x={$a_:'b'}", parse("var x = {'$a_' : 'b'}"));
+ assertEquals("var x={1.2:'b'}", parse("var x = {1.2 : 'b'}"));
+ }
+
+ private String parse(String js) throws Exception {
+ List<JsStatement> statements = parser.parse(new JsProgram().getScope(),
+ new StringReader(js), 0);
+ TextOutput text = new DefaultTextOutput(true);
+ JsVisitor generator = new JsToStringGenerationVisitor(text);
+ generator.acceptList(statements);
+ return text.toString();
+ }
+}
