Skip to content

Commit

Permalink
Get rid of the getVars method and switch callers to getVarIterable. U…
Browse files Browse the repository at this point in the history
…sing an Iterable instead of an Iterator generally leads to shorter and less awkward code.

This also lets us get rid of an @Suppress annotation \o/
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=123805778
  • Loading branch information
tbreisacher authored and blickly committed Jun 1, 2016
1 parent 95d441f commit f257eb0
Show file tree
Hide file tree
Showing 18 changed files with 38 additions and 96 deletions.
15 changes: 4 additions & 11 deletions src/com/google/javascript/jscomp/CoalesceVariableNames.java
Expand Up @@ -35,7 +35,6 @@

import java.util.Comparator;
import java.util.Deque;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.Set;
import java.util.TreeSet;
Expand Down Expand Up @@ -174,9 +173,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
// make this fast.
String pseudoName = null;
Set<String> allMergedNames = new TreeSet<>();
for (Iterator<Var> i = t.getScope().getVars(); i.hasNext();) {
Var iVar = i.next();

for (Var iVar : t.getScope().getVarIterable()) {
// Look for all the variables that can be merged (in the graph by now)
// and it is merged with the current coalescedVar.
if (colorings.peek().getGraph().getNode(iVar) != null
Expand Down Expand Up @@ -212,8 +209,7 @@ private UndiGraph<Var, Void> computeVariableNamesInterferenceGraph(
Scope scope = t.getScope();

// First create a node for each non-escaped variable.
for (Iterator<Var> i = scope.getVars(); i.hasNext();) {
Var v = i.next();
for (Var v : scope.getVarIterable()) {
if (!escaped.contains(v)) {

// TODO(user): In theory, we CAN coalesce function names just like
Expand All @@ -229,13 +225,10 @@ private UndiGraph<Var, Void> computeVariableNamesInterferenceGraph(
}

// Go through each variable and try to connect them.
for (Iterator<Var> i1 = scope.getVars(); i1.hasNext();) {
Var v1 = i1.next();
for (Var v1 : scope.getVarIterable()) {

NEXT_VAR_PAIR:
for (Iterator<Var> i2 = scope.getVars(); i2.hasNext();) {
Var v2 = i2.next();

for (Var v2 : scope.getVarIterable()) {
// Skip duplicate pairs.
if (v1.index >= v2.index) {
continue;
Expand Down
4 changes: 1 addition & 3 deletions src/com/google/javascript/jscomp/DataFlowAnalysis.java
Expand Up @@ -28,7 +28,6 @@

import java.util.ArrayList;
import java.util.Comparator;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -577,8 +576,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {

// 1: Remove the exception name in CATCH which technically isn't local to
// begin with.
for (Iterator<Var> i = jsScope.getVars(); i.hasNext();) {
Var var = i.next();
for (Var var : jsScope.getVarIterable()) {
if (var.getParentNode().isCatch() ||
compiler.getCodingConvention().isExported(var.getName())) {
escaped.add(var);
Expand Down
Expand Up @@ -21,7 +21,6 @@

import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -71,8 +70,7 @@ public final boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
Scope fScope = creator.createScope(n, t.getScope());
Scope fBlockScope = creator.createScope(block, fScope);
Map<String, String> currFuncRenameMap = new HashMap<>();
for (Iterator<Var> it = fBlockScope.getVars(); it.hasNext();) {
Var var = it.next();
for (Var var : fBlockScope.getVarIterable()) {
String oldName = var.getName();
if (collector.currFuncReferences.contains(oldName)
&& !currFuncRenameMap.containsKey(oldName)) {
Expand Down
5 changes: 1 addition & 4 deletions src/com/google/javascript/jscomp/InlineObjectLiterals.java
Expand Up @@ -30,7 +30,6 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -83,9 +82,7 @@ private class InliningBehavior implements Behavior {

@Override
public void afterExitScope(NodeTraversal t, ReferenceMap referenceMap) {
for (Iterator<Var> it = t.getScope().getVars(); it.hasNext();) {
Var v = it.next();

for (Var v : t.getScope().getVarIterable()) {
if (isVarInlineForbidden(v)) {
continue;
}
Expand Down
7 changes: 2 additions & 5 deletions src/com/google/javascript/jscomp/InlineVariables.java
Expand Up @@ -166,8 +166,7 @@ public void afterExitScope(NodeTraversal t, ReferenceMap referenceMap) {
private void collectAliasCandidates(NodeTraversal t,
ReferenceMap referenceMap) {
if (mode != Mode.CONSTANTS_ONLY) {
for (Iterator<Var> it = t.getScope().getVars(); it.hasNext();) {
Var v = it.next();
for (Var v : t.getScope().getVarIterable()) {
ReferenceCollection referenceInfo = referenceMap.getReferences(v);

// NOTE(nicksantos): Don't handle variables that are never used.
Expand Down Expand Up @@ -195,9 +194,7 @@ private void doInlinesForScope(NodeTraversal t, ReferenceMap referenceMap) {

boolean maybeModifiedArguments =
maybeEscapedOrModifiedArguments(t.getScope(), referenceMap);
for (Iterator<Var> it = t.getScope().getVars(); it.hasNext();) {
Var v = it.next();

for (Var v : t.getScope().getVarIterable()) {
ReferenceCollection referenceInfo = referenceMap.getReferences(v);

// referenceInfo will be null if we're in constants-only mode
Expand Down
4 changes: 1 addition & 3 deletions src/com/google/javascript/jscomp/LinkedFlowScope.java
Expand Up @@ -26,7 +26,6 @@
import com.google.javascript.rhino.jstype.StaticTypedScope;
import com.google.javascript.rhino.jstype.StaticTypedSlot;

import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
Expand Down Expand Up @@ -219,8 +218,7 @@ public StaticTypedSlot<JSType> findUniqueRefinedSlot(FlowScope blindScope) {
@Override
public void completeScope(StaticTypedScope<JSType> staticScope) {
TypedScope scope = (TypedScope) staticScope;
for (Iterator<TypedVar> it = scope.getVars(); it.hasNext();) {
TypedVar var = it.next();
for (TypedVar var : scope.getVarIterable()) {
if (var.isTypeInferred()) {
JSType type = var.getType();
if (type == null || type.isUnknownType()) {
Expand Down
Expand Up @@ -29,7 +29,6 @@
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -329,8 +328,7 @@ public void exitScope(NodeTraversal t) {
return;
}

for (Iterator<Var> it = t.getScope().getVars(); it.hasNext();) {
Var v = it.next();
for (Var v : t.getScope().getVarIterable()) {
handleScopeVar(v);
}

Expand Down
11 changes: 4 additions & 7 deletions src/com/google/javascript/jscomp/MustBeReachingVariableDef.java
Expand Up @@ -26,7 +26,6 @@

import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
Expand Down Expand Up @@ -126,10 +125,9 @@ public MustDef() {
reachingDef = new HashMap<>();
}

public MustDef(Iterator<Var> vars) {
public MustDef(Iterable<? extends Var> vars) {
this();
while (vars.hasNext()) {
Var var = vars.next();
for (Var var : vars) {
// Every variable in the scope is defined once in the beginning of the
// function: all the declared variables are undefined, all functions
// have been assigned and all arguments has its value from the caller.
Expand Down Expand Up @@ -209,7 +207,7 @@ boolean isForward() {

@Override
MustDef createEntryLattice() {
return new MustDef(jsScope.getVars());
return new MustDef(jsScope.getVarIterable());
}

@Override
Expand Down Expand Up @@ -370,8 +368,7 @@ private void addToDefIfLocal(String name, @Nullable Node node,
}

private void escapeParameters(MustDef output) {
for (Iterator<Var> i = jsScope.getVars(); i.hasNext();) {
Var v = i.next();
for (Var v : jsScope.getVarIterable()) {
if (isParameter(v)) {
// Assume we no longer know where the parameter comes from
// anymore.
Expand Down
5 changes: 1 addition & 4 deletions src/com/google/javascript/jscomp/PureFunctionIdentifier.java
Expand Up @@ -43,7 +43,6 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -533,9 +532,7 @@ public void exitScope(NodeTraversal t) {
return;
}

for (Iterator<Var> i = t.getScope().getVars(); i.hasNext();) {
Var v = i.next();

for (Var v : t.getScope().getVarIterable()) {
boolean param = v.getParentNode().isParamList();
if (param &&
!sideEffectInfo.blacklisted().contains(v) &&
Expand Down
4 changes: 1 addition & 3 deletions src/com/google/javascript/jscomp/RemoveUnusedVars.java
Expand Up @@ -29,7 +29,6 @@
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -367,8 +366,7 @@ private void traverseFunction(Node n, Scope parentScope) {
* for yet, add it to the list of variables to check later.
*/
private void collectMaybeUnreferencedVars(Scope scope) {
for (Iterator<Var> it = scope.getVars(); it.hasNext(); ) {
Var var = it.next();
for (Var var : scope.getVarIterable()) {
if (isRemovableVar(var)) {
maybeUnreferenced.add(var);
}
Expand Down
5 changes: 1 addition & 4 deletions src/com/google/javascript/jscomp/RenameVars.java
Expand Up @@ -29,7 +29,6 @@
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -216,9 +215,7 @@ public void enterScope(NodeTraversal t) {
return;
}
Scope scope = t.getScope();
Iterator<Var> it = scope.getVars();
while (it.hasNext()) {
Var current = it.next();
for (Var current : scope.getVarIterable()) {
if (current.isBleedingFunction()) {
localBleedingFunctions.add(current);
localBleedingFunctionsPerScope.put(
Expand Down
13 changes: 1 addition & 12 deletions src/com/google/javascript/jscomp/Scope.java
Expand Up @@ -21,7 +21,6 @@
import com.google.javascript.rhino.StaticScope;

import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;

Expand Down Expand Up @@ -190,21 +189,11 @@ public boolean isDeclared(String name, boolean recurse) {
}
}

/**
* Return an iterator over all of the variables declared in this scope.
*/
@SuppressWarnings("unchecked")
// Untyped scopes always only contain untyped vars; getVars is polymorphic
// so that TypedScope#getVars can return Iterator<TypedVar>.
public <T extends Var> Iterator<T> getVars() {
return (Iterator<T>) vars.values().iterator();
}

/**
* Return an iterable over all of the variables declared in this scope
* (except the special 'arguments' variable).
*/
Iterable<Var> getVarIterable() {
public Iterable<? extends Var> getVarIterable() {
return vars.values();
}

Expand Down
5 changes: 1 addition & 4 deletions src/com/google/javascript/jscomp/ShadowVariables.java
Expand Up @@ -24,7 +24,6 @@
import com.google.javascript.rhino.Node;

import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.SortedSet;
Expand Down Expand Up @@ -182,9 +181,7 @@ public void enterScope(NodeTraversal t) {
return;
}

for (Iterator<Var> vars = s.getVars(); vars.hasNext();) {
Var var = vars.next();

for (Var var : s.getVarIterable()) {
// Don't shadow variables that is bleed-out to fix an IE bug.
if (var.isBleedingFunction()) {
continue;
Expand Down
5 changes: 1 addition & 4 deletions src/com/google/javascript/jscomp/TypeInference.java
Expand Up @@ -106,10 +106,7 @@ class TypeInference

// For each local variable declared with the VAR keyword, the entry
// type is VOID.
Iterator<TypedVar> varIt =
functionScope.getDeclarativelyUnboundVarsWithoutTypes();
while (varIt.hasNext()) {
TypedVar var = varIt.next();
for (TypedVar var : functionScope.getDeclarativelyUnboundVarsWithoutTypes()) {
if (isUnflowable(var)) {
continue;
}
Expand Down
20 changes: 10 additions & 10 deletions src/com/google/javascript/jscomp/TypedScope.java
Expand Up @@ -18,14 +18,13 @@

import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterators;
import com.google.common.collect.Iterables;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.ObjectType;
import com.google.javascript.rhino.jstype.StaticTypedScope;

import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;

Expand Down Expand Up @@ -144,6 +143,7 @@ public JSType getTypeOfThis() {
}
}

@Override
Var declare(String name, Node nameNode, CompilerInput input) {
throw new IllegalStateException(
"Method declare(untyped) cannot be called on typed scopes.");
Expand Down Expand Up @@ -193,6 +193,7 @@ public TypedVar getVar(String name) {
return null;
}

@Override
public Var getArgumentsVar() {
throw new IllegalStateException("Method getArgumentsVar cannot be called on typed scopes.");
}
Expand All @@ -213,12 +214,8 @@ public boolean isDeclared(String name, boolean recurse) {
}

@Override
public Iterator<TypedVar> getVars() {
return vars.values().iterator();
}

Iterable<Var> getVarIterable() {
throw new IllegalStateException("Method getVarIterable cannot be called on typed scopes.");
public Iterable<TypedVar> getVarIterable() {
return vars.values();
}

@Override
Expand All @@ -241,8 +238,8 @@ public boolean isLocal() {
return parent != null;
}

public Iterator<TypedVar> getDeclarativelyUnboundVarsWithoutTypes() {
return Iterators.filter(getVars(), DECLARATIVELY_UNBOUND_VARS_WITHOUT_TYPES);
public Iterable<TypedVar> getDeclarativelyUnboundVarsWithoutTypes() {
return Iterables.filter(getVarIterable(), DECLARATIVELY_UNBOUND_VARS_WITHOUT_TYPES);
}

static interface TypeResolver {
Expand All @@ -263,16 +260,19 @@ void setTypeResolver(TypeResolver resolver) {
this.typeResolver = resolver;
}

@Override
public boolean isBlockScope() {
// TypedScope is not ES6 compatible yet, so always return false for now.
return false;
}

@Override
public boolean isFunctionBlockScope() {
throw new IllegalStateException(
"Method isFunctionBlockScope cannot be called on typed scopes.");
}

@Override
public Scope getClosestHoistScope() {
throw new IllegalStateException(
"Method getClosestHoistScope cannot be called on typed scopes.");
Expand Down

0 comments on commit f257eb0

Please sign in to comment.