Fixes issue #978.
- Removes extra whitespace and parentheses around negative numbers
- Removes unnecessary semicolons after the last statement of a block
Suggested by: fredsa
Patch by: sgross
Review by: me
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1805 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/js/JsRequiresSemiVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsRequiresSemiVisitor.java
new file mode 100644
index 0000000..87f46ff
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/js/JsRequiresSemiVisitor.java
@@ -0,0 +1,148 @@
+/*
+ * Copyright 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.gwt.dev.js;
+
+import com.google.gwt.dev.js.ast.JsBlock;
+import com.google.gwt.dev.js.ast.JsBreak;
+import com.google.gwt.dev.js.ast.JsContext;
+import com.google.gwt.dev.js.ast.JsDebugger;
+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.JsFor;
+import com.google.gwt.dev.js.ast.JsForIn;
+import com.google.gwt.dev.js.ast.JsIf;
+import com.google.gwt.dev.js.ast.JsLabel;
+import com.google.gwt.dev.js.ast.JsReturn;
+import com.google.gwt.dev.js.ast.JsStatement;
+import com.google.gwt.dev.js.ast.JsSwitch;
+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.JsVisitor;
+import com.google.gwt.dev.js.ast.JsWhile;
+
+/**
+ * Determines if a statement at the end of a block requires a semicolon.
+ *
+ * For example, the following statements require semicolons:<br>
+ * <ul>
+ * <li>if (cond);</li>
+ * <li>while (cond);</li>
+ * </ul>
+ *
+ * The following do not require semicolons:<br>
+ * <ul>
+ * <li>return 1</li>
+ * <li>do {} while(true)</li>
+ * </ul>
+ */
+public class JsRequiresSemiVisitor extends JsVisitor {
+
+ public static boolean exec(JsStatement lastStatement) {
+ JsRequiresSemiVisitor visitor = new JsRequiresSemiVisitor();
+ visitor.accept(lastStatement);
+ return visitor.needsSemicolon;
+ }
+
+ private boolean needsSemicolon = false;
+
+ private JsRequiresSemiVisitor() {
+ }
+
+ public boolean visit(JsBlock x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsBreak x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsDebugger x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsDoWhile x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsEmpty x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsExprStmt x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsFor x, JsContext<JsStatement> ctx) {
+ if (x.getBody() instanceof JsEmpty) {
+ needsSemicolon = true;
+ }
+ return false;
+ }
+
+ public boolean visit(JsForIn x, JsContext<JsStatement> ctx) {
+ if (x.getBody() instanceof JsEmpty) {
+ needsSemicolon = true;
+ }
+ return false;
+ }
+
+ public boolean visit(JsIf x, JsContext<JsStatement> ctx) {
+ JsStatement thenStmt = x.getThenStmt();
+ JsStatement elseStmt = x.getElseStmt();
+ if (elseStmt instanceof JsEmpty
+ || (elseStmt == null && thenStmt instanceof JsEmpty)) {
+ needsSemicolon = true;
+ }
+ return false;
+ }
+
+ public boolean visit(JsLabel x, JsContext<JsStatement> ctx) {
+ if (x.getStmt() instanceof JsEmpty) {
+ needsSemicolon = true;
+ }
+ return false;
+ }
+
+ public boolean visit(JsReturn x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsSwitch x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsThrow x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsTry x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsVars x, JsContext<JsStatement> ctx) {
+ return false;
+ }
+
+ public boolean visit(JsWhile x, JsContext<JsStatement> ctx) {
+ if (x.getBody() instanceof JsEmpty) {
+ needsSemicolon = true;
+ }
+ 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 bc035ff..23ba302 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2007 Google Inc.
+ * Copyright 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
@@ -301,12 +301,7 @@
@Override
public boolean visit(JsDecimalLiteral x, JsContext<JsExpression> ctx) {
- String s = x.getValue();
- // TODO: optimize this to only the cases that need it
- if (s.startsWith("-")) {
- _space();
- }
- p.print(s);
+ p.print(x.getValue());
return false;
}
@@ -510,15 +505,7 @@
@Override
public boolean visit(JsIntegralLiteral x, JsContext<JsExpression> ctx) {
- String s = x.getValue().toString();
- boolean needParens = s.startsWith("-");
- if (needParens) {
- _lparen();
- }
- p.print(s);
- if (needParens) {
- _rparen();
- }
+ p.print(x.getValue().toString());
return false;
}
@@ -823,17 +810,30 @@
accept(stmt);
if (needSemi) {
/*
- * Special treatment of function decls: function decls always set
- * needSemi back to true. But if they are the only item in a statement
- * (i.e. not part of an assignment operation), just give them a newline
- * instead of a semi since it makes obfuscated code so much "nicer"
- * (sic).
+ * Special treatment of function decls: If they are the only item in a
+ * statement (i.e. not part of an assignment operation), just give them
+ * a newline instead of a semi.
*/
- if (stmt instanceof JsExprStmt
- && ((JsExprStmt) stmt).getExpression() instanceof JsFunction) {
- _newline();
+ boolean functionStmt = stmt instanceof JsExprStmt
+ && ((JsExprStmt) stmt).getExpression() instanceof JsFunction;
+ /*
+ * Special treatment of the last statement in a block: only a few
+ * statements at the end of a block require semicolons.
+ */
+ boolean lastStatement = !iter.hasNext() && needBraces
+ && !JsRequiresSemiVisitor.exec(stmt);
+ if (functionStmt) {
+ if (lastStatement) {
+ _newlineOpt();
+ } else {
+ _newline();
+ }
} else {
- _semi();
+ if (lastStatement) {
+ _semiOpt();
+ } else {
+ _semi();
+ }
_newlineOpt();
}
}
@@ -1070,6 +1070,10 @@
p.print(';');
}
+ private void _semiOpt() {
+ p.printOpt(';');
+ }
+
private boolean _sepCommaOptSpace(boolean sep) {
if (sep) {
p.print(',');
@@ -1096,6 +1100,16 @@
&& (op2 == JsUnaryOperator.DEC || op2 == JsUnaryOperator.NEG)
|| (op == JsBinaryOperator.ADD && op2 == JsUnaryOperator.INC);
}
+ if (arg instanceof JsIntegralLiteral) {
+ JsIntegralLiteral literal = (JsIntegralLiteral) arg;
+ return (op == JsBinaryOperator.SUB || op == JsUnaryOperator.NEG)
+ && (literal.getValue().signum() == -1);
+ }
+ if (arg instanceof JsDecimalLiteral) {
+ JsDecimalLiteral literal = (JsDecimalLiteral) arg;
+ return (op == JsBinaryOperator.SUB || op == JsUnaryOperator.NEG)
+ && (literal.getValue().startsWith("-"));
+ }
return false;
}
diff --git a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java
index fe4041a..82c62e2 100644
--- a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2007 Google Inc.
+ * Copyright 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
@@ -108,6 +108,14 @@
doTest("var x = -(-(--y))");
}
+ public void testEmptyStatements() throws Exception {
+ doTest("function f() {if (x);}");
+ doTest("function f() {while (x);}");
+ doTest("function f() {label:;}");
+ doTest("function f() {for (i=0;i<n;i++);}");
+ doTest("function f() {for (var x in s);}");
+ }
+
private void doTest(String js) throws Exception {
List<JsStatement> expected = parser.parse(new JsProgram().getScope(),
new StringReader(js), 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
index a0943a7..edcf1c8 100644
--- a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorConcisenessTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorConcisenessTest.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2007 Google Inc.
+ * Copyright 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
@@ -68,6 +68,13 @@
assertEquals("var x={1.2:'b'}", parse("var x = {1.2 : 'b'}"));
}
+ public void testLastStatement() throws Exception {
+ assertEquals("function(){x++;i++}", parse("function() {x++;i++;}"));
+ assertEquals("function(){do{}while(x)}",
+ parse("function() {do{} while(x);}"));
+ assertEquals("function(){if(b){}}", parse("function() {if(b){}}"));
+ }
+
private String parse(String js) throws Exception {
List<JsStatement> statements = parser.parse(new JsProgram().getScope(),
new StringReader(js), 0);
