1) Incorporating a variant of a patch by sandymac in issue #599 to reduce
the amount of whitespace we generate around binary operators. Before, we
generated whitespace around every binOp. After this patch, we will only
generate whitespace between a binary operator and a unary operator that
occurs immediately before or after it.
2) A couple of other peephole fixes where spaces were being generated
unnecessarily.
3) Removed the JsDelete node. "delete" is truly an operator in JS, so
there's no reason not to really model it as such.
Patch by: scottb, sandymac (JsToStringGenerationVisitor.java; modified by me)
Review by: mmendez
Fixes: 599
M dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
M dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java
M dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java
M dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java
M dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java
D dev/core/src/com/google/gwt/dev/js/ast/JsDelete.java
D dev/core/src/com/google/gwt/dev/js/ast/JsAbstractVisitorWithEndVisits.java
M dev/core/src/com/google/gwt/dev/js/ast/JsOperator.java
M dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java
M dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
M dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java
M dev/core/src/com/google/gwt/dev/js/JsParser.java
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@356 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java
index 43f47aa..5bc8ad8 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java
@@ -27,7 +27,6 @@
import com.google.gwt.dev.js.ast.JsContinue;
import com.google.gwt.dev.js.ast.JsDecimalLiteral;
import com.google.gwt.dev.js.ast.JsDefault;
-import com.google.gwt.dev.js.ast.JsDelete;
import com.google.gwt.dev.js.ast.JsDoWhile;
import com.google.gwt.dev.js.ast.JsEmpty;
import com.google.gwt.dev.js.ast.JsExprStmt;
@@ -101,9 +100,6 @@
public void endVisit(JsDefault x) {
}
- public void endVisit(JsDelete x) {
- }
-
public void endVisit(JsDoWhile while1) {
}
@@ -239,10 +235,6 @@
return true;
}
- public boolean visit(JsDelete x) {
- return true;
- }
-
public boolean visit(JsDoWhile x) {
return true;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java
index ca7a5fa..abf78e5 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java
@@ -27,7 +27,6 @@
import com.google.gwt.dev.js.ast.JsContinue;
import com.google.gwt.dev.js.ast.JsDecimalLiteral;
import com.google.gwt.dev.js.ast.JsDefault;
-import com.google.gwt.dev.js.ast.JsDelete;
import com.google.gwt.dev.js.ast.JsDoWhile;
import com.google.gwt.dev.js.ast.JsEmpty;
import com.google.gwt.dev.js.ast.JsExprStmt;
@@ -100,9 +99,6 @@
public void endVisit(JsDefault x) {
}
- public void endVisit(JsDelete x) {
- }
-
public void endVisit(JsDoWhile while1) {
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java
index 543e3dd..4fd389d 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java
@@ -27,7 +27,6 @@
import com.google.gwt.dev.js.ast.JsContinue;
import com.google.gwt.dev.js.ast.JsDecimalLiteral;
import com.google.gwt.dev.js.ast.JsDefault;
-import com.google.gwt.dev.js.ast.JsDelete;
import com.google.gwt.dev.js.ast.JsDoWhile;
import com.google.gwt.dev.js.ast.JsEmpty;
import com.google.gwt.dev.js.ast.JsExprStmt;
@@ -112,10 +111,6 @@
return true;
}
- public boolean visit(JsDelete x) {
- return true;
- }
-
public boolean visit(JsDoWhile x) {
return true;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsParser.java b/dev/core/src/com/google/gwt/dev/js/JsParser.java
index a33ae9d..be3fcfb 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsParser.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsParser.java
@@ -27,7 +27,6 @@
import com.google.gwt.dev.js.ast.JsConditional;
import com.google.gwt.dev.js.ast.JsContinue;
import com.google.gwt.dev.js.ast.JsDefault;
-import com.google.gwt.dev.js.ast.JsDelete;
import com.google.gwt.dev.js.ast.JsDoWhile;
import com.google.gwt.dev.js.ast.JsExpression;
import com.google.gwt.dev.js.ast.JsExpressions;
@@ -468,17 +467,15 @@
private JsNode mapDeleteProp(Node node) throws JsParserException {
Node from = node.getFirstChild();
JsExpression to = mapExpression(from);
- JsDelete toDelete = new JsDelete();
if (to instanceof JsNameRef) {
- toDelete.setExpr((JsNameRef) to);
+ return new JsPrefixOperation(JsUnaryOperator.DELETE, to);
} else if (to instanceof JsArrayAccess) {
- toDelete.setExpr((JsArrayAccess) to);
+ return new JsPrefixOperation(JsUnaryOperator.DELETE, to);
} else {
throw createParserException(
"'delete' can only operate on property names and array elements",
from);
}
- return toDelete;
}
private JsStatement mapDoOrWhileStatement(boolean isWhile, Node ifNode)
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 5ed8182..1f41312 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
@@ -15,7 +15,6 @@
*/
package com.google.gwt.dev.js;
-import com.google.gwt.dev.js.ast.JsAbstractVisitorWithEndVisits;
import com.google.gwt.dev.js.ast.JsArrayAccess;
import com.google.gwt.dev.js.ast.JsArrayLiteral;
import com.google.gwt.dev.js.ast.JsBinaryOperation;
@@ -28,7 +27,6 @@
import com.google.gwt.dev.js.ast.JsContinue;
import com.google.gwt.dev.js.ast.JsDecimalLiteral;
import com.google.gwt.dev.js.ast.JsDefault;
-import com.google.gwt.dev.js.ast.JsDelete;
import com.google.gwt.dev.js.ast.JsDoWhile;
import com.google.gwt.dev.js.ast.JsEmpty;
import com.google.gwt.dev.js.ast.JsExprStmt;
@@ -149,11 +147,6 @@
throw new RuntimeException("Only expressions have precedence.");
}
- public boolean visit(JsDelete x) {
- answer = 14; // not modeled as an operator
- return false;
- }
-
public boolean visit(JsDoWhile x) {
throw new RuntimeException("Only expressions have precedence.");
}
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 9624bf6..b7ad13e 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
@@ -29,7 +29,6 @@
import com.google.gwt.dev.js.ast.JsContinue;
import com.google.gwt.dev.js.ast.JsDecimalLiteral;
import com.google.gwt.dev.js.ast.JsDefault;
-import com.google.gwt.dev.js.ast.JsDelete;
import com.google.gwt.dev.js.ast.JsDoWhile;
import com.google.gwt.dev.js.ast.JsEmpty;
import com.google.gwt.dev.js.ast.JsExprStmt;
@@ -142,10 +141,20 @@
JsExpression arg1 = x.getArg1();
_parenPush(x, arg1, !op.isLeftAssociative());
arg1.traverse(this);
- _parenPopOrSpace(x, arg1, !op.isLeftAssociative());
+ boolean needSpace = op.isKeyword() || arg1 instanceof JsPostfixOperation;
+ if (needSpace) {
+ _parenPopOrSpace(x, arg1, !op.isLeftAssociative());
+ } else {
+ _parenPop(x, arg1, !op.isLeftAssociative());
+ }
p.print(op.getSymbol());
JsExpression arg2 = x.getArg2();
- _parenPushOrSpace(x, arg2, op.isLeftAssociative());
+ needSpace = op.isKeyword() || arg2 instanceof JsPrefixOperation;
+ if (needSpace) {
+ _parenPushOrSpace(x, arg2, op.isLeftAssociative());
+ } else {
+ _parenPush(x, arg2, op.isLeftAssociative());
+ }
arg2.traverse(this);
_parenPop(x, arg2, op.isLeftAssociative());
return false;
@@ -297,14 +306,11 @@
public boolean visit(JsDecimalLiteral x) {
String s = x.getValue();
- boolean needParens = s.startsWith("-");
- if (needParens) {
- _lparen();
+ // TODO: optimize this to only the cases that need it
+ if (s.startsWith("-")) {
+ _space();
}
p.print(s);
- if (needParens) {
- _rparen();
- }
return false;
}
@@ -327,15 +333,6 @@
return false;
}
- public boolean visit(JsDelete x) {
- _delete();
- JsExpression expr = x.getExpr();
- _parenPushOrSpace(expr, x, true);
- expr.traverse(this);
- _parenPopOrSpace(expr, x, true);
- return false;
- }
-
public boolean visit(JsDoWhile x) {
_do();
_nestedPush(x.getBody(), true);
@@ -621,6 +618,9 @@
public boolean visit(JsPrefixOperation x) {
JsUnaryOperator op = x.getOperator();
p.print(op.getSymbol());
+ if (op.isKeyword()) {
+ _space();
+ }
JsExpression arg = x.getArg();
// unary operators always associate correctly (I think)
_parenPush(x, arg, true);
@@ -654,9 +654,9 @@
public boolean visit(JsReturn x) {
_return();
- _space();
JsExpression expr = x.getExpr();
if (expr != null) {
+ _space();
expr.traverse(this);
}
return false;
@@ -930,7 +930,6 @@
return doPop;
}
- // TODO(later): we often leave a space when we don't need to
private boolean _parenPush(JsExpression parent, JsExpression child,
boolean wrongAssoc) {
boolean doPush = _parenCalc(parent, child, wrongAssoc);
@@ -957,6 +956,7 @@
return doPush;
}
+ // TODO(later): we often leave a space when we don't need to
private boolean _parenPushOrSpace(JsExpression parent, JsExpression child,
boolean wrongAssoc) {
boolean doPush = _parenCalc(parent, child, wrongAssoc);
@@ -1040,8 +1040,9 @@
private void _while() {
p.print(CHARS_WHILE);
}
+
// CHECKSTYLE_NAMING_ON
-
+
private void indent() {
p.indentIn();
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsAbstractVisitorWithEndVisits.java b/dev/core/src/com/google/gwt/dev/js/ast/JsAbstractVisitorWithEndVisits.java
deleted file mode 100644
index 45a0000..0000000
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsAbstractVisitorWithEndVisits.java
+++ /dev/null
@@ -1,148 +0,0 @@
-/*
- * Copyright 2006 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.ast;
-
-/**
- * Implements stubs for the <code>endVisit()</code> interface methods.
- */
-public abstract class JsAbstractVisitorWithEndVisits implements JsVisitor {
-
- public void endVisit(JsArrayAccess x) {
- }
-
- public void endVisit(JsArrayLiteral x) {
- }
-
- public void endVisit(JsBinaryOperation x) {
- }
-
- public void endVisit(JsBlock x) {
- }
-
- public void endVisit(JsBooleanLiteral x) {
- }
-
- public void endVisit(JsBreak x) {
- }
-
- public void endVisit(JsCase x) {
- }
-
- public void endVisit(JsCatch x) {
- }
-
- public void endVisit(JsConditional x) {
- }
-
- public void endVisit(JsContinue x) {
- }
-
- public void endVisit(JsDecimalLiteral x) {
- }
-
- public void endVisit(JsDefault x) {
- }
-
- public void endVisit(JsDelete x) {
- }
-
- public void endVisit(JsDoWhile while1) {
- }
-
- public void endVisit(JsEmpty x) {
- }
-
- public void endVisit(JsExprStmt x) {
- }
-
- public void endVisit(JsFor x) {
- }
-
- public void endVisit(JsForIn x) {
- }
-
- public void endVisit(JsFunction x) {
- }
-
- public void endVisit(JsIf x) {
- }
-
- public void endVisit(JsIntegralLiteral x) {
- }
-
- public void endVisit(JsInvocation x) {
- }
-
- public void endVisit(JsLabel x) {
- }
-
- public void endVisit(JsNameRef x) {
- }
-
- public void endVisit(JsNew x) {
- }
-
- public void endVisit(JsNullLiteral x) {
- }
-
- public void endVisit(JsObjectLiteral x) {
- }
-
- public void endVisit(JsParameter x) {
- }
-
- public void endVisit(JsParameters x) {
- }
-
- public void endVisit(JsPostfixOperation x) {
- }
-
- public void endVisit(JsPrefixOperation x) {
- }
-
- public void endVisit(JsProgram x) {
- }
-
- public void endVisit(JsPropertyInitializer x) {
- }
-
- public void endVisit(JsRegExp x) {
- }
-
- public void endVisit(JsReturn x) {
- }
-
- public void endVisit(JsStringLiteral x) {
- }
-
- public void endVisit(JsSwitch x) {
- }
-
- public void endVisit(JsThisRef x) {
- }
-
- public void endVisit(JsThrow x) {
- }
-
- public void endVisit(JsTry x) {
- }
-
- public void endVisit(JsVars x) {
- }
-
- public void endVisit(JsWhile x) {
- }
-}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java b/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java
index 2173aff..98453ce 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java
@@ -46,7 +46,7 @@
public static final JsBinaryOperator GT = create(">", 10, LEFT_INFIX);
public static final JsBinaryOperator GTE = create(">=", 10, LEFT_INFIX);
public static final JsBinaryOperator INSTANCEOF = create("instanceof", 10,
- LEFT_INFIX);
+ LEFT_INFIX);
public static final JsBinaryOperator INOP = create("in", 10, LEFT_INFIX);
public static final JsBinaryOperator EQ = create("==", 9, LEFT_INFIX);
@@ -81,12 +81,18 @@
public static final JsBinaryOperator ASG_BIT_XOR = create("^=", 2, INFIX);
public static final JsBinaryOperator COMMA = create(",", 1, LEFT_INFIX);
-
+
private static JsBinaryOperator create(String symbol, int precedence, int mask) {
JsBinaryOperator op = new JsBinaryOperator(symbol, precedence, mask);
return op;
}
+
private JsBinaryOperator(String symbol, int precedence, int mask) {
super(symbol, precedence, mask);
}
+
+ public boolean isKeyword() {
+ return this == INSTANCEOF || this == INOP;
+ }
+
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsDelete.java b/dev/core/src/com/google/gwt/dev/js/ast/JsDelete.java
deleted file mode 100644
index 8390295..0000000
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsDelete.java
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * Copyright 2006 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.ast;
-
-/**
- * Represents the JavaScript delete statement.
- */
-public class JsDelete extends JsExpression {
-
- private JsExpression toDelete;
-
- public JsDelete() {
- }
-
- public JsExpression getExpr() {
- return toDelete;
- }
-
- public void setExpr(JsArrayAccess arrayElem) {
- this.toDelete = arrayElem;
- }
-
- public void setExpr(JsNameRef nameRef) {
- this.toDelete = nameRef;
- }
-
- public void traverse(JsVisitor v) {
- if (v.visit(this)) {
- toDelete.traverse(v);
- }
- v.endVisit(this);
- }
-}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsOperator.java b/dev/core/src/com/google/gwt/dev/js/ast/JsOperator.java
index 5fd7aac..dc04e21 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsOperator.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsOperator.java
@@ -18,7 +18,7 @@
/**
* A JavaScript operator.
*/
-public class JsOperator {
+public abstract class JsOperator {
protected static final int LEFT = 0x01;
protected static final int INFIX = 0x02;
@@ -45,6 +45,8 @@
return symbol;
}
+ public abstract boolean isKeyword();
+
public boolean isLeftAssociative() {
return (mask & LEFT) != 0;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java b/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java
index c5c0ea2..7c17de9 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java
@@ -28,12 +28,9 @@
public static final JsUnaryOperator NOT = create("!", 14, PREFIX);
public static final JsUnaryOperator DEC = create("--", 14, POSTFIX | PREFIX);
public static final JsUnaryOperator INC = create("++", 14, POSTFIX | PREFIX);
-// 'delete' is modeled as JsDelete
-// public static final JsUnaryOperator DELETE = create("delete", 14, PREFIX);
- // TODO(later): Keep the trailing space on "typeof" until jsgen is better
- public static final JsUnaryOperator TYPEOF = create("typeof ", 14, PREFIX);
- // TODO(later): Keep the trailing space on "void" until jsgen is better
- public static final JsUnaryOperator VOID = create("void ", 14, PREFIX);
+ public static final JsUnaryOperator DELETE = create("delete", 14, PREFIX);
+ public static final JsUnaryOperator TYPEOF = create("typeof", 14, PREFIX);
+ public static final JsUnaryOperator VOID = create("void", 14, PREFIX);
private static JsUnaryOperator create(String symbol, int precedence, int mask) {
JsUnaryOperator op = new JsUnaryOperator(symbol, precedence, mask);
@@ -43,4 +40,8 @@
private JsUnaryOperator(String symbol, int precedence, int mask) {
super(symbol, precedence, mask);
}
+
+ public boolean isKeyword() {
+ return this == DELETE || this == TYPEOF || this == VOID;
+ }
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java b/dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java
index c03ff52..df4ebee 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java
@@ -44,8 +44,6 @@
void endVisit(JsDefault x);
- void endVisit(JsDelete x);
-
void endVisit(JsDoWhile while1);
void endVisit(JsEmpty x);
@@ -128,8 +126,6 @@
boolean visit(JsDefault x);
- boolean visit(JsDelete x);
-
boolean visit(JsDoWhile x);
boolean visit(JsEmpty x);
