Skip to content

Commit

Permalink
Fix lint warnings and be more concise.
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=143447042
  • Loading branch information
@blickly
tbreisacher authored and blickly committed Jan 3, 2017
1 parent 1eda5f2 commit 0006772
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 56 deletions.
33 changes: 15 additions & 18 deletions src/com/google/javascript/jscomp/Normalize.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ class Normalize implements CompilerPass {
public static final DiagnosticType CATCH_BLOCK_VAR_ERROR =
DiagnosticType.error(
"JSC_CATCH_BLOCK_VAR_ERROR",
"The use of scope variable {0} is not allowed within a catch block " +
"with a catch exception of the same name.");
"The use of scope variable {0} is not allowed within a catch block"
+ " with a catch exception of the same name.");


Normalize(AbstractCompiler compiler, boolean assertOnChange) {
Expand Down Expand Up @@ -238,16 +238,16 @@ public void visit(NodeTraversal t, Node n, Node parent) {
}

boolean shouldBeConstant =
(info != null && info.isConstant()) ||
NodeUtil.isConstantByConvention(compiler.getCodingConvention(), n);
(info != null && info.isConstant())
|| NodeUtil.isConstantByConvention(compiler.getCodingConvention(), n);
boolean isMarkedConstant = n.getBooleanProp(Node.IS_CONSTANT_NAME);
if (shouldBeConstant && !isMarkedConstant) {
if (assertOnChange) {
String name = n.getString();
throw new IllegalStateException(
"Unexpected const change.\n" +
" name: "+ name + "\n" +
" parent:" + n.getParent().toStringTree());
"Unexpected const change.\n"
+ " name: " + name + "\n"
+ " parent:" + n.getParent().toStringTree());
}
n.putBooleanProp(Node.IS_CONSTANT_NAME, true);
}
Expand All @@ -261,8 +261,8 @@ public void visit(NodeTraversal t, Node n, Node parent) {
static class VerifyConstants extends AbstractPostOrderCallback
implements CompilerPass {

final private AbstractCompiler compiler;
final private boolean checkUserDeclarations;
private final AbstractCompiler compiler;
private final boolean checkUserDeclarations;

VerifyConstants(AbstractCompiler compiler, boolean checkUserDeclarations) {
this.compiler = compiler;
Expand Down Expand Up @@ -418,20 +418,17 @@ private void annotateConstantsByConvention(Node n, Node parent) {
// may be a variable reference: The right side of a GETPROP
// or an OBJECTLIT key.
boolean isObjLitKey = NodeUtil.isObjectLitKey(n);
boolean isProperty = isObjLitKey ||
(parent.isGetProp() &&
parent.getLastChild() == n);
boolean isProperty = isObjLitKey || (parent.isGetProp() && parent.getLastChild() == n);
if (n.isName() || isProperty) {
boolean isMarkedConstant = n.getBooleanProp(Node.IS_CONSTANT_NAME);
if (!isMarkedConstant &&
NodeUtil.isConstantByConvention(
compiler.getCodingConvention(), n)) {
if (!isMarkedConstant
&& NodeUtil.isConstantByConvention(compiler.getCodingConvention(), n)) {
if (assertOnChange) {
String name = n.getString();
throw new IllegalStateException(
"Unexpected const change.\n" +
" name: "+ name + "\n" +
" parent:" + n.getParent().toStringTree());
"Unexpected const change.\n"
+ " name: " + name + "\n"
+ " parent:" + n.getParent().toStringTree());
}
n.putBooleanProp(Node.IS_CONSTANT_NAME, true);
}
Expand Down
72 changes: 41 additions & 31 deletions test/com/google/javascript/jscomp/NormalizeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,18 +179,21 @@ public void testWhile() {
public void testMoveFunctions1() throws Exception {
test("function f() { if (x) return; foo(); function foo() {} }",
"function f() {function foo() {} if (x) return; foo(); }");
test("function f() { " +
"function foo() {} " +
"if (x) return;" +
"foo(); " +
"function bar() {} " +
"}",
"function f() {" +
"function foo() {}" +
"function bar() {}" +
"if (x) return;" +
"foo();" +
"}");
test(
LINE_JOINER.join(
"function f() { ",
"function foo() {} ",
"if (x) return;",
"foo(); ",
"function bar() {} ",
"}"),
LINE_JOINER.join(
"function f() {",
"function foo() {}",
"function bar() {}",
"if (x) return;",
"foo();",
"}"));
}

public void testMoveFunctions2() throws Exception {
Expand Down Expand Up @@ -356,23 +359,27 @@ public void testIssue166a() {
}

public void testIssue166b() {
testError("function a() {" +
"try { throw 1 } catch(e) { /** @suppress {duplicate} */ var e=2 }" +
"};",
testError(
LINE_JOINER.join(
"function a() {",
" try { throw 1 } catch(e) { /** @suppress {duplicate} */ var e=2 }",
"};"),
Normalize.CATCH_BLOCK_VAR_ERROR);
}

public void testIssue166c() {
testError("var e = 0; try { throw 1 } catch(e) {" +
"/** @suppress {duplicate} */ var e=2 }",
testError(
"var e = 0; try { throw 1 } catch(e) { /** @suppress {duplicate} */ var e=2 }",
Normalize.CATCH_BLOCK_VAR_ERROR);
}

public void testIssue166d() {
testError("function a() {" +
"var e = 0; try { throw 1 } catch(e) {" +
"/** @suppress {duplicate} */ var e=2 }" +
"};",
testError(
LINE_JOINER.join(
"function a() {",
" var e = 0;",
" try { throw 1 } catch(e) { /** @suppress {duplicate} */ var e=2 }",
"};"),
Normalize.CATCH_BLOCK_VAR_ERROR);
}

Expand All @@ -382,12 +389,17 @@ public void testIssue166e() {
}

public void testIssue166f() {
test("function a() {" +
"var e = 2; try { throw 1 } catch(e) {}" +
"}",
"function a() {" +
"var e = 2; try { throw 1 } catch(e$jscomp$1) {}" +
"}");
test(
LINE_JOINER.join(
"function a() {",
" var e = 2;",
" try { throw 1 } catch(e) {}",
"}"),
LINE_JOINER.join(
"function a() {",
" var e = 2;",
" try { throw 1 } catch(e$jscomp$1) {}",
"}"));
}

public void testIssue() {
Expand Down Expand Up @@ -442,8 +454,7 @@ public void testPropertyIsConstant2() throws Exception {
}

public void testGetterPropertyIsConstant() throws Exception {
testSame("var a = { get CONST() {return 3} }; " +
"var b = a.CONST;");
testSame("var a = { get CONST() {return 3} }; var b = a.CONST;");
Node n = getLastCompiler().getRoot();

Set<Node> constantNodes = findNodesWithProperty(n, Node.IS_CONSTANT_NAME);
Expand All @@ -455,8 +466,7 @@ public void testGetterPropertyIsConstant() throws Exception {

public void testSetterPropertyIsConstant() throws Exception {
// Verifying that a SET is properly annotated.
testSame("var a = { set CONST(b) {throw 'invalid'} }; " +
"var c = a.CONST;");
testSame("var a = { set CONST(b) {throw 'invalid'} }; var c = a.CONST;");
Node n = getLastCompiler().getRoot();

Set<Node> constantNodes = findNodesWithProperty(n, Node.IS_CONSTANT_NAME);
Expand Down
17 changes: 10 additions & 7 deletions test/com/google/javascript/jscomp/VariableReferenceCheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

package com.google.javascript.jscomp;

import static com.google.javascript.jscomp.VariableReferenceCheck.EARLY_REFERENCE;
import static com.google.javascript.jscomp.VariableReferenceCheck.REDECLARED_VARIABLE;
import static com.google.javascript.jscomp.VariableReferenceCheck.UNUSED_LOCAL_ASSIGNMENT;

/**
* Test that warnings are generated in appropriate cases and appropriate
* cases only by VariableReferenceCheck
Expand Down Expand Up @@ -163,8 +167,7 @@ public void testDestructuringInFor() {
testSameEs6("for (let [key, [nestKey, nestVal], val] of X){}");

testSameEs6("var {x: a, y: b} = {x: 1, y: 2}; a++; b++;");
testWarningEs6("a++; var {x: a} = {x: 1};",
VariableReferenceCheck.EARLY_REFERENCE);
testWarningEs6("a++; var {x: a} = {x: 1};", EARLY_REFERENCE);
}

public void testNoWarnInExterns1() {
Expand Down Expand Up @@ -263,21 +266,21 @@ public void testForIn() {
* Expects the JS to generate one bad-read error.
*/
private void assertRedeclare(String js) {
testWarning(js, VariableReferenceCheck.REDECLARED_VARIABLE);
testWarning(js, REDECLARED_VARIABLE);
}

/**
* Expects the JS to generate one bad-write warning.
*/
private void assertUndeclared(String js) {
testWarning(js, VariableReferenceCheck.EARLY_REFERENCE);
testWarning(js, EARLY_REFERENCE);
}

/**
* Expects the JS to generate one bad-write warning.
*/
private void assertUndeclaredEs6(String js) {
testWarningEs6(js, VariableReferenceCheck.EARLY_REFERENCE);
testWarningEs6(js, EARLY_REFERENCE);
}

/**
Expand All @@ -291,14 +294,14 @@ private void assertAmbiguousEs6(String js) {
* Expects the JS to generate one unused local error.
*/
private void assertUnused(String js) {
testWarning(js, VariableReferenceCheck.UNUSED_LOCAL_ASSIGNMENT);
testWarning(js, UNUSED_LOCAL_ASSIGNMENT);
}

/**
* Expects the JS to generate one unused local error.
*/
private void assertUnusedEs6(String js) {
testWarningEs6(js, VariableReferenceCheck.UNUSED_LOCAL_ASSIGNMENT);
testWarningEs6(js, UNUSED_LOCAL_ASSIGNMENT);
}

/**
Expand Down

0 comments on commit 0006772

Please sign in to comment.