Skip to content

Commit

Permalink
Disallow redeclaration with function declarations in blocks.
Browse files Browse the repository at this point in the history 
Fixes #2755

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=179079879
  • Loading branch information
@brad4d
tbreisacher authored and brad4d committed Dec 15, 2017
1 parent 073ee36 commit f5e3ed7
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 27 deletions.
19 changes: 15 additions & 4 deletions src/com/google/javascript/jscomp/VarCheck.java
View file
Expand Up @@ -81,10 +81,10 @@ class VarCheck extends AbstractPostOrderCallback implements
"JSC_VAR_ARGUMENTS_SHADOWED_ERROR", "JSC_VAR_ARGUMENTS_SHADOWED_ERROR",
"Shadowing \"arguments\" is not allowed"); "Shadowing \"arguments\" is not allowed");


static final DiagnosticType LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR = static final DiagnosticType BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR =
DiagnosticType.error( DiagnosticType.error(
"JSC_LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR", "JSC_BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR",
"Duplicate let / const / class declaration in the same scope is not allowed."); "Duplicate let / const / class / function declaration in the same scope is not allowed.");


// The arguments variable is special, in that it's declared in every local // The arguments variable is special, in that it's declared in every local
// scope, but not explicitly declared. // scope, but not explicitly declared.
Expand Down Expand Up @@ -412,7 +412,18 @@ public void onRedeclaration(
|| parent.isClass() || parent.isClass()
|| (origParent != null || (origParent != null
&& (origParent.isLet() || origParent.isConst() || origParent.isClass()))) { && (origParent.isLet() || origParent.isConst() || origParent.isClass()))) {
compiler.report(JSError.make(n, LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR)); compiler.report(JSError.make(n, BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR));
return;
} else if (parent.isFunction()
// Redeclarations of functions in global scope are fairly common, so allow them
// (at least for now).
&& !s.isGlobal()
&& origParent != null
&& (origParent.isFunction()
|| origParent.isLet()
|| origParent.isConst()
|| origParent.isClass())) {
compiler.report(JSError.make(n, BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR));
return; return;
} }


Expand Down
79 changes: 56 additions & 23 deletions test/com/google/javascript/jscomp/VarCheckTest.java
View file
Expand Up @@ -18,6 +18,7 @@


import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.javascript.jscomp.VarCheck.BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR;
import static com.google.javascript.jscomp.VarCheck.VAR_MULTIPLY_DECLARED_ERROR; import static com.google.javascript.jscomp.VarCheck.VAR_MULTIPLY_DECLARED_ERROR;


import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -156,26 +157,26 @@ public void testMultiplyDeclaredVars4() {
} }


public void testMultiplyDeclaredLets() { public void testMultiplyDeclaredLets() {
testError("let x = 1; let x = 2;", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("let x = 1; let x = 2;", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("let x = 1; var x = 2;", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("let x = 1; var x = 2;", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("var x = 1; let x = 2;", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("var x = 1; let x = 2;", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
} }


public void testMultiplyDeclaredConsts() { public void testMultiplyDeclaredConsts() {
testError("const x = 1; const x = 2;", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("const x = 1; const x = 2;", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("const x = 1; var x = 2;", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("const x = 1; var x = 2;", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("var x = 1; const x = 2;", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("var x = 1; const x = 2;", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
} }


public void testMultiplyDeclaredConsts_withES6Modules() { public void testMultiplyDeclaredConsts_withES6Modules() {
testError("export function f() { const x = 1; const x = 2; }", testError("export function f() { const x = 1; const x = 2; }",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);


testError("export const x = 1; export var x = 2;", testError("export const x = 1; export var x = 2;",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);


testError("export const a = 1, a = 2;", testError("export const a = 1, a = 2;",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
} }


public void testMultiplyDeclareLetsInDifferentScope() { public void testMultiplyDeclareLetsInDifferentScope() {
Expand All @@ -184,12 +185,12 @@ public void testMultiplyDeclareLetsInDifferentScope() {
} }


public void testReferencedVarDefinedClass() { public void testReferencedVarDefinedClass() {
testError("var x; class x{ }", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("var x; class x{ }", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("let x; class x{ }", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("let x; class x{ }", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("const x = 1; class x{ }", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("const x = 1; class x{ }", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("class x{ } let x;", VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); testError("class x{ } let x;", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("export default class x{ } let x;", testError("export default class x{ } let x;",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
} }


public void testNamedClass() { public void testNamedClass() {
Expand Down Expand Up @@ -575,11 +576,11 @@ public void testDuplicateVar() {
public void testDontAllowSuppressDupeOnLet() { public void testDontAllowSuppressDupeOnLet() {
testError( testError(
"let a; /** @suppress {duplicate} */ let a; ", "let a; /** @suppress {duplicate} */ let a; ",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);


testError( testError(
"function f() { let a; /** @suppress {duplicate} */ let a; }", "function f() { let a; /** @suppress {duplicate} */ let a; }",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
} }


public void testDuplicateBlockScopedDeclarationInSwitch() { public void testDuplicateBlockScopedDeclarationInSwitch() {
Expand All @@ -595,7 +596,7 @@ public void testDuplicateBlockScopedDeclarationInSwitch() {
" break;", " break;",
" }", " }",
"}"), "}"),
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);


testError( testError(
lines( lines(
Expand All @@ -609,20 +610,20 @@ public void testDuplicateBlockScopedDeclarationInSwitch() {
" break;", " break;",
" }", " }",
"}"), "}"),
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
} }


public void testLetConstRedeclareWithFunctions_withEs6Modules() { public void testLetConstRedeclareWithFunctions_withEs6Modules() {
testError("function f() {} let f = 1; export {f};", testError("function f() {} let f = 1; export {f};",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("let f = 1; function f() {} export {f};", testError("let f = 1; function f() {} export {f};",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("const f = 1; function f() {} export {f};", testError("const f = 1; function f() {} export {f};",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("function f() {} const f = 1; export {f};", testError("function f() {} const f = 1; export {f};",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError("export default function f() {}; let f = 5;", testError("export default function f() {}; let f = 5;",
VarCheck.LET_CONST_CLASS_MULTIPLY_DECLARED_ERROR); BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
} }


public void testFunctionScopeArguments() { public void testFunctionScopeArguments() {
Expand All @@ -637,6 +638,38 @@ public void testFunctionScopeArguments() {
testSame("function f() {var arguments}"); testSame("function f() {var arguments}");
} }


public void testFunctionRedeclared_global() {
// Redeclaration in global scope is allowed.
testSame("/** @fileoverview @suppress {duplicate} */ function f() {};function f() {};");
}

public void testFunctionRedeclared1() {
testError("{ function f() {}; function f() {}; }", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError(
"if (0) { function f() {}; function f() {}; }", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError(
"try { function f() {}; function f() {}; } catch (e) {}",
BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
}

public void testFunctionRedeclared2() {
testError("{ let f = 0; function f() {}; }", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError(
"if (0) { const f = 1; function f() {}; }", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError(
"try { class f {}; function f() {}; } catch (e) {}",
BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
}

public void testFunctionRedeclared3() {
testError("{ function f() {}; let f = 0; }", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError(
"if (0) { function f() {}; const f = 0;}", BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
testError(
"try { function f() {}; class f {}; } catch (e) {}",
BLOCK_SCOPED_DECL_MULTIPLY_DECLARED_ERROR);
}

public void testNoUndeclaredVarWhenUsingClosurePass() { public void testNoUndeclaredVarWhenUsingClosurePass() {
enableClosurePass(); enableClosurePass();
// We don't want to get goog as an undeclared var here. // We don't want to get goog as an undeclared var here.
Expand Down

0 comments on commit f5e3ed7

Please sign in to comment.