Skip to content

Commit

Permalink
Split VariableReferenceCheck.checkVar into several methods
Browse files Browse the repository at this point in the history 
Separate the logic for checking declarations and references. We might be able
to clean up this class a bit more but this should be a good first step.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=148524771
  • Loading branch information
@Dominator008
Dominator008 authored and Tyler Breisacher committed Feb 27, 2017
1 parent 4c6018b commit 29121a3
Showing 1 changed file with 131 additions and 119 deletions.
250 changes: 131 additions & 119 deletions src/com/google/javascript/jscomp/VariableReferenceCheck.java
View file
Expand Up @@ -212,97 +212,42 @@ private void checkShadowParam(Var v, Scope functionScope, List<Reference> refere
*/ */
private void checkVar(Var v, List<Reference> references) { private void checkVar(Var v, List<Reference> references) {
blocksWithDeclarations.clear(); blocksWithDeclarations.clear();
boolean isDeclaredInScope = false; boolean hasSeenDeclaration = false;
boolean hasErrors = false; boolean hasErrors = false;
boolean isRead = false; boolean isRead = false;
Reference hoistedFn = null;
Reference unusedAssignment = null; Reference unusedAssignment = null;


// Look for hoisted functions. Reference hoistedFn = lookForHoistedFunction(references);
for (Reference reference : references) { if (hoistedFn != null) {
if (reference.isHoistedFunction()) { hasSeenDeclaration = true;
blocksWithDeclarations.add(reference.getBasicBlock());
isDeclaredInScope = true;
hoistedFn = reference;
break;
}
} }


for (Reference reference : references) { for (Reference reference : references) {
if (reference == hoistedFn) { if (reference == hoistedFn) {
continue; continue;
} }

Node referenceNode = reference.getNode();
BasicBlock basicBlock = reference.getBasicBlock(); BasicBlock basicBlock = reference.getBasicBlock();
boolean isDeclaration = reference.isDeclaration(); boolean isDeclaration = reference.isDeclaration();
Node referenceNode = reference.getNode();
boolean isAssignment = isDeclaration || reference.isLvalue(); boolean isAssignment = isDeclaration || reference.isLvalue();


boolean allowDupe = VarCheck.hasDuplicateDeclarationSuppression(referenceNode, v); if (isDeclaration) {
boolean letConstShadowsVar = // Checks for declarations
v.getParentNode().isVar() hasSeenDeclaration = true;
&& (reference.isLetDeclaration() || reference.isConstDeclaration()); hasErrors = checkRedeclaration(v, reference, referenceNode, hoistedFn, basicBlock);
boolean isVarNodeSameAsReferenceNode = v.getNode() == reference.getNode(); // Add the current basic block after checking redeclarations
// We disallow redeclaration of caught exceptions blocksWithDeclarations.add(basicBlock);
boolean shadowCatchVar = checkBlocklessDeclaration(v, reference, referenceNode);
isDeclaration } else {
&& v.getParentNode().isCatch() // Checks for references
&& !isVarNodeSameAsReferenceNode; if (!hasSeenDeclaration) {
boolean shadowParam = hasErrors = checkEarlyReference(v, reference, referenceNode);
isDeclaration
&& v.isParam()
&& NodeUtil.isBlockScopedDeclaration(referenceNode)
&& v.getScope() == reference.getScope().getParent();
boolean shadowDetected = false;
if (isDeclaration && !allowDupe) {
// Look through all the declarations we've found so far, and
// check if any of them are before this block.
for (BasicBlock declaredBlock : blocksWithDeclarations) {
if (declaredBlock.provablyExecutesBefore(basicBlock)) {
shadowDetected = true;
DiagnosticType diagnosticType;
Node warningNode = referenceNode;
if (v.isLet()
|| v.isConst()
|| v.isClass()
|| letConstShadowsVar
|| shadowCatchVar
|| shadowParam) {
// These cases are all hard errors that violate ES6 semantics
diagnosticType = REDECLARED_VARIABLE_ERROR;
} else if (reference.getNode().getParent().isCatch() || allowDupe) {
return;
} else {
// These diagnostics are for valid, but suspicious, code, and are suppressible.
// For vars defined in the global scope, give the same error as VarCheck
diagnosticType =
v.getScope().isGlobal()
? VarCheck.VAR_MULTIPLY_DECLARED_ERROR
: REDECLARED_VARIABLE;
// Since we skip hoisted functions, we would have the wrong warning node in cases
// where the redeclaration is a function declaration. Check for that case.
if (isVarNodeSameAsReferenceNode
&& hoistedFn != null
&& v.name.equals(hoistedFn.getNode().getString())) {
warningNode = hoistedFn.getNode();
}
}
compiler.report(
JSError.make(
warningNode,
diagnosticType,
v.name,
v.input != null ? v.input.getName() : "??"));
hasErrors = true;
break;
}
} }
}


if (!shadowDetected if (!hasErrors && v.isConst() && reference.isLvalue()) {
&& isDeclaration compiler.report(JSError.make(referenceNode, REASSIGNED_CONSTANT, v.name));
&& (letConstShadowsVar || shadowCatchVar) }
&& v.getScope() == reference.getScope()) {
compiler.report(JSError.make(referenceNode, REDECLARED_VARIABLE_ERROR, v.name));
} }


if (isAssignment) { if (isAssignment) {
Expand All @@ -324,63 +269,130 @@ private void checkVar(Var v, List<Reference> references) {
} else { } else {
isRead = true; isRead = true;
} }
}


boolean isUndeclaredReference = false; if (unusedAssignment != null && !isRead && !hasErrors) {
if (!isDeclaration && !isDeclaredInScope) { checkForUnusedLocalVar(v, unusedAssignment);
// Don't check the order of refer in externs files. }
if (!referenceNode.isFromExterns()) { }
// Special case to deal with var goog = goog || {}. Note that }
// let x = x || {} is illegal, just like var y = x || {}; let x = y;
if (v.isVar()) {
Node curr = reference.getParent();
while (curr.isOr() && curr.getParent().getFirstChild() == curr) {
curr = curr.getParent();
}
if (curr.isName() && curr.getString().equals(v.name)) {
continue;
}
}


// Only generate warnings if the scopes do not match in order /**
// to deal with possible forward declarations and recursion * @return The reference to the hoisted function, if the variable is one
// TODO(moz): Remove the bypass for "goog" once VariableReferenceCheck */
// is run after the Closure passes. private Reference lookForHoistedFunction(List<Reference> references) {
if (reference.getScope() == v.scope && !v.getName().equals("goog")) { for (Reference reference : references) {
isUndeclaredReference = true; if (reference.isHoistedFunction()) {
compiler.report( blocksWithDeclarations.add(reference.getBasicBlock());
JSError.make( return reference;
reference.getNode(), }
(v.isLet() || v.isConst() || v.isClass() || v.isParam()) }
? EARLY_REFERENCE_ERROR return null;
: EARLY_REFERENCE, }
v.name));
hasErrors = true; private void checkBlocklessDeclaration(Var v, Reference reference, Node referenceNode) {
if (!reference.isVarDeclaration() && reference.getGrandparent().isAddedBlock()
&& BLOCKLESS_DECLARATION_FORBIDDEN_STATEMENTS.contains(
reference.getGrandparent().getParent().getToken())) {
compiler.report(JSError.make(referenceNode, DECLARATION_NOT_DIRECTLY_IN_BLOCK, v.name));
}
}

/**
* @return If a redeclaration error has been found
*/
private boolean checkRedeclaration(
Var v, Reference reference, Node referenceNode, Reference hoistedFn, BasicBlock basicBlock) {
boolean allowDupe = VarCheck.hasDuplicateDeclarationSuppression(referenceNode, v);
boolean letConstShadowsVar = v.getParentNode().isVar()
&& (reference.isLetDeclaration() || reference.isConstDeclaration());
boolean isVarNodeSameAsReferenceNode = v.getNode() == reference.getNode();
// We disallow redeclaration of caught exceptions
boolean shadowCatchVar = v.getParentNode().isCatch() && !isVarNodeSameAsReferenceNode;
boolean shadowParam = v.isParam() && NodeUtil.isBlockScopedDeclaration(referenceNode)
&& v.getScope() == reference.getScope().getParent();
boolean shadowDetected = false;
if (!allowDupe) {
// Look through all the declarations we've found so far, and
// check if any of them are before this block.
for (BasicBlock declaredBlock : blocksWithDeclarations) {
if (declaredBlock.provablyExecutesBefore(basicBlock)) {
shadowDetected = true;
DiagnosticType diagnosticType;
Node warningNode = referenceNode;
if (v.isLet() || v.isConst() || v.isClass() || letConstShadowsVar || shadowCatchVar
|| shadowParam) {
// These cases are all hard errors that violate ES6 semantics
diagnosticType = REDECLARED_VARIABLE_ERROR;
} else if (reference.getNode().getParent().isCatch() || allowDupe) {
return false;
} else {
// These diagnostics are for valid, but suspicious, code, and are suppressible.
// For vars defined in the global scope, give the same error as VarCheck
diagnosticType =
v.getScope().isGlobal()
? VarCheck.VAR_MULTIPLY_DECLARED_ERROR
: REDECLARED_VARIABLE;
// Since we skip hoisted functions, we would have the wrong warning node in cases
// where the redeclaration is a function declaration. Check for that case.
if (isVarNodeSameAsReferenceNode
&& hoistedFn != null
&& v.name.equals(hoistedFn.getNode().getString())) {
warningNode = hoistedFn.getNode();
} }
} }
compiler.report(
JSError.make(
warningNode,
diagnosticType,
v.name,
v.input != null ? v.input.getName() : "??"));
return true;
} }
}
}


if (!isDeclaration && !isUndeclaredReference && v.isConst() && reference.isLvalue()) { if (!shadowDetected && (letConstShadowsVar || shadowCatchVar)
compiler.report(JSError.make(referenceNode, REASSIGNED_CONSTANT, v.name)); && v.getScope() == reference.getScope()) {
} compiler.report(JSError.make(referenceNode, REDECLARED_VARIABLE_ERROR, v.name));
return true;
}
return false;
}


if (isDeclaration /**
&& !reference.isVarDeclaration() * @return If an early reference has been found
&& reference.getGrandparent().isAddedBlock() */
&& BLOCKLESS_DECLARATION_FORBIDDEN_STATEMENTS.contains( private boolean checkEarlyReference(Var v, Reference reference, Node referenceNode) {
reference.getGrandparent().getParent().getToken())) { // Don't check the order of refer in externs files.
compiler.report(JSError.make(referenceNode, DECLARATION_NOT_DIRECTLY_IN_BLOCK, v.name)); if (!referenceNode.isFromExterns()) {
// Special case to deal with var goog = goog || {}. Note that
// let x = x || {} is illegal, just like var y = x || {}; let x = y;
if (v.isVar()) {
Node curr = reference.getParent();
while (curr.isOr() && curr.getParent().getFirstChild() == curr) {
curr = curr.getParent();
} }

if (curr.isName() && curr.getString().equals(v.name)) {
if (isDeclaration) { return false;
blocksWithDeclarations.add(basicBlock);
isDeclaredInScope = true;
} }
} }


if (unusedAssignment != null && !isRead && !hasErrors) { // Only generate warnings if the scopes do not match in order
checkForUnusedLocalVar(v, unusedAssignment); // to deal with possible forward declarations and recursion
// TODO(moz): See if we can remove the bypass for "goog"
if (reference.getScope() == v.scope && !v.getName().equals("goog")) {
compiler.report(
JSError.make(
reference.getNode(),
(v.isLet() || v.isConst() || v.isClass() || v.isParam())
? EARLY_REFERENCE_ERROR
: EARLY_REFERENCE,
v.name));
return true;
} }
} }
return false;
} }


// Only check for unused local if not in a goog.scope function. // Only check for unused local if not in a goog.scope function.
Expand Down

0 comments on commit 29121a3

Please sign in to comment.