Skip to content

Commit

Permalink
Add NodeUtil methods for permanently deleting nodes from the AST
Browse files Browse the repository at this point in the history
In addition to removing the node, these methods make sure to do the correct
reporting to the compiler about changes/removals.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=159726802
  • Loading branch information
blickly committed Jun 21, 2017
1 parent 37ac2e9 commit 75774a5
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 28 deletions.
33 changes: 11 additions & 22 deletions src/com/google/javascript/jscomp/ConvertToTypedInterface.java
Expand Up @@ -63,9 +63,7 @@ private void unhoistExternsToCode(Node externs, Node root) {
// Clear out the contents of existing scripts.
for (Node script = root.getFirstChild(); script != null; script = script.getNext()) {
if (script.hasChildren()) {
NodeUtil.markFunctionsDeleted(script, compiler);
script.removeChildren();
compiler.reportChangeToChangeScope(script);
NodeUtil.deleteChildren(script, compiler);
}
}

Expand Down Expand Up @@ -136,25 +134,20 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
&& !callee.matchesQualifiedName("goog.define")
&& !callee.matchesQualifiedName("goog.require")
&& !callee.matchesQualifiedName("goog.module")) {
n.detach();
NodeUtil.markFunctionsDeleted(n, t.getCompiler());
t.reportCodeChange();
NodeUtil.deleteNode(n, t.getCompiler());
}
return false;
case ASSIGN:
if (!expr.getFirstChild().isQualifiedName()
|| (expr.getFirstChild().isName() && !t.inGlobalScope() && !t.inModuleScope())) {
NodeUtil.removeChild(parent, n);
t.reportCodeChange(parent);
NodeUtil.markFunctionsDeleted(n, t.getCompiler());
NodeUtil.deleteNode(n, t.getCompiler());
return false;
}
return true;
case GETPROP:
return true;
default:
n.detach();
t.reportCodeChange();
NodeUtil.deleteNode(n, t.getCompiler());
return false;
}
case THROW:
Expand All @@ -164,8 +157,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
case DEBUGGER:
case EMPTY:
if (NodeUtil.isStatementParent(parent)) {
NodeUtil.removeChild(parent, n);
t.reportCodeChange();
NodeUtil.deleteNode(n, compiler);
}
return false;
case LABEL:
Expand All @@ -174,24 +166,23 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
case CASE:
case WHILE:
// First child can't have declaration. Statement itself will be removed post-order.
n.removeFirstChild();
NodeUtil.deleteNode(n.getFirstChild(), compiler);
return true;
case TRY:
case DO:
// Second child can't have declarations. Statement itself will be removed post-order.
n.getSecondChild().detach();
NodeUtil.deleteNode(n.getSecondChild(), compiler);
return true;
case FOR:
n.getSecondChild().detach();
NodeUtil.deleteNode(n.getSecondChild(), compiler);
// fall-through
case FOR_OF:
case FOR_IN:
n.getSecondChild().detach();
NodeUtil.deleteNode(n.getSecondChild(), compiler);
Node initializer = n.removeFirstChild();
if (initializer.isVar()) {
n.getLastChild().addChildToFront(initializer);
}
t.reportCodeChange(parent);
return true;
case CONST:
case LET:
Expand Down Expand Up @@ -409,8 +400,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
} else if (callee.matchesQualifiedName("goog.require")) {
currentFile.markImportedName(expr.getLastChild().getString());
} else if (callee.matchesQualifiedName("goog.define")) {
expr.getLastChild().detach();
t.reportCodeChange();
NodeUtil.deleteNode(expr.getLastChild(), compiler);
} else {
checkState(callee.matchesQualifiedName("goog.module"));
}
Expand Down Expand Up @@ -508,8 +498,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
});
final Node functionBody = function.getLastChild();
checkState(functionBody.isNormalBlock());
functionBody.removeChildren();
compiler.reportChangeToEnclosingScope(functionBody);
NodeUtil.deleteChildren(functionBody, compiler);
}

enum RemovalType {
Expand Down
20 changes: 20 additions & 0 deletions src/com/google/javascript/jscomp/NodeUtil.java
Expand Up @@ -2650,6 +2650,26 @@ static boolean isInSyntheticScript(Node n) {
return n.getSourceFileName() != null && n.getSourceFileName().startsWith(" [synthetic:");
}

/**
* Permanently delete the given node from the AST, as well as report
* the related AST changes/deletions to the given compiler.
*/
public static void deleteNode(Node n, AbstractCompiler compiler) {
Node parent = n.getParent();
NodeUtil.markFunctionsDeleted(n, compiler);
n.detach();
compiler.reportChangeToEnclosingScope(parent);
}

/**
* Permanently delete all the children of the given node, including reporting changes.
*/
public static void deleteChildren(Node n, AbstractCompiler compiler) {
while (n.hasChildren()) {
deleteNode(n.getFirstChild(), compiler);
}
}

/**
* Safely remove children while maintaining a valid node structure.
* In some cases, this is done by removing the parent from the AST as well.
Expand Down
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/PeepholeFoldConstants.java
Expand Up @@ -597,7 +597,7 @@ private Node tryFoldAndOr(Node n, Node left, Node right) {
// or: false_with_sideeffects && foo() => false_with_sideeffects, foo()
// This, combined with PeepholeRemoveDeadCode, helps reduce expressions
// like "x() || false || z()".
n.detachChildren();
NodeUtil.deleteChildren(n, compiler);
result = IR.comma(left, right);
}
}
Expand All @@ -607,7 +607,7 @@ private Node tryFoldAndOr(Node n, Node left, Node right) {

if (result != null) {
// Fold it!
n.detachChildren();
NodeUtil.deleteChildren(n, compiler);
parent.replaceChild(n, result);
reportCodeChange();

Expand Down
5 changes: 1 addition & 4 deletions src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java
Expand Up @@ -864,10 +864,7 @@ private Node tryFoldHook(Node n) {
boolean condHasSideEffects = mayHaveSideEffects(cond);
// Must detach after checking for side effects, to ensure that the parents
// of nodes are set correctly.
for (Node child = n.getFirstChild(); child != null; child = child.getNext()) {
NodeUtil.markFunctionsDeleted(child, compiler);
}
n.detachChildren();
NodeUtil.deleteChildren(n, compiler);

if (condHasSideEffects) {
replacement = IR.comma(cond, branchToKeep).srcref(n);
Expand Down

0 comments on commit 75774a5

Please sign in to comment.