Skip to content

Commit

Permalink
Move a static method from NameBasedDefinitionProvider to DefinitionsR…
Browse files Browse the repository at this point in the history 
…emover. NameBasedDefinitionsProvider is on the path for removal. While we are here remove a couple of unused public methods.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=229213308
  • Loading branch information
@concavelenz @EatingW
concavelenz authored and EatingW committed Jan 15, 2019
1 parent 3a13b14 commit 5fe8fa7
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 60 deletions.
40 changes: 33 additions & 7 deletions src/com/google/javascript/jscomp/DefinitionsRemover.java
View file
Expand Up @@ -25,6 +25,7 @@
import com.google.javascript.rhino.IR; import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token; import com.google.javascript.rhino.Token;
import javax.annotation.Nullable;


/** /**
* Models an assignment that defines a variable and the removal of it. * Models an assignment that defines a variable and the removal of it.
Expand Down Expand Up @@ -157,6 +158,31 @@ public void remove(AbstractCompiler compiler) {
*/ */
protected abstract void performRemove(AbstractCompiler compiler); protected abstract void performRemove(AbstractCompiler compiler);


/**
* Extract a name from a node. In the case of GETPROP nodes, replace the namespace or object
* expression with "this" for simplicity and correctness at the expense of inefficiencies due to
* higher chances of name collisions.
*
* <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}.
*/
@Nullable
public static String getSimplifiedName(Node node) {
if (node.isName()) {
String name = node.getString();
if (name != null && !name.isEmpty()) {
return name;
} else {
return null;
}
} else if (node.isGetProp()) {
return "this." + node.getLastChild().getString();
} else if (node.isMemberFunctionDef()) {
return "this." + node.getString();
}
return null;
}

public String getSimplifiedName() { public String getSimplifiedName() {
return simplifiedName; return simplifiedName;
} }
Expand Down Expand Up @@ -201,7 +227,7 @@ abstract static class IncompleteDefinition extends Definition {
private final Node lValue; private final Node lValue;


IncompleteDefinition(Node lValue, boolean inExterns) { IncompleteDefinition(Node lValue, boolean inExterns) {
super(inExterns, NameBasedDefinitionProvider.getSimplifiedName(lValue)); super(inExterns, Definition.getSimplifiedName(lValue));
checkNotNull(lValue); checkNotNull(lValue);


Preconditions.checkArgument( Preconditions.checkArgument(
Expand Down Expand Up @@ -280,7 +306,7 @@ abstract static class FunctionDefinition extends Definition {
protected final Node function; protected final Node function;


FunctionDefinition(Node node, boolean inExterns) { FunctionDefinition(Node node, boolean inExterns) {
this(node, inExterns, NameBasedDefinitionProvider.getSimplifiedName(node.getFirstChild())); this(node, inExterns, Definition.getSimplifiedName(node.getFirstChild()));
} }


FunctionDefinition(Node node, boolean inExterns, String name) { FunctionDefinition(Node node, boolean inExterns, String name) {
Expand Down Expand Up @@ -344,7 +370,7 @@ static final class MemberFunctionDefinition extends FunctionDefinition {
protected final Node memberFunctionDef; protected final Node memberFunctionDef;


MemberFunctionDefinition(Node node, boolean inExterns) { MemberFunctionDefinition(Node node, boolean inExterns) {
super(node.getFirstChild(), inExterns, NameBasedDefinitionProvider.getSimplifiedName(node)); super(node.getFirstChild(), inExterns, Definition.getSimplifiedName(node));
checkState(node.isMemberFunctionDef(), node); checkState(node.isMemberFunctionDef(), node);
memberFunctionDef = node; memberFunctionDef = node;
} }
Expand All @@ -369,7 +395,7 @@ abstract static class ClassDefinition extends Definition {
protected final Node c; protected final Node c;


ClassDefinition(Node node, boolean inExterns) { ClassDefinition(Node node, boolean inExterns) {
super(inExterns, NameBasedDefinitionProvider.getSimplifiedName(node.getFirstChild())); super(inExterns, Definition.getSimplifiedName(node.getFirstChild()));
Preconditions.checkArgument(node.isClass()); Preconditions.checkArgument(node.isClass());
c = node; c = node;
} }
Expand Down Expand Up @@ -426,7 +452,7 @@ static final class AssignmentDefinition extends Definition {
private final Node assignment; private final Node assignment;


AssignmentDefinition(Node node, boolean inExterns) { AssignmentDefinition(Node node, boolean inExterns) {
super(inExterns, NameBasedDefinitionProvider.getSimplifiedName(node.getFirstChild())); super(inExterns, Definition.getSimplifiedName(node.getFirstChild()));
checkArgument(node.isAssign()); checkArgument(node.isAssign());
assignment = node; assignment = node;
} }
Expand Down Expand Up @@ -479,7 +505,7 @@ static final class ObjectLiteralPropertyDefinition extends Definition {
private final Node value; private final Node value;


ObjectLiteralPropertyDefinition(Node name, Node value, boolean isExtern) { ObjectLiteralPropertyDefinition(Node name, Node value, boolean isExtern) {
super(isExtern, NameBasedDefinitionProvider.getSimplifiedName(getLValue(name))); super(isExtern, Definition.getSimplifiedName(getLValue(name)));


this.name = name; this.name = name;
this.value = value; this.value = value;
Expand Down Expand Up @@ -528,7 +554,7 @@ public Node getRValue() {
static final class VarDefinition extends Definition { static final class VarDefinition extends Definition {
private final Node name; private final Node name;
VarDefinition(Node node, boolean inExterns) { VarDefinition(Node node, boolean inExterns) {
super(inExterns, NameBasedDefinitionProvider.getSimplifiedName(node)); super(inExterns, Definition.getSimplifiedName(node));
checkArgument(NodeUtil.isNameDeclaration(node.getParent()) && node.isName()); checkArgument(NodeUtil.isNameDeclaration(node.getParent()) && node.isName());
Preconditions.checkArgument(inExterns || node.hasChildren(), Preconditions.checkArgument(inExterns || node.hasChildren(),
"VAR Declaration of %s must be assigned a value.", node.getString()); "VAR Declaration of %s must be assigned a value.", node.getString());
Expand Down
50 changes: 1 addition & 49 deletions src/com/google/javascript/jscomp/NameBasedDefinitionProvider.java
View file
Expand Up @@ -22,7 +22,6 @@


import com.google.common.collect.HashMultimap; import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
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;
import com.google.common.collect.Multimap; import com.google.common.collect.Multimap;
Expand All @@ -36,10 +35,8 @@
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
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. * Simple name-based definition gatherer.
Expand Down Expand Up @@ -91,20 +88,6 @@ public void process(Node externs, Node source) {
NodeTraversal.traverse(compiler, source, new DefinitionGatheringCallback(false)); NodeTraversal.traverse(compiler, source, new DefinitionGatheringCallback(false));
} }


public void rebuildScopeRoots(List<Node> changedScopeRoots, List<Node> deletedScopeRoots) {
for (Node scopeRoot : Iterables.concat(deletedScopeRoots, changedScopeRoots)) {
for (DefinitionSite definitionSite : definitionSitesByScopeNode.removeAll(scopeRoot)) {
Definition definition = definitionSite.definition;
definitionNodes.remove(definitionSite.node);
definitionsByName.remove(definition.getSimplifiedName(), definition);
definitionSitesByDefinitionSiteNode.remove(definitionSite.node);
}
}

DefinitionGatheringCallback cb = new DefinitionGatheringCallback();
NodeTraversal.traverseScopeRoots(compiler, null, changedScopeRoots, cb, cb, false);
}

/** @return Whether the node has a JSDoc that actually declares something. */ /** @return Whether the node has a JSDoc that actually declares something. */
private boolean jsdocContainsDeclarations(Node node) { private boolean jsdocContainsDeclarations(Node node) {
JSDocInfo info = node.getJSDocInfo(); JSDocInfo info = node.getJSDocInfo();
Expand Down Expand Up @@ -177,7 +160,7 @@ public Collection<Definition> getDefinitionsReferencedAt(Node useSiteNode) {
} }
} }


String name = getSimplifiedName(useSiteNode); String name = DefinitionsRemover.Definition.getSimplifiedName(useSiteNode);
if (name != null) { if (name != null) {
return definitionsByName.get(name); return definitionsByName.get(name);
} }
Expand Down Expand Up @@ -297,31 +280,6 @@ private void addDefinition(
definitionSitesByScopeNode.put(scopeNode, definitionSite); definitionSitesByScopeNode.put(scopeNode, definitionSite);
} }


/**
* Extract a name from a node. In the case of GETPROP nodes, replace the namespace or object
* expression with "this" for simplicity and correctness at the expense of inefficiencies due to
* higher chances of name collisions.
*
* <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}.
*/
@Nullable
public static String getSimplifiedName(Node node) {
if (node.isName()) {
String name = node.getString();
if (name != null && !name.isEmpty()) {
return name;
} else {
return null;
}
} else if (node.isGetProp()) {
return "this." + node.getLastChild().getString();
} else if (node.isMemberFunctionDef()) {
return "this." + node.getString();
}
return null;
}

/** /**
* Returns the collection of definition sites found during traversal. * Returns the collection of definition sites found during traversal.
* *
Expand All @@ -331,10 +289,4 @@ 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();
} }

public DefinitionSite getDefinitionForFunction(Node function) {
checkState(hasProcessBeenRun, "Hasn't been initialized with process() yet.");
checkState(function.isFunction());
return definitionSitesByDefinitionSiteNode.get(NodeUtil.getNameNode(function));
}
} }
6 changes: 3 additions & 3 deletions src/com/google/javascript/jscomp/PureFunctionIdentifier.java
View file
Expand Up @@ -156,7 +156,7 @@ String getDebugReport() {
Iterable<Node> expanded = unwrapCallableExpression(call.getFirstChild()); Iterable<Node> expanded = unwrapCallableExpression(call.getFirstChild());
if (expanded != null) { if (expanded != null) {
for (Node comp : expanded) { for (Node comp : expanded) {
String name = NameBasedDefinitionProvider.getSimplifiedName(comp); String name = DefinitionsRemover.Definition.getSimplifiedName(comp);
sb.append(name).append("|"); sb.append(name).append("|");
} }
} else { } else {
Expand Down Expand Up @@ -270,7 +270,7 @@ private List<FunctionInformation> getSideEffectsForCall(Node call) {
continue; continue;
} }


String name = NameBasedDefinitionProvider.getSimplifiedName(expression); String name = DefinitionsRemover.Definition.getSimplifiedName(expression);
FunctionInformation info = null; FunctionInformation info = null;
if (name != null) { if (name != null) {
info = functionInfoByName.get(name); info = functionInfoByName.get(name);
Expand Down Expand Up @@ -307,7 +307,7 @@ private void buildGraph() {
if (definition.getLValue() != null) { if (definition.getLValue() != null) {
Node getOrName = definition.getLValue(); Node getOrName = definition.getLValue();
checkArgument(getOrName.isGetProp() || getOrName.isName(), getOrName); checkArgument(getOrName.isGetProp() || getOrName.isName(), getOrName);
String name = NameBasedDefinitionProvider.getSimplifiedName(getOrName); String name = DefinitionsRemover.Definition.getSimplifiedName(getOrName);
checkNotNull(name); checkNotNull(name);
if (isSupportedFunctionDefinition(definition.getRValue())) { if (isSupportedFunctionDefinition(definition.getRValue())) {
addSupportedDefinition(site, name); addSupportedDefinition(site, name);
Expand Down
2 changes: 1 addition & 1 deletion test/com/google/javascript/jscomp/NameBasedDefinitionProviderTest.java
View file
Expand Up @@ -56,7 +56,7 @@ private static Node parse(String js) {


@Nullable @Nullable
private String getSimplifiedName(Node n) { private String getSimplifiedName(Node n) {
return NameBasedDefinitionProvider.getSimplifiedName(n); return DefinitionsRemover.Definition.getSimplifiedName(n);
} }


@Nullable @Nullable
Expand Down

0 comments on commit 5fe8fa7

Please sign in to comment.