Skip to content

Commit

Permalink
A couple of refactorings:
Browse files Browse the repository at this point in the history
* Remove interface that only has one implementation
* Change a method that returned a Collection or null, to just return an empty Collection instead of null.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=176561722
  • Loading branch information
tbreisacher authored and lauraharker committed Nov 22, 2017
1 parent f5fb684 commit 664ebb6
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 60 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/AbstractCompiler.java
Expand Up @@ -245,7 +245,7 @@ static enum MostRecentTypechecker {
* modules. * modules.
* @return A SCRIPT node (never null). * @return A SCRIPT node (never null).
*/ */
abstract Node getNodeForCodeInsertion(JSModule module); abstract Node getNodeForCodeInsertion(@Nullable JSModule module);


/** /**
* Only used by passes in the old type checker. * Only used by passes in the old type checker.
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/Compiler.java
Expand Up @@ -2986,7 +2986,7 @@ public Region getSourceRegion(String sourceName, int lineNumber) {
//------------------------------------------------------------------------ //------------------------------------------------------------------------


@Override @Override
Node getNodeForCodeInsertion(JSModule module) { Node getNodeForCodeInsertion(@Nullable JSModule module) {
if (module == null) { if (module == null) {
if (inputs.isEmpty()) { if (inputs.isEmpty()) {
throw new IllegalStateException("No inputs"); throw new IllegalStateException("No inputs");
Expand Down
40 changes: 0 additions & 40 deletions src/com/google/javascript/jscomp/DefinitionProvider.java

This file was deleted.

5 changes: 3 additions & 2 deletions src/com/google/javascript/jscomp/DefinitionUseSiteFinder.java
Expand Up @@ -21,6 +21,7 @@


import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.HashMultimap; import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap; import com.google.common.collect.Multimap;
Expand Down Expand Up @@ -57,7 +58,7 @@ private static class NameAndUseSite {
@VisibleForTesting @VisibleForTesting
Multimap<String, UseSite> getUseSitesByName() { Multimap<String, UseSite> getUseSitesByName() {
// Defensive copy. // Defensive copy.
return LinkedHashMultimap.create(useSitesByName); return ImmutableMultimap.copyOf(useSitesByName);
} }


public DefinitionUseSiteFinder(AbstractCompiler compiler) { public DefinitionUseSiteFinder(AbstractCompiler compiler) {
Expand Down Expand Up @@ -92,7 +93,7 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
} }


Collection<Definition> defs = getDefinitionsReferencedAt(node); Collection<Definition> defs = getDefinitionsReferencedAt(node);
if (defs == null) { if (defs.isEmpty()) {
return; return;
} }


Expand Down
26 changes: 15 additions & 11 deletions src/com/google/javascript/jscomp/NameBasedDefinitionProvider.java
Expand Up @@ -21,6 +21,7 @@
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;


import com.google.common.collect.HashMultimap; import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
Expand All @@ -38,9 +39,10 @@
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import javax.annotation.Nullable;


/** /**
* Simple name-based definition gatherer that implements {@link DefinitionProvider}. * Simple name-based definition gatherer.
* *
* <p>It treats all variable writes as happening in the global scope and treats all objects as * <p>It treats all variable writes as happening in the global scope and treats all objects as
* capable of having the same set of properties. The current implementation only handles definitions * capable of having the same set of properties. The current implementation only handles definitions
Expand All @@ -52,7 +54,7 @@
* use the type system to make this more accurate, in practice after disambiguate properties has * use the type system to make this more accurate, in practice after disambiguate properties has
* run, names are unique enough that this works well enough to accept the performance gain. * run, names are unique enough that this works well enough to accept the performance gain.
*/ */
public class NameBasedDefinitionProvider implements DefinitionProvider, CompilerPass { public class NameBasedDefinitionProvider implements CompilerPass {


protected final AbstractCompiler compiler; protected final AbstractCompiler compiler;


Expand Down Expand Up @@ -156,13 +158,16 @@ private void dropUntypedExterns() {
} }
} }


@Override /**
* Returns a collection of definitions that characterize the possible values of a variable or
* property.
*/
public Collection<Definition> getDefinitionsReferencedAt(Node useSiteNode) { public Collection<Definition> getDefinitionsReferencedAt(Node useSiteNode) {
checkState(hasProcessBeenRun, "Hasn't been initialized with process() yet."); checkState(hasProcessBeenRun, "Hasn't been initialized with process() yet.");
checkArgument(useSiteNode.isGetProp() || useSiteNode.isName()); checkArgument(useSiteNode.isGetProp() || useSiteNode.isName(), useSiteNode);


if (definitionNodes.contains(useSiteNode)) { if (definitionNodes.contains(useSiteNode)) {
return null; return ImmutableList.of();
} }


if (useSiteNode.isGetProp()) { if (useSiteNode.isGetProp()) {
Expand All @@ -174,10 +179,9 @@ public Collection<Definition> getDefinitionsReferencedAt(Node useSiteNode) {


String name = getSimplifiedName(useSiteNode); String name = getSimplifiedName(useSiteNode);
if (name != null) { if (name != null) {
Collection<Definition> definitions = definitionsByName.get(name); return definitionsByName.get(name);
return definitions.isEmpty() ? null : definitions;
} }
return null; return ImmutableList.of();
} }


private class DefinitionGatheringCallback implements Callback, ChangeScopeRootCallback { private class DefinitionGatheringCallback implements Callback, ChangeScopeRootCallback {
Expand Down Expand Up @@ -215,13 +219,13 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
@Override @Override
public void visit(NodeTraversal traversal, Node node, Node parent) { public void visit(NodeTraversal traversal, Node node, Node parent) {
if (inExterns) { if (inExterns) {
visitExterns(traversal, node, parent); visitExterns(traversal, node);
} else { } else {
visitCode(traversal, node); visitCode(traversal, node);
} }
} }


private void visitExterns(NodeTraversal traversal, Node node, Node parent) { private void visitExterns(NodeTraversal traversal, Node node) {
if (node.getJSDocInfo() != null) { if (node.getJSDocInfo() != null) {
for (Node typeRoot : node.getJSDocInfo().getTypeNodes()) { for (Node typeRoot : node.getJSDocInfo().getTypeNodes()) {
traversal.traverse(typeRoot); traversal.traverse(typeRoot);
Expand Down Expand Up @@ -301,6 +305,7 @@ private void addDefinition(
* <p>TODO(user) revisit. it would be helpful to at least use fully qualified names in the case of * <p>TODO(user) revisit. it would be helpful to at least use fully qualified names in the case of
* namespaces. Might not matter as much if this pass runs after {@link CollapseProperties}. * namespaces. Might not matter as much if this pass runs after {@link CollapseProperties}.
*/ */
@Nullable
public static String getSimplifiedName(Node node) { public static String getSimplifiedName(Node node) {
if (node.isName()) { if (node.isName()) {
String name = node.getString(); String name = node.getString();
Expand All @@ -320,7 +325,6 @@ public static String getSimplifiedName(Node node) {
* *
* @return definition site collection. * @return definition site collection.
*/ */
@Override
public Collection<DefinitionSite> getDefinitionSites() { public Collection<DefinitionSite> getDefinitionSites() {
checkState(hasProcessBeenRun, "Hasn't been initialized with process() yet."); checkState(hasProcessBeenRun, "Hasn't been initialized with process() yet.");
return definitionSitesByDefinitionSiteNode.values(); return definitionSitesByDefinitionSiteNode.values();
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/NodeUtil.java
Expand Up @@ -4885,7 +4885,7 @@ static boolean mayBeUndefined(Node n) {
/** /**
* @return Whether the provided expression is known not to evaluate to 'undefined'. * @return Whether the provided expression is known not to evaluate to 'undefined'.
* *
* Similiar to #getKnownValueType only for 'undefined'. This is useful for simplifing * Similar to #getKnownValueType only for 'undefined'. This is useful for simplifying
* default value expressions. * default value expressions.
*/ */
static boolean isDefinedValue(Node value) { static boolean isDefinedValue(Node value) {
Expand Down
9 changes: 5 additions & 4 deletions src/com/google/javascript/jscomp/PureFunctionIdentifier.java
Expand Up @@ -71,7 +71,7 @@
*/ */
class PureFunctionIdentifier implements CompilerPass { class PureFunctionIdentifier implements CompilerPass {
private final AbstractCompiler compiler; private final AbstractCompiler compiler;
private final DefinitionProvider definitionProvider; private final NameBasedDefinitionProvider definitionProvider;


/** Map of function names to side effect gathering representative nodes */ /** Map of function names to side effect gathering representative nodes */
private final Map<String, FunctionInformation> functionInfoByName = new HashMap<>(); private final Map<String, FunctionInformation> functionInfoByName = new HashMap<>();
Expand Down Expand Up @@ -105,7 +105,8 @@ class PureFunctionIdentifier implements CompilerPass {
private Node externs; private Node externs;
private Node root; private Node root;


public PureFunctionIdentifier(AbstractCompiler compiler, DefinitionProvider definitionProvider) { public PureFunctionIdentifier(
AbstractCompiler compiler, NameBasedDefinitionProvider definitionProvider) {
this.compiler = checkNotNull(compiler); this.compiler = checkNotNull(compiler);
this.definitionProvider = definitionProvider; this.definitionProvider = definitionProvider;
this.functionSideEffectMap = ArrayListMultimap.create(); this.functionSideEffectMap = ArrayListMultimap.create();
Expand Down Expand Up @@ -353,8 +354,8 @@ private void addSupportedDefinition(DefinitionSite definitionSite, String name)


/** /**
* Propagate side effect information by building a graph based on call site information stored in * Propagate side effect information by building a graph based on call site information stored in
* FunctionInformation and the DefinitionProvider and then running GraphReachability to determine * FunctionInformation and the NameBasedDefinitionProvider and then running GraphReachability to
* the set of functions that have side effects. * determine the set of functions that have side effects.
*/ */
private void propagateSideEffects() { private void propagateSideEffects() {
// Propagate side effect information to a fixed point. // Propagate side effect information to a fixed point.
Expand Down

0 comments on commit 664ebb6

Please sign in to comment.