From 1423f1b179369000cd6552adb970ea51a8edd309 Mon Sep 17 00:00:00 2001 From: stalcup Date: Fri, 9 Jun 2017 17:37:08 -0700 Subject: [PATCH] Adds a deleteTimeline with recording/querying/testing. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=158584751 --- .../javascript/jscomp/AbstractCompiler.java | 7 ++ .../google/javascript/jscomp/Compiler.java | 18 ++++ .../javascript/jscomp/CompilerTest.java | 95 +++++++++++++++++-- 3 files changed, 110 insertions(+), 10 deletions(-) diff --git a/src/com/google/javascript/jscomp/AbstractCompiler.java b/src/com/google/javascript/jscomp/AbstractCompiler.java index 0093eb8f2f4..5a5f4003945 100644 --- a/src/com/google/javascript/jscomp/AbstractCompiler.java +++ b/src/com/google/javascript/jscomp/AbstractCompiler.java @@ -322,6 +322,13 @@ LifeCycleStage getLifeCycleStage() { */ abstract List getChangedScopeNodesForPass(String passName); + /** + * An accumulation of deleted scope nodes since the last time the given pass was run. A returned + * null or empty list means no scope nodes have been deleted since the last run or this is the + * first time the pass has run. + */ + abstract List getDeletedScopeNodesForPass(String passName); + /** Called to indicate that the current change stamp has been used */ abstract void incrementChangeStamp(); diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index 5aefcaad9c2..ead14add4bb 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -302,6 +302,7 @@ public SourceFile loadSource(String filename) { private int changeStamp = 1; private final Timeline changeTimeline = new Timeline<>(); + private final Timeline deleteTimeline = new Timeline<>(); /** * Creates a Compiler that reports errors and warnings to its logger. @@ -2579,6 +2580,13 @@ List getChangedScopeNodesForPass(String passName) { return changedScopeNodes; } + @Override + List getDeletedScopeNodesForPass(String passName) { + List deletedScopeNodes = deleteTimeline.getSince(passName); + deleteTimeline.mark(passName); + return deletedScopeNodes; + } + @Override public void incrementChangeStamp() { changeStamp++; @@ -2610,6 +2618,15 @@ private Node getChangeScopeForNode(Node n) { } private void recordChange(Node n) { + if (n.isDeleted()) { + // Some complicated passes (like SmartNameRemoval) might both change and delete a scope in + // the same pass, and they might even perform the change after the deletion because of + // internal queueing. Just ignore the spurious attempt to mark changed after already marking + // deleted. There's no danger of deleted nodes persisting in the AST since this is enforced + // separately in ChangeVerifier. + return; + } + n.setChangeTime(changeStamp); // Every code change happens at a different time changeStamp++; @@ -2653,6 +2670,7 @@ public void reportFunctionDeleted(Node n) { checkState(n.isFunction()); n.setDeleted(true); changeTimeline.remove(n); + deleteTimeline.add(n); } @Override diff --git a/test/com/google/javascript/jscomp/CompilerTest.java b/test/com/google/javascript/jscomp/CompilerTest.java index 80327000bd4..47026c62a4c 100644 --- a/test/com/google/javascript/jscomp/CompilerTest.java +++ b/test/com/google/javascript/jscomp/CompilerTest.java @@ -1122,15 +1122,47 @@ public void testReportChangeWithScopeSucceeds() { compiler.reportChangeToEnclosingScope(attachedNode); } - /** See TimelineTest.java for the many behavior tests that don't make sense to duplicate here. */ - public void testGetChangedScopeNodesForPass_doesntIncludeDeleted() { + /** + * See TimelineTest.java for the many timeline behavior tests that don't make sense to duplicate + * here. + */ + public void testGetChangesAndDeletions_baseline() { + Compiler compiler = new Compiler(); + + // In the initial state nothing has been marked changed or deleted. + assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).isNull(); + assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).isNull(); + } + + public void testGetChangesAndDeletions_changeReportsVisible() { Compiler compiler = new Compiler(); Node function1 = IR.function(IR.name("foo"), IR.paramList(), IR.block()); Node function2 = IR.function(IR.name("foo"), IR.paramList(), IR.block()); IR.root(IR.script(function1, function2)); - // In the initial state everything is attached and nothing has been marked changed. - assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).isNull(); + // Mark original baseline. + compiler.getChangedScopeNodesForPass("FunctionInliner"); + compiler.getDeletedScopeNodesForPass("FunctionInliner"); + + // Mark both functions changed. + compiler.reportChangeToChangeScope(function1); + compiler.reportChangeToChangeScope(function2); + + // Both function1 and function2 are seen as changed and nothing is seen as deleted. + assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")) + .containsExactly(function1, function2); + assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).isEmpty(); + } + + public void testGetChangesAndDeletions_deleteOverridesChange() { + Compiler compiler = new Compiler(); + Node function1 = IR.function(IR.name("foo"), IR.paramList(), IR.block()); + Node function2 = IR.function(IR.name("foo"), IR.paramList(), IR.block()); + IR.root(IR.script(function1, function2)); + + // Mark original baseline. + compiler.getChangedScopeNodesForPass("FunctionInliner"); + compiler.getDeletedScopeNodesForPass("FunctionInliner"); // Mark both functions changed, then delete function2 and mark it deleted. compiler.reportChangeToChangeScope(function1); @@ -1138,14 +1170,57 @@ public void testGetChangedScopeNodesForPass_doesntIncludeDeleted() { function2.detach(); compiler.reportFunctionDeleted(function2); - // Only function1 will be seen as changed, since deleting something removes it from the changed - // items list. - assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")) - .containsExactly(function1); + // Now function1 will be seen as changed and function2 will be seen as deleted, since delete + // overrides change. + assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).containsExactly(function1); + assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).containsExactly(function2); + } + + public void testGetChangesAndDeletions_changeDoesntOverrideDelete() { + Compiler compiler = new Compiler(); + Node function1 = IR.function(IR.name("foo"), IR.paramList(), IR.block()); + Node function2 = IR.function(IR.name("foo"), IR.paramList(), IR.block()); + IR.root(IR.script(function1, function2)); + + // Mark original baseline. + compiler.getChangedScopeNodesForPass("FunctionInliner"); + compiler.getDeletedScopeNodesForPass("FunctionInliner"); + + // Mark function1 changed and function2 deleted, then try to mark function2 changed. + compiler.reportChangeToChangeScope(function1); + function2.detach(); + compiler.reportFunctionDeleted(function2); + compiler.reportChangeToChangeScope(function2); + + // Now function1 will be seen as changed and function2 will be seen as deleted, since change + // does not override delete. + assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).containsExactly(function1); + assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).containsExactly(function2); + } + + public void testGetChangesAndDeletions_onlySeesChangesSinceLastRequest() { + Compiler compiler = new Compiler(); + Node function1 = IR.function(IR.name("foo"), IR.paramList(), IR.block()); + Node function2 = IR.function(IR.name("foo"), IR.paramList(), IR.block()); + IR.root(IR.script(function1, function2)); + + // Mark original baseline. + compiler.getChangedScopeNodesForPass("FunctionInliner"); + compiler.getDeletedScopeNodesForPass("FunctionInliner"); + + // Mark function1 changed and function2 deleted. + compiler.reportChangeToChangeScope(function1); + function2.detach(); + compiler.reportFunctionDeleted(function2); + + // Verify their respective states are seen. + assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).containsExactly(function1); + assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).containsExactly(function2); - // Asking for changes again will now show nothing, since no changes have occurred between these - // consecutive "FunctionInliner" markings. + // Check states again. Should find nothing since nothing has changed since the last + // 'FunctionInliner' request. assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).isEmpty(); + assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).isEmpty(); } private static CompilerOptions createNewFlagBasedOptions() {