Skip to content

Commit

Permalink
Consider a ++ or -- expression as a read, if the result is used.
Browse files Browse the repository at this point in the history
Removes some false positives for the "unused local variable" warning.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=106978097
  • Loading branch information
tbreisacher authored and blickly committed Nov 4, 2015
1 parent 5938b81 commit adcff66
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 6 deletions.
Expand Up @@ -530,8 +530,8 @@ boolean isAssignedOnceInLifetime() {
} }


/** /**
* @return The one and only assignment. Returns if there are 0 or 2+ * @return The one and only assignment. Returns null if the number of assignments is not
* assignments. * exactly one.
*/ */
private Reference getOneAndOnlyAssignment() { private Reference getOneAndOnlyAssignment() {
Reference assignment = null; Reference assignment = null;
Expand Down Expand Up @@ -715,9 +715,7 @@ boolean isInitializingDeclaration() {
* return the assigned value, otherwise null. * return the assigned value, otherwise null.
*/ */
Node getAssignedValue() { Node getAssignedValue() {
Node parent = getParent(); return NodeUtil.getRValueOfLValue(nameNode);
return (parent.isFunction())
? parent : NodeUtil.getAssignedValue(nameNode);
} }


BasicBlock getBasicBlock() { BasicBlock getBasicBlock() {
Expand Down Expand Up @@ -747,6 +745,11 @@ boolean isSimpleAssignmentToName() {
&& parent.getFirstChild() == nameNode; && parent.getFirstChild() == nameNode;
} }


/**
* Returns whether the name node for this reference is an lvalue.
* TODO(tbreisacher): This method disagrees with NodeUtil#isLValue for
* "var x;" and "let x;". Consider updating it to match.
*/
boolean isLvalue() { boolean isLvalue() {
Node parent = getParent(); Node parent = getParent();
int parentType = parent.getType(); int parentType = parent.getType();
Expand Down
4 changes: 4 additions & 0 deletions src/com/google/javascript/jscomp/VariableReferenceCheck.java
Expand Up @@ -276,6 +276,10 @@ private void checkVar(Var v, List<Reference> references) {
&& !lhsOfForInLoop) { && !lhsOfForInLoop) {
unusedAssignment = reference; unusedAssignment = reference;
} }
if ((reference.getParent().isDec() || reference.getParent().isInc())
&& NodeUtil.isExpressionResultUsed(reference.getNode())) {
isRead = true;
}
} else { } else {
isRead = true; isRead = true;
} }
Expand Down
Expand Up @@ -204,7 +204,7 @@ public void testUnusedLocalLet() {
assertUnusedEs6("function f() { let a; a = 2; }"); assertUnusedEs6("function f() { let a; a = 2; }");
} }


public void xtestUnusedLocalConst() { public void testUnusedLocalConst() {
enableUnusedLocalAssignmentCheck = true; enableUnusedLocalAssignmentCheck = true;
assertUnusedEs6("function f() { const a = 2; }"); assertUnusedEs6("function f() { const a = 2; }");
} }
Expand All @@ -224,6 +224,14 @@ public void testUnusedAssignedInInnerFunction() {
assertUnused("function f() { var x = 1; function g() { x = 2; } }"); assertUnused("function f() { var x = 1; function g() { x = 2; } }");
} }


public void testIncrementDecrementResultUsed() {
enableUnusedLocalAssignmentCheck = true;
assertNoWarning("function f() { var x = 5; while (x-- > 0) {} }");
assertNoWarning("function f() { var x = -5; while (x++ < 0) {} }");
assertNoWarning("function f() { var x = 5; while (--x > 0) {} }");
assertNoWarning("function f() { var x = -5; while (++x < 0) {} }");
}

public void testUsedInInnerFunction() { public void testUsedInInnerFunction() {
enableUnusedLocalAssignmentCheck = true; enableUnusedLocalAssignmentCheck = true;
assertNoWarning("function f() { var x = 1; function g() { use(x); } }"); assertNoWarning("function f() { var x = 1; function g() { use(x); } }");
Expand Down

0 comments on commit adcff66

Please sign in to comment.