Revision: 3873
Author: [email protected]
Date: Mon Nov 30 17:29:53 2009
Log: Fix rendering of Switch statements
http://codereview.appspot.com/159052
Previously,
switch (foo) {
case 1:
bar();
break;
}
would render as
switch (foo) {
case 1: {
bar();
break;
}
}
and many switch statements rendered with spurious noops.
This fixes that, and makes the switch body getter available on the
ancestor class of both default: and case: statements.
And made sure that a program consisting of only a directive prologue
does not fail to parse.
[email protected]
http://code.google.com/p/google-caja/source/detail?r=3873
Modified:
/trunk/src/com/google/caja/parser/js/CaseStmt.java
/trunk/src/com/google/caja/parser/js/DefaultCaseStmt.java
/trunk/src/com/google/caja/parser/js/Parser.java
/trunk/src/com/google/caja/parser/js/SwitchCase.java
/trunk/src/com/google/caja/parser/js/SwitchStmt.java
/trunk/tests/com/google/caja/parser/js/rendergolden1.txt
=======================================
--- /trunk/src/com/google/caja/parser/js/CaseStmt.java Thu Oct 15 15:42:13
2009
+++ /trunk/src/com/google/caja/parser/js/CaseStmt.java Mon Nov 30 17:29:53
2009
@@ -15,7 +15,6 @@
package com.google.caja.parser.js;
import com.google.caja.lexer.FilePosition;
-import com.google.caja.lexer.TokenConsumer;
import com.google.caja.parser.ParseTreeNode;
import com.google.caja.reporting.RenderContext;
@@ -55,7 +54,14 @@
public Expression getCaseValue() { return caseValue; }
+ @Override
public Statement getBody() { return body; }
+
+ @Override
+ protected void renderHead(RenderContext rc) {
+ rc.getOut().consume("case");
+ caseValue.render(rc);
+ }
@Override
protected void childrenChanged() {
@@ -67,14 +73,4 @@
@Override
public Object getValue() { return null; }
-
- public void render(RenderContext rc) {
- TokenConsumer out = rc.getOut();
- out.mark(getFilePosition());
- out.consume("case");
- caseValue.render(rc);
- out.consume(":");
- out.consume("\n");
- body.renderBlock(rc, false);
- }
-}
+}
=======================================
--- /trunk/src/com/google/caja/parser/js/DefaultCaseStmt.java Thu Oct 15
15:42:13 2009
+++ /trunk/src/com/google/caja/parser/js/DefaultCaseStmt.java Mon Nov 30
17:29:53 2009
@@ -15,7 +15,6 @@
package com.google.caja.parser.js;
import com.google.caja.lexer.FilePosition;
-import com.google.caja.lexer.TokenConsumer;
import com.google.caja.reporting.RenderContext;
import java.util.List;
@@ -48,14 +47,11 @@
@Override
public Object getValue() { return null; }
+ @Override
public Statement getBody() { return body; }
- public void render(RenderContext rc) {
- TokenConsumer out = rc.getOut();
- out.mark(getFilePosition());
- out.consume("default");
- out.consume(":");
- out.consume("\n");
- body.renderBlock(rc, false);
+ @Override
+ protected void renderHead(RenderContext rc) {
+ rc.getOut().consume("default");
}
}
=======================================
--- /trunk/src/com/google/caja/parser/js/Parser.java Thu Nov 19 15:57:15
2009
+++ /trunk/src/com/google/caja/parser/js/Parser.java Mon Nov 30 17:29:53
2009
@@ -73,16 +73,15 @@
*
* <xmp>
* // Terminal productions
- * Program => <UseSubsetDirective>? <TerminatedStatement>*
+ * Program => <DirectivePrologue>? <TerminatedStatement>*
* Statement => <TerminalStatement>
* | <NonTerminalStatement>
* Expression => <CommaOperator>
*
* // Non-terminal productions. Indentation indicates that a nonterminal
is
* // used only by the unindented nonterminal above it.
- * UseSubsetDirective => '"' 'use' <UseSubsetList>? '"'
- * UseSubsetList => <UseSubset> <UseSubsetTail>*
- * UseSubsetTail => ',' <UseSubset>
+ * DirectivePrologue => <Directive> <DirectivePrologue>?
+ * Directive => <StringLiteral> ';'
* TerminatedStatement => <BlockStatement>
* | <SimpleStatement> <StatementTerminator>
* StatementTerminator => ';'
@@ -323,62 +322,51 @@
Mark startOfPrologue = tq.mark();
List<Directive> directives = Lists.newArrayList();
- while (tq.peek().type == JsTokenType.STRING) {
+ while (!tq.isEmpty() && tq.peek().type == JsTokenType.STRING) {
Mark startOfDirective = tq.mark();
Token<JsTokenType> quotedString = tq.pop();
- boolean isDirective = false;
- switch(tq.peek().type) {
- case STRING:
- case REGEXP:
- case WORD:
- case INTEGER:
- case FLOAT:
- case KEYWORD:
- case COMMENT:
- isDirective = true;
- mq.addMessage(
- MessageType.SEMICOLON_INSERTED,
- FilePosition.endOf(quotedString.pos));
+ if (!tq.checkToken(Punctuation.SEMI)) {
+ Token<JsTokenType> t = !tq.isEmpty() ? tq.peek() : null;
+ if ((t == null || !continuesExpr(t.text)) && semicolonInserted()) {
+ FilePosition semiPoint = FilePosition.endOf(tq.lastPosition());
+ MessageLevel lvl = tq.isEmpty()
+ || tq.lookaheadToken(Punctuation.RCURLY)
+ ? MessageLevel.LOG : MessageLevel.LINT;
+ mq.addMessage(MessageType.SEMICOLON_INSERTED, lvl, semiPoint);
+ } else {
+ tq.rewind(startOfDirective);
break;
- case PUNCTUATION:
- if (tq.lookaheadToken(Punctuation.SEMI)) {
- isDirective = true;
- tq.pop();
- } else if (tq.lookaheadToken(Punctuation.RCURLY)) {
- isDirective = true;
- mq.addMessage(
- MessageType.SEMICOLON_INSERTED,
- FilePosition.endOf(quotedString.pos));
- }
- break;
- case LINE_CONTINUATION:
- break;
+ }
}
- if (!isDirective) {
- tq.rewind(startOfDirective);
- break;
- }
-
+ String unquoted = quotedString.text.substring(
+ 1, quotedString.text.length() - 1);
String decoded = StringLiteral.getUnquotedValueOf(quotedString.text);
- if (!Directive.RecognizedValue.isDirectiveStringRecognized(decoded))
{
+ if (!unquoted.equals(decoded)
+ |
| !Directive.RecognizedValue.isDirectiveStringRecognized(unquoted)) {
mq.addMessage(
MessageType.UNRECOGNIZED_DIRECTIVE_IN_PROLOGUE,
quotedString.pos, MessagePart.Factory.valueOf(decoded));
}
- directives.add(new Directive(
- FilePosition.startOf(quotedString.pos), decoded));
+ Directive d = new Directive(posFrom(quotedString.pos), decoded);
+ finish(d, startOfDirective);
+ directives.add(d);
}
if (directives.isEmpty()) { return null; }
- DirectivePrologue prologue =
- new DirectivePrologue(posFrom(startOfPrologue), directives);
+ DirectivePrologue prologue = new DirectivePrologue(
+ posFrom(startOfPrologue), directives);
finish(prologue, startOfPrologue);
return prologue;
}
+ private static boolean continuesExpr(String tokenText) {
+ return Operator.lookupOperation(tokenText, OperatorType.INFIX) != null
+ || Operator.lookupOperation(tokenText, OperatorType.BRACKET) !=
null
+ || Operator.lookupOperation(tokenText, OperatorType.TERNARY) !=
null;
+ }
private LabeledStatement parseLoopOrSwitch(FilePosition start, String
label)
throws ParseException {
=======================================
--- /trunk/src/com/google/caja/parser/js/SwitchCase.java Thu Nov 12
13:05:03 2009
+++ /trunk/src/com/google/caja/parser/js/SwitchCase.java Mon Nov 30
17:29:53 2009
@@ -15,7 +15,9 @@
package com.google.caja.parser.js;
import com.google.caja.lexer.FilePosition;
+import com.google.caja.lexer.TokenConsumer;
import com.google.caja.parser.ParseTreeNode;
+import com.google.caja.reporting.RenderContext;
/**
* Base class for case and default blocks.
@@ -25,4 +27,22 @@
protected SwitchCase(FilePosition pos) { super(pos,
ParseTreeNode.class); }
public boolean hasHangingConditional() { return false; }
-}
+
+ public abstract Statement getBody();
+
+ protected abstract void renderHead(RenderContext rc);
+
+ public final void render(RenderContext rc) {
+ TokenConsumer out = rc.getOut();
+ out.mark(getFilePosition());
+ renderHead(rc);
+ out.consume(":");
+ out.consume("\n");
+ Statement body = getBody();
+ if (body instanceof Block) {
+ ((Block) body).renderBody(rc);
+ } else if (!(body instanceof Noop)) {
+ body.renderBlock(rc, true);
+ }
+ }
+}
=======================================
--- /trunk/src/com/google/caja/parser/js/SwitchStmt.java Thu Nov 12
13:05:03 2009
+++ /trunk/src/com/google/caja/parser/js/SwitchStmt.java Mon Nov 30
17:29:53 2009
@@ -91,10 +91,6 @@
while (it.hasNext()) {
SwitchCase caseStmt = (SwitchCase) it.next();
caseStmt.render(rc);
- if (!caseStmt.isTerminal()) {
- out.mark(FilePosition.endOfOrNull(caseStmt.getFilePosition()));
- out.consume(";");
- }
}
out.mark(FilePosition.endOfOrNull(getFilePosition()));
out.consume("}");
=======================================
--- /trunk/tests/com/google/caja/parser/js/rendergolden1.txt Mon Jan 26
14:35:22 2009
+++ /trunk/tests/com/google/caja/parser/js/rendergolden1.txt Mon Nov 30
17:29:53 2009
@@ -26,14 +26,10 @@
case 2:
if (a === 4) { break; }
case BAR:
- {
- f();
- g();
- }
+ f();
+ g();
default:
- {
- zowie(wowie());;;
- }
+ zowie(wowie());;;
}
for (var i in array) {
f(i);
@@ -44,12 +40,9 @@
var o = new Object;
switch (n) {
case 0:
- ;
case 1:
- {
- { foo(); }
- break;
- }
+ { foo(); }
+ break;
default:
panic();
case 2:
