Skip to content

Commit

Permalink
Report invalid labelled let and const declarations
Browse files Browse the repository at this point in the history 
This is a SyntaxError because, using the spec's language, only Statement nodes are allowed to be in labels and let/const are LexicalDeclarations but not Statements.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=226039040
  • Loading branch information
@lauraharker @blickly
lauraharker authored and blickly committed Dec 19, 2018
1 parent c1ce398 commit b488e51
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 31 deletions.
1 change: 1 addition & 0 deletions src/com/google/javascript/jscomp/VariableReferenceCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class VariableReferenceCheck implements HotSwapCompilerPass {

// These types do not permit a block-scoped declaration inside them without an explicit block.
// e.g. if (b) let x;
// This list omits Token.LABEL intentionally. It's handled differently in IRFactory.
private static final ImmutableSet<Token> BLOCKLESS_DECLARATION_FORBIDDEN_STATEMENTS =
Sets.immutableEnumSet(Token.IF, Token.FOR, Token.FOR_IN, Token.FOR_OF, Token.WHILE);

Expand Down
16 changes: 9 additions & 7 deletions src/com/google/javascript/jscomp/parsing/IRFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1607,14 +1607,16 @@ Node processThisExpression(ThisExpressionTree node) {

Node processLabeledStatement(LabelledStatementTree labelTree) {
Node statement = transform(labelTree.statement);
if (statement.isFunction()) {
if (statement.isFunction()
|| statement.isClass()
|| statement.isLet()
|| statement.isConst()) {
errorReporter.error(
"Functions can only be declared at top level or inside a block.",
sourceName, lineno(labelTree), charno(labelTree));
} else if (statement.isClass()) {
errorReporter.error(
"Classes can only be declared at top level or inside a block.",
sourceName, lineno(labelTree), charno(labelTree));
"Lexical declarations are only allowed at top level or inside a block.",
sourceName,
lineno(labelTree),
charno(labelTree));
return statement; // drop the LABEL node so that the resulting AST is valid
}
return newNode(Token.LABEL,
transformLabelName(labelTree.name),
Expand Down
3 changes: 1 addition & 2 deletions test/com/google/javascript/jscomp/Es6SplitVariableDeclarationsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ public void testCannotSplitInForLoopInitializer() {
@Test
public void testCannotSplitLabeledDeclaration() {
testError("label: var [a] = [], b = 3;", Es6ToEs3Util.CANNOT_CONVERT_YET);
testError("label: let [a] = [], b = 3;", Es6ToEs3Util.CANNOT_CONVERT_YET);
testError("label: const [a] = [], b = 3;", Es6ToEs3Util.CANNOT_CONVERT_YET);
// `label: let [a] = [], b = 3;` and `label: const [a] = [], b = 3;` are syntax errors
}

@Test
Expand Down
81 changes: 68 additions & 13 deletions test/com/google/javascript/jscomp/LiveVariablesAnalysisTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,13 +338,13 @@ public void testBug1449316() {
@Test
public void testSimpleLet() {
// a is defined after X and not used
assertNotLiveBeforeX("X:let a;", "a");
assertNotLiveAfterX("X:let a;", "a");
assertNotLiveAfterX("X:let a=1;", "a");
assertNotLiveBeforeDecl("let a;", "a");
assertNotLiveAfterDecl("let a;", "a");
assertNotLiveAfterDecl("let a=1;", "a");

// a is used and defined after X
assertLiveAfterX("X:let a=1; a()", "a");
assertNotLiveBeforeX("X:let a=1; a()", "a");
assertLiveAfterDecl("let a=1; a()", "a");
assertNotLiveBeforeDecl("let a=1; a()", "a");

// no assignment to x; let is initialized with undefined
assertLiveBeforeX("let a;X:a;", "a");
Expand All @@ -368,9 +368,9 @@ public void testLetInnerBlock() {
public void testSimpleConst() {
// a is defined after X and not used
assertLiveBeforeX("const a = 4; X:a;", "a");
assertNotLiveBeforeX("X:let a = 1;", "a");
assertNotLiveBeforeX("X:const a = 1;", "a");
assertNotLiveAfterX("X:const a = 1;", "a");
assertNotLiveBeforeDecl("let a = 1;", "a");
assertNotLiveBeforeDecl("const a = 1;", "a");
assertNotLiveAfterDecl("const a = 1;", "a");
}

@Test
Expand All @@ -383,7 +383,7 @@ public void testArrayDestructuring() {
assertNotEscaped("var [a, ,b] = [1, 2, 3];", "b");
assertNotLiveBeforeX("var x = 3; X: [x] = [4]; x;", "x");
assertLiveBeforeX("var x = {}; X: [x.a] = [3]; x.a;", "x");
assertLiveBeforeX("var x = []; X: const [c] = x;", "x");
assertLiveBeforeX("var x = []; X: var [c] = x;", "x");
}

@Test
Expand All @@ -395,7 +395,7 @@ public void testObjectDestructuring() {
assertNotEscaped("var {a: x = 3, b: y} = g();", "x");
assertNotLiveBeforeX("var x = {}; X: ({x} = {}); x;", "x");
assertLiveBeforeX("var x = {}; X: ({a: x.a} = {}); x.a;", "x");
assertLiveBeforeX("var x = {}; X: const {c} = x;", "x");
assertLiveBeforeX("var x = {}; X: var {c} = x;", "x");
}

@Test
Expand All @@ -404,8 +404,8 @@ public void testComplexDestructuringPattern() {
assertLiveBeforeX("var x = 3, y; X: [y = x] = [];", "x");
assertLiveBeforeX("var x = 3; X: var {y = x} = {};", "x");
assertLiveBeforeX("var x = 3; X: var {key: y = x} = {};", "x");
assertLiveBeforeX("var x = 3; X: const {[x + x]: foo} = obj; x;", "x");
assertLiveBeforeX("var x = 3; X: const {[x + x]: x} = obj; x;", "x");
assertLiveBeforeX("var x = 3; X: var {[x + x]: foo} = obj; x;", "x");
assertLiveBeforeX("var x = 3; X: var {[x + x]: x} = obj; x;", "x");
}

@Test
Expand Down Expand Up @@ -442,7 +442,34 @@ private void assertNotLiveAfterX(String src, String var) {
private void assertNotLiveBeforeX(String src, String var) {
FlowState<LiveVariablesAnalysis.LiveVariableLattice> state = getFlowStateAtX(src);
assertWithMessage("Label X should be in the input program.").that(state).isNotNull();
assertWithMessage("Variable" + var + " should not be live before X")
assertWithMessage("Variable " + var + " should not be live before X")
.that(state.getIn().isLive(liveness.getVarIndex(var)))
.isFalse();
}

private void assertLiveAfterDecl(String src, String var) {
FlowState<LiveVariablesAnalysis.LiveVariableLattice> state =
getFlowStateAtDeclaration(src, var);
assertWithMessage("Variable " + var + " should be declared").that(state).isNotNull();
assertWithMessage("Variable" + var + " should be live after its declaration")
.that(state.getOut().isLive(liveness.getVarIndex(var)))
.isTrue();
}

private void assertNotLiveAfterDecl(String src, String var) {
FlowState<LiveVariablesAnalysis.LiveVariableLattice> state =
getFlowStateAtDeclaration(src, var);
assertWithMessage("Variable " + var + " should be declared").that(state).isNotNull();
assertWithMessage("Variable " + var + " should not be live after its declaration")
.that(state.getOut().isLive(liveness.getVarIndex(var)))
.isFalse();
}

private void assertNotLiveBeforeDecl(String src, String var) {
FlowState<LiveVariablesAnalysis.LiveVariableLattice> state =
getFlowStateAtDeclaration(src, var);
assertWithMessage("Variable " + var + " should be declared").that(state).isNotNull();
assertWithMessage("Variable " + var + " should not be live before its declaration")
.that(state.getIn().isLive(liveness.getVarIndex(var)))
.isFalse();
}
Expand All @@ -468,6 +495,34 @@ private FlowState<LiveVariablesAnalysis.LiveVariableLattice> getFlowStateAtX(
return null;
}

private FlowState<LiveVariablesAnalysis.LiveVariableLattice> getFlowStateAtDeclaration(
String src, String name) {
liveness = computeLiveness(src);
return getFlowStateAtDeclaration(
liveness.getCfg().getEntry().getValue(), liveness.getCfg(), name);
}

/**
* Use this for lexical declarations which can't be labelled; e.g. `LABEL: let x = 0;` is invalid
* syntax.
*/
private FlowState<LiveVariablesAnalysis.LiveVariableLattice> getFlowStateAtDeclaration(
Node node, ControlFlowGraph<Node> cfg, String name) {
if (NodeUtil.isNameDeclaration(node)) {
if (node.getFirstChild().getString().equals(name)) {
return cfg.getNode(node).getAnnotation();
}
}
for (Node c = node.getFirstChild(); c != null; c = c.getNext()) {
FlowState<LiveVariablesAnalysis.LiveVariableLattice> state =
getFlowStateAtDeclaration(c, cfg, name);
if (state != null) {
return state;
}
}
return null;
}

private static void assertEscaped(String src, String name) {
for (Var var : computeLiveness(src).getEscapedLocals()) {
if (var.name.equals(name)) {
Expand Down
3 changes: 0 additions & 3 deletions test/com/google/javascript/jscomp/MustBeReachingVariableDefTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ public void testStraightLine() {
assertNotMatch("D:var x; U:x; x=1; x");
assertMatch("D: var x = 1; var y = 2; y; U:x");

assertMatch("D:let x=1; U: x");
assertMatch("let x; D:x=1; U: x");

assertMatch("D: const x = 1; U: x");
}

@Test
Expand Down
6 changes: 4 additions & 2 deletions test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,10 @@ public void testFoldBlock() {
fold("for (x of y) {x}", "for(x of y);");
foldSame("for (let x = 1; x <10; x++ ) {}");
foldSame("for (var x = 1; x <10; x++ ) {}");
}

// Block with declarations
@Test
public void testFoldBlockWithDeclaration() {
foldSame("{let x}");
foldSame("function f() {let x}");
foldSame("{const x = 1}");
Expand All @@ -124,8 +126,8 @@ public void testFoldBlock() {
fold("{x = 4; {let y}}", "x = 4; {let y}");
foldSame("{function f() {} } {function f() {}}");
foldSame("{class C {}} {class C {}}");
foldSame("{label: let x}");
fold("{label: var x}", "label: var x");
// `{label: let x}` is a syntax error
foldSame("{label: var x; let y;}");
}

Expand Down
1 change: 0 additions & 1 deletion test/com/google/javascript/jscomp/StatementFusionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ public void testNoFuseIntoBlock() {
fuseSame("a; { b; const a = 1; }");
fuseSame("a; { b; class a {} }");
fuseSame("a; { b; function a() {} }");
fuseSame("a; { b; label: let a = 1; }");
fuseSame("a; { b; const otherVariable = 1; }");

enableNormalize();
Expand Down
20 changes: 17 additions & 3 deletions test/com/google/javascript/jscomp/parsing/ParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,14 +371,28 @@ public void testNonDuplicateLabelCrossFunction() {
@Test
public void testLabeledFunctionDeclaration() {
parseError(
"foo:function f() {}", "Functions can only be declared at top level or inside a block.");
"foo:function f() {}",
"Lexical declarations are only allowed at top level or inside a block.");
}

@Test
public void testLabeledClassDeclaration() {
mode = LanguageMode.ECMASCRIPT6;
parseError(
"foo:class Foo {}", "Classes can only be declared at top level or inside a block.");
"foo:class Foo {}",
"Lexical declarations are only allowed at top level or inside a block.");
}

@Test
public void testLabeledLetDeclaration() {
parseError(
"foo: let x = 0;", "Lexical declarations are only allowed at top level or inside a block.");
}

@Test
public void testLabeledConstDeclaration() {
parseError(
"foo: const x = 0;",
"Lexical declarations are only allowed at top level or inside a block.");
}

@Test
Expand Down

0 comments on commit b488e51

Please sign in to comment.