From 0797e0f1df87b3d1f715977681215ef72fcb065a Mon Sep 17 00:00:00 2001 From: johnlenz Date: Thu, 28 Mar 2019 09:52:53 -0700 Subject: [PATCH] Remove MarkNoSideEffects in favor of PureFunctionIdentifier. MarkNoSideEffects was only used for unit tests but encouraged confusing tests. This leaves PureFunctionIdentifier as the last remaining use of NameBasedDefinitionProvider. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240792582 --- .../javascript/jscomp/CompilerOptions.java | 9 - .../javascript/jscomp/DefaultPassConfig.java | 25 -- .../jscomp/MarkNoSideEffectCalls.java | 193 ------------ .../google/javascript/jscomp/PassNames.java | 1 - .../debugger/common/CompilationParam.java | 12 - .../javascript/jscomp/CompilerTestCase.java | 23 +- .../FlowSensitiveInlineVariablesTest.java | 5 +- .../jscomp/FunctionInjectorTest.java | 2 +- .../jscomp/FunctionToBlockMutatorTest.java | 2 +- .../jscomp/InlineFunctionsTest.java | 2 +- .../javascript/jscomp/IntegrationTest.java | 12 - .../jscomp/MarkNoSideEffectCallsTest.java | 291 ------------------ .../RemoveUnusedCodeNameAnalyzerTest.java | 97 +++--- 13 files changed, 56 insertions(+), 618 deletions(-) delete mode 100644 src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java delete mode 100644 test/com/google/javascript/jscomp/MarkNoSideEffectCallsTest.java diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java index a24cdc822f5..c08daab11bf 100644 --- a/src/com/google/javascript/jscomp/CompilerOptions.java +++ b/src/com/google/javascript/jscomp/CompilerOptions.java @@ -808,9 +808,6 @@ public void setReplaceMessagesWithChromeI18n( protected transient Multimap customPasses; - /** Mark no side effect calls */ - public boolean markNoSideEffectCalls; - /** Replacements for @defines. Will be Boolean, Numbers, or Strings */ private Map defineReplacements; @@ -1296,7 +1293,6 @@ public CompilerOptions() { stripNamePrefixes = ImmutableSet.of(); stripTypePrefixes = ImmutableSet.of(); customPasses = null; - markNoSideEffectCalls = false; defineReplacements = new HashMap<>(); tweakProcessing = TweakProcessing.OFF; tweakReplacements = new HashMap<>(); @@ -2475,10 +2471,6 @@ public void addCustomPass(CustomPassExecutionTime time, CompilerPass customPass) customPasses.put(time, customPass); } - public void setMarkNoSideEffectCalls(boolean markNoSideEffectCalls) { - this.markNoSideEffectCalls = markNoSideEffectCalls; - } - public void setDefineReplacements(Map defineReplacements) { this.defineReplacements = defineReplacements; } @@ -2939,7 +2931,6 @@ public String toString() { .add("lineLengthThreshold", lineLengthThreshold) .add("locale", locale) .add("markAsCompiled", markAsCompiled) - .add("markNoSideEffectCalls", markNoSideEffectCalls) .add("maxFunctionSizeAfterInlining", maxFunctionSizeAfterInlining) .add("messageBundle", messageBundle) .add("moduleRoots", moduleRoots) diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index dd28f4f4337..7a9846cf545 100644 --- a/src/com/google/javascript/jscomp/DefaultPassConfig.java +++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java @@ -651,17 +651,6 @@ protected List getOptimizations() { if (options.computeFunctionSideEffects) { passes.add(markPureFunctions); - } else if (options.markNoSideEffectCalls) { - // TODO(user) The properties that this pass adds to CALL and NEW - // AST nodes increase the AST's in-memory size. Given that we are - // already running close to our memory limits, we could run into - // trouble if we end up using the @nosideeffects annotation a lot - // or compute @nosideeffects annotations by looking at function - // bodies. It should be easy to propagate @nosideeffects - // annotations as part of passes that depend on this property and - // store the result outside the AST (which would allow garbage - // collection once the pass is done). - passes.add(markNoSideEffectCalls); } if (options.smartNameRemoval) { @@ -2555,20 +2544,6 @@ protected FeatureSet featureSet() { } }; - /** Look for function calls that have no side effects, and annotate them that way. */ - private final PassFactory markNoSideEffectCalls = - new PassFactory(PassNames.MARK_NO_SIDE_EFFECT_CALLS, true) { - @Override - protected CompilerPass create(AbstractCompiler compiler) { - return new MarkNoSideEffectCalls(compiler); - } - - @Override - protected FeatureSet featureSet() { - return ES5; - } - }; - /** Inlines variables heuristically. */ private final PassFactory inlineVariables = new PassFactory(PassNames.INLINE_VARIABLES, false) { diff --git a/src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java b/src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java deleted file mode 100644 index 64e00f381dd..00000000000 --- a/src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java +++ /dev/null @@ -1,193 +0,0 @@ -/* - * Copyright 2009 The Closure Compiler Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.javascript.jscomp; - -import static com.google.common.base.Preconditions.checkNotNull; - -import com.google.javascript.jscomp.DefinitionsRemover.Definition; -import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; -import com.google.javascript.rhino.JSDocInfo; -import com.google.javascript.rhino.Node; -import com.google.javascript.rhino.Node.SideEffectFlags; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - -/** - * Set the NoSideEffects property for function and constructor calls - * that refer to functions that are known to have no side effects. - * Current implementation relies on @nosideeffects annotations at - * function definition sites; eventually we should traverse function - * bodies to determine if they have side effects. - * - */ -class MarkNoSideEffectCalls implements CompilerPass { - private final AbstractCompiler compiler; - - // Left hand side expression associated with a function node that - // has a @nosideeffects annotation. - private final Set noSideEffectFunctionNames; - - MarkNoSideEffectCalls(AbstractCompiler compiler) { - this.compiler = compiler; - this.noSideEffectFunctionNames = new HashSet<>(); - } - - @Override - public void process(Node externs, Node root) { - NameBasedDefinitionProvider defFinder = new NameBasedDefinitionProvider(compiler, false); - defFinder.process(externs, root); - - // Gather the list of function nodes that have @nosideeffects annotations. - // For use by SetNoSideEffectCallProperty. - NodeTraversal.traverse( - compiler, externs, new GatherNoSideEffectFunctions()); - NodeTraversal.traverse( - compiler, root, new GatherNoSideEffectFunctions()); - - NodeTraversal.traverse(compiler, root, new SetNoSideEffectCallProperty(defFinder)); - } - - /** - * Determines if the type of the value of the RHS expression can - * be a function node. - */ - private static boolean definitionTypeContainsFunctionType(Definition def) { - Node rhs = def.getRValue(); - if (rhs == null) { - return true; - } - - switch (rhs.getToken()) { - case ASSIGN: - case AND: - case CALL: - case GETPROP: - case GETELEM: - case FUNCTION: - case HOOK: - case NAME: - case NEW: - case OR: - return true; - default: - return false; - } - } - - /** - * Get the value of the @nosideeffects annotation stored in the - * doc info. - */ - private static boolean hasNoSideEffectsAnnotation(Node node) { - JSDocInfo docInfo = node.getJSDocInfo(); - return docInfo != null && docInfo.isNoSideEffects(); - } - - /** - * Gather function nodes that have @nosideeffects annotations. - */ - private class GatherNoSideEffectFunctions extends AbstractPostOrderCallback { - @Override - public void visit(NodeTraversal traversal, Node node, Node parent) { - if (node.isGetProp()) { - if (parent.isExprResult() && - hasNoSideEffectsAnnotation(node)) { - noSideEffectFunctionNames.add(node); - } - } else if (node.isFunction()) { - - // The annotation may attached to the function node, the - // variable declaration or assignment expression. - boolean hasAnnotation = hasNoSideEffectsAnnotation(node); - List nameNodes = new ArrayList<>(); - nameNodes.add(node.getFirstChild()); - - if (parent.isName()) { - Node gramp = parent.getParent(); - if (gramp.isVar() && - gramp.hasOneChild() && - hasNoSideEffectsAnnotation(gramp)) { - hasAnnotation = true; - } - - nameNodes.add(parent); - } else if (parent.isAssign()) { - if (hasNoSideEffectsAnnotation(parent)) { - hasAnnotation = true; - } - - nameNodes.add(parent.getFirstChild()); - } - - if (hasAnnotation) { - noSideEffectFunctionNames.addAll(nameNodes); - } - } - } - } - - /** - * Set the no side effects property for CALL and NEW nodes that - * refer to function names that are known to have no side effects. - */ - private class SetNoSideEffectCallProperty extends AbstractPostOrderCallback { - private final NameBasedDefinitionProvider defFinder; - - SetNoSideEffectCallProperty(NameBasedDefinitionProvider defFinder) { - this.defFinder = defFinder; - } - - @Override - public void visit(NodeTraversal traversal, Node node, Node parent) { - if (!NodeUtil.isCallOrNew(node)) { - return; - } - Node nameNode = node.getFirstChild(); - // This is the result of an anonymous function execution. function() {}(); - if (!nameNode.isName() && !nameNode.isGetProp()) { - return; - } - - Collection definitions = defFinder.getDefinitionsReferencedAt(nameNode); - if (definitions == null) { - return; - } - - boolean maybeFunction = false; - for (Definition def : definitions) { - Node lValue = def.getLValue(); - checkNotNull(lValue); - if (definitionTypeContainsFunctionType(def)) { - maybeFunction = true; - if (!noSideEffectFunctionNames.contains(lValue)) { - return; - } - } - } - - if (maybeFunction) { - if (node.getSideEffectFlags() != SideEffectFlags.NO_SIDE_EFFECTS) { - node.setSideEffectFlags(SideEffectFlags.NO_SIDE_EFFECTS); - compiler.reportChangeToEnclosingScope(node); - } - } - } - } -} diff --git a/src/com/google/javascript/jscomp/PassNames.java b/src/com/google/javascript/jscomp/PassNames.java index b5bb00d3285..da98d6bfa8e 100644 --- a/src/com/google/javascript/jscomp/PassNames.java +++ b/src/com/google/javascript/jscomp/PassNames.java @@ -70,7 +70,6 @@ public final class PassNames { public static final String INLINE_TYPE_ALIASES = "inlineTypeAliases"; public static final String INLINE_VARIABLES = "inlineVariables"; public static final String LINT_CHECKS = "lintChecks"; - public static final String MARK_NO_SIDE_EFFECT_CALLS = "markNoSideEffectCalls"; public static final String MOVE_FUNCTION_DECLARATIONS = "moveFunctionDeclarations"; public static final String NAME_ANONYMOUS_FUNCTIONS = "nameAnonymousFunctions"; public static final String NORMALIZE = "normalize"; diff --git a/src/com/google/javascript/jscomp/debugger/common/CompilationParam.java b/src/com/google/javascript/jscomp/debugger/common/CompilationParam.java index 4669b770ea6..857bd6597b7 100644 --- a/src/com/google/javascript/jscomp/debugger/common/CompilationParam.java +++ b/src/com/google/javascript/jscomp/debugger/common/CompilationParam.java @@ -453,18 +453,6 @@ public boolean isApplied(CompilerOptions options) { } }, - MARK_NO_SIDE_EFFECT_CALLS(ParamGroup.OPTIMIZATION){ - @Override - public void apply(CompilerOptions options, boolean value) { - options.setMarkNoSideEffectCalls(value); - } - - @Override - public boolean isApplied(CompilerOptions options) { - return options.markNoSideEffectCalls; - } - }, - /** Converts quoted property accesses to dot syntax (a['b'] -> a.b) */ CONVERT_TO_DOTTED_PROPERTIES(ParamGroup.OPTIMIZATION){ @Override diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java index 134486d819e..8fff4eb6b5b 100644 --- a/test/com/google/javascript/jscomp/CompilerTestCase.java +++ b/test/com/google/javascript/jscomp/CompilerTestCase.java @@ -164,11 +164,6 @@ public abstract class CompilerTestCase { */ private DiagnosticType expectedSymbolTableError; - /** - * Whether the MarkNoSideEffectsCalls pass runs before the pass being tested - */ - private boolean markNoSideEffects; - /** * Whether the PureFunctionIdentifier pass runs before the pass being tested */ @@ -631,7 +626,6 @@ public void setUp() throws Exception { this.verifyNoNewGettersOrSetters = false; this.inferConsts = false; this.languageOut = LanguageMode.ECMASCRIPT5; - this.markNoSideEffects = false; this.multistageCompilation = true; this.normalizeEnabled = false; this.parseTypeInfo = false; @@ -938,20 +932,10 @@ protected final void disableNormalize() { normalizeEnabled = false; } - /** - * Run the MarkSideEffectCalls pass before running the test pass. - * - * @see MarkNoSideEffectCalls - */ - protected final void enableMarkNoSideEffects() { - checkState(this.setUpRan, "Attempted to configure before running setUp()."); - markNoSideEffects = true; - } - /** * Run the PureFunctionIdentifier pass before running the test pass. * - * @see MarkNoSideEffectCalls + * @see PureFunctionIdentifier */ protected final void enableComputeSideEffects() { checkState(this.setUpRan, "Attempted to configure before running setUp()."); @@ -1603,11 +1587,6 @@ private void testInternal( hasCodeChanged = hasCodeChanged || recentChange.hasCodeChanged(); } - if (markNoSideEffects && i == 0) { - MarkNoSideEffectCalls mark = new MarkNoSideEffectCalls(compiler); - mark.process(externsRoot, mainRoot); - } - if (gatherExternPropertiesEnabled && i == 0) { (new GatherExternProperties(compiler)).process(externsRoot, mainRoot); } diff --git a/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java b/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java index 143f6358635..deffee17dc4 100644 --- a/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java +++ b/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java @@ -50,12 +50,11 @@ protected int getNumRepetitions() { @Override protected CompilerPass getProcessor(final Compiler compiler) { - //return new FlowSensitiveInlineVariables(compiler); return new CompilerPass() { @Override public void process(Node externs, Node root) { - (new MarkNoSideEffectCalls(compiler)).process(externs, root); - (new FlowSensitiveInlineVariables(compiler)).process(externs, root); + new PureFunctionIdentifier.Driver(compiler).process(externs, root); + new FlowSensitiveInlineVariables(compiler).process(externs, root); } }; } diff --git a/test/com/google/javascript/jscomp/FunctionInjectorTest.java b/test/com/google/javascript/jscomp/FunctionInjectorTest.java index 9282b862351..a06f230611b 100644 --- a/test/com/google/javascript/jscomp/FunctionInjectorTest.java +++ b/test/com/google/javascript/jscomp/FunctionInjectorTest.java @@ -1863,7 +1863,7 @@ public void helperInlineReferenceToFunction( final Node expectedRoot = parseExpected(new Compiler(), expectedResult); Node mainRoot = tree; - MarkNoSideEffectCalls mark = new MarkNoSideEffectCalls(compiler); + PureFunctionIdentifier.Driver mark = new PureFunctionIdentifier.Driver(compiler); mark.process(externsRoot, mainRoot); Normalize normalize = new Normalize(compiler, false); diff --git a/test/com/google/javascript/jscomp/FunctionToBlockMutatorTest.java b/test/com/google/javascript/jscomp/FunctionToBlockMutatorTest.java index 062caeb8bcc..a83f74967af 100644 --- a/test/com/google/javascript/jscomp/FunctionToBlockMutatorTest.java +++ b/test/com/google/javascript/jscomp/FunctionToBlockMutatorTest.java @@ -288,7 +288,7 @@ public void helperMutate(String code, String expectedResult, String fnName, Stri compiler.externsRoot = new Node(Token.ROOT); compiler.jsRoot = IR.root(script); compiler.externAndJsRoot = IR.root(compiler.externsRoot, compiler.jsRoot); - MarkNoSideEffectCalls mark = new MarkNoSideEffectCalls(compiler); + PureFunctionIdentifier.Driver mark = new PureFunctionIdentifier.Driver(compiler); mark.process(compiler.externsRoot, compiler.jsRoot); final Node fnNode = findFunction(script, fnName); diff --git a/test/com/google/javascript/jscomp/InlineFunctionsTest.java b/test/com/google/javascript/jscomp/InlineFunctionsTest.java index 7405caebdab..c471fbd7347 100644 --- a/test/com/google/javascript/jscomp/InlineFunctionsTest.java +++ b/test/com/google/javascript/jscomp/InlineFunctionsTest.java @@ -2538,7 +2538,7 @@ public void testIssue5159924b() { @Test public void testInlineObject() { disableCompareAsTree(); - enableMarkNoSideEffects(); + enableComputeSideEffects(); this.inliningReach = Reach.LOCAL_ONLY; assumeStrictThis = true; diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java index 29c72e6d792..85567c262d3 100644 --- a/test/com/google/javascript/jscomp/IntegrationTest.java +++ b/test/com/google/javascript/jscomp/IntegrationTest.java @@ -2422,18 +2422,6 @@ public void testMarkPureCalls() { test(options, testCode, "function foo() {}"); } - @Test - public void testMarkNoSideEffects() { - String testCode = "noSideEffects();"; - CompilerOptions options = createCompilerOptions(); - options.setRemoveDeadCode(true); - - testSame(options, testCode); - - options.markNoSideEffectCalls = true; - test(options, testCode, ""); - } - @Test public void testExtraAnnotationNames() { CompilerOptions options = createCompilerOptions(); diff --git a/test/com/google/javascript/jscomp/MarkNoSideEffectCallsTest.java b/test/com/google/javascript/jscomp/MarkNoSideEffectCallsTest.java deleted file mode 100644 index 19a14a13d8d..00000000000 --- a/test/com/google/javascript/jscomp/MarkNoSideEffectCallsTest.java +++ /dev/null @@ -1,291 +0,0 @@ -/* - * Copyright 2009 The Closure Compiler Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.javascript.jscomp; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.common.collect.ImmutableList; -import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; -import com.google.javascript.rhino.Node; -import java.util.ArrayList; -import java.util.List; -import org.junit.After; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** - * Tests for {@link MarkNoSideEffectCalls} - * - */ -@RunWith(JUnit4.class) -public final class MarkNoSideEffectCallsTest extends CompilerTestCase { - List noSideEffectCalls = new ArrayList<>(); - - private static final String EXTERNS = - "function externSef1(){}" - + "/**@nosideeffects*/function externNsef1(){}" - + "var externSef2 = function(){};" - + "/**@nosideeffects*/var externNsef2 = function(){};" - + "var externNsef3 = /**@nosideeffects*/function(){};" - + "var externObj;" - + "externObj.sef1 = function(){};" - + "/**@nosideeffects*/externObj.nsef1 = function(){};" - + "externObj.nsef2 = /**@nosideeffects*/function(){};" - + "externObj.sef2;" - + "/**@nosideeffects*/externObj.nsef3;"; - - public MarkNoSideEffectCallsTest() { - super(EXTERNS); - } - - @Override - protected int getNumRepetitions() { - // run pass once. - return 1; - } - - @Override - @After - public void tearDown() throws Exception { - super.tearDown(); - noSideEffectCalls.clear(); - } - - @Test - public void testFunctionAnnotation() { - testMarkCalls("/**@nosideeffects*/function f(){}", "f()", - ImmutableList.of("f")); - testMarkCalls("/**@nosideeffects*/var f = function(){};", "f()", - ImmutableList.of("f")); - testMarkCalls("var f = /**@nosideeffects*/function(){};", "f()", - ImmutableList.of("f")); - testMarkCalls("f = /**@nosideeffects*/function(){};", "f()", - ImmutableList.of("f")); - testMarkCalls("/**@nosideeffects*/ f = function(){};", "f()", - ImmutableList.of("f")); - - // no annotation - testMarkCalls("function f(){}", ImmutableList.of()); - testMarkCalls("function f(){} f()", ImmutableList.of()); - - // 2 annotations - testMarkCalls("/**@nosideeffects*/var f = " + - "/**@nosideeffects*/function(){};", - "f()", - ImmutableList.of("f")); - } - - @Test - public void testNamespaceAnnotation() { - testMarkCalls("var o = {}; o.f = /**@nosideeffects*/function(){};", - "o.f()", ImmutableList.of("o.f")); - testMarkCalls("var o = {}; o.f = /**@nosideeffects*/function(){};", - "o.f()", ImmutableList.of("o.f")); - testMarkCalls("var o = {}; o.f = function(){}; o.f()", ImmutableList.of()); - } - - @Test - public void testConstructorAnnotation() { - testMarkCalls("/**@nosideeffects*/function c(){};", "new c", - ImmutableList.of("c")); - testMarkCalls("var c = /**@nosideeffects*/function(){};", "new c", - ImmutableList.of("c")); - testMarkCalls("/**@nosideeffects*/var c = function(){};", "new c", - ImmutableList.of("c")); - testMarkCalls("function c(){}; new c", ImmutableList.of()); - } - - @Test - public void testMultipleDefinition() { - testMarkCalls("/**@nosideeffects*/function f(){}" + - "/**@nosideeffects*/f = function(){};", - "f()", - ImmutableList.of("f")); - testMarkCalls( - "function f(){}" + "/**@nosideeffects*/f = function(){};", "f()", ImmutableList.of()); - testMarkCalls( - "/**@nosideeffects*/function f(){}", "f = function(){};" + "f()", ImmutableList.of()); - } - - @Test - public void testAssignNoFunction() { - testMarkCalls("/**@nosideeffects*/function f(){}", "f = 1; f()", - ImmutableList.of("f")); - testMarkCalls("/**@nosideeffects*/function f(){}", "f = 1 || 2; f()", ImmutableList.of()); - } - - @Test - public void testPrototype() { - testMarkCalls("function c(){};" + - "/**@nosideeffects*/c.prototype.g = function(){};", - "var o = new c; o.g()", - ImmutableList.of("o.g")); - testMarkCalls("function c(){};" + - "/**@nosideeffects*/c.prototype.g = function(){};", - "function f(){}" + - "var o = new c; o.g(); f()", - ImmutableList.of("o.g")); - - // replace o.f with a function that has side effects - testMarkCalls("function c(){};" + - "/**@nosideeffects*/c.prototype.g = function(){};", - "var o = new c;" + - "o.g = function(){};" + - "o.g()", - ImmutableList.of()); - // two classes with same property; neither has side effects - testMarkCalls("function c1(){};" + - "/**@nosideeffects*/c1.prototype.f = function(){};" + - "function c2(){};" + - "/**@nosideeffects*/c2.prototype.f = function(){};", - "var o = new c1;" + - "o.f()", - ImmutableList.of("o.f")); - - // two classes with same property; one has side effects - testMarkCalls( - "function c1(){};" + "/**@nosideeffects*/c1.prototype.f = function(){};", - "function c2(){};" + "c2.prototype.f = function(){};" + "var o = new c1;" + "o.f()", - ImmutableList.of()); - } - - @Test - public void testAnnotationInExterns() { - testMarkCalls("externSef1()", ImmutableList.of()); - testMarkCalls("externSef2()", ImmutableList.of()); - testMarkCalls("externNsef1()", ImmutableList.of("externNsef1")); - testMarkCalls("externNsef2()", ImmutableList.of("externNsef2")); - testMarkCalls("externNsef3()", ImmutableList.of("externNsef3")); - } - - @Test - public void testNamespaceAnnotationInExterns() { - testMarkCalls("externObj.sef1()", ImmutableList.of()); - testMarkCalls("externObj.sef2()", ImmutableList.of()); - testMarkCalls("externObj.nsef1()", ImmutableList.of("externObj.nsef1")); - testMarkCalls("externObj.nsef2()", ImmutableList.of("externObj.nsef2")); - - testMarkCalls("externObj.nsef3()", ImmutableList.of("externObj.nsef3")); - } - - @Test - public void testOverrideDefinitionInSource() { - // both have side effects. - testMarkCalls("var obj = {}; obj.sef1 = function(){}; obj.sef1()", ImmutableList.of()); - - // extern has side effects. - testMarkCalls( - "var obj = {};" + "/**@nosideeffects*/obj.sef1 = function(){};", - "obj.sef1()", - ImmutableList.of()); - - // override in source also has side effects. - testMarkCalls("var obj = {}; obj.nsef1 = function(){}; obj.nsef1()", ImmutableList.of()); - - // override in source also has no side effects. - testMarkCalls("var obj = {};" + - "/**@nosideeffects*/obj.nsef1 = function(){};", - "obj.nsef1()", - ImmutableList.of("obj.nsef1")); - } - - @Test - public void testApply1() { - testMarkCalls("/**@nosideeffects*/ var f = function() {}", - "f.apply()", - ImmutableList.of("f.apply")); - } - - @Test - public void testApply2() { - testMarkCalls("var f = function() {}", - "f.apply()", - ImmutableList.of()); - } - - @Test - public void testCall1() { - testMarkCalls("/**@nosideeffects*/ var f = function() {}", - "f.call()", - ImmutableList.of("f.call")); - } - - @Test - public void testCall2() { - testMarkCalls("var f = function() {}", - "f.call()", - ImmutableList.of()); - } - - @Test - public void testCallNumber() { - testMarkCalls("", "var x = 1; x();", - ImmutableList.of()); - } - - void testMarkCalls(String source, List expected) { - testMarkCalls("", source, expected); - } - - void testMarkCalls( - String extraExterns, String source, List expected) { - testSame(externs(EXTERNS + extraExterns), srcs(source)); - assertThat(noSideEffectCalls).isEqualTo(expected); - noSideEffectCalls.clear(); - } - - @Override - protected CompilerPass getProcessor(Compiler compiler) { - return new NoSideEffectCallEnumerator(compiler); - } - - /** - * Run MarkNoSideEffectCalls, then gather a list of calls that are - * marked as having no side effects. - */ - private class NoSideEffectCallEnumerator - extends AbstractPostOrderCallback implements CompilerPass { - private final MarkNoSideEffectCalls passUnderTest; - private final Compiler compiler; - - NoSideEffectCallEnumerator(Compiler compiler) { - this.passUnderTest = new MarkNoSideEffectCalls(compiler); - this.compiler = compiler; - } - - @Override - public void process(Node externs, Node root) { - passUnderTest.process(externs, root); - NodeTraversal.traverse(compiler, externs, this); - NodeTraversal.traverse(compiler, root, this); - } - - @Override - public void visit(NodeTraversal t, Node n, Node parent) { - if (n.isNew()) { - if (!NodeUtil.constructorCallHasSideEffects(n)) { - noSideEffectCalls.add(n.getFirstChild().getQualifiedName()); - } - } else if (n.isCall()) { - if (!NodeUtil.functionCallHasSideEffects(n)) { - noSideEffectCalls.add(n.getFirstChild().getQualifiedName()); - } - } - } - } -} diff --git a/test/com/google/javascript/jscomp/RemoveUnusedCodeNameAnalyzerTest.java b/test/com/google/javascript/jscomp/RemoveUnusedCodeNameAnalyzerTest.java index 51f907f31a4..3d3b22ea430 100644 --- a/test/com/google/javascript/jscomp/RemoveUnusedCodeNameAnalyzerTest.java +++ b/test/com/google/javascript/jscomp/RemoveUnusedCodeNameAnalyzerTest.java @@ -60,7 +60,8 @@ public final class RemoveUnusedCodeNameAnalyzerTest extends CompilerTestCase { "function doThing1() {}", "function doThing2() {}", "function use(something) {}", - "function alert(something) {}"); + "function alert(something) {}", + "function sideEffect() {}"); public RemoveUnusedCodeNameAnalyzerTest() { super(EXTERNS); @@ -78,11 +79,11 @@ protected CompilerPass getProcessor(Compiler compiler) { } private static class MarkNoSideEffectCallsAndRemoveUnusedCodeRunner implements CompilerPass { - MarkNoSideEffectCalls markNoSideEffectCalls; + PureFunctionIdentifier.Driver pureFunctionIdentifier; RemoveUnusedCode removeUnusedCode; MarkNoSideEffectCallsAndRemoveUnusedCodeRunner(Compiler compiler) { - this.markNoSideEffectCalls = new MarkNoSideEffectCalls(compiler); + this.pureFunctionIdentifier = new PureFunctionIdentifier.Driver(compiler); this.removeUnusedCode = new RemoveUnusedCode.Builder(compiler) .removeGlobals(true) @@ -99,7 +100,7 @@ private static class MarkNoSideEffectCallsAndRemoveUnusedCodeRunner implements C @Override public void process(Node externs, Node root) { - markNoSideEffectCalls.process(externs, root); + pureFunctionIdentifier.process(externs, root); removeUnusedCode.process(externs, root); } } @@ -434,73 +435,75 @@ public void testSideEffectClassification7() { @Test public void testNoSideEffectAnnotation1() { - test("function f(){} var a = f();", "function f(){} f()"); + test(externs("function f(){}"), srcs("var a = f();"), expected("f()")); - test("function f(){} let a = f();", "function f(){} f()"); + test(externs("function f(){}"), srcs("let a = f();"), expected("f()")); - test("function f(){} const a = f();", "function f(){} f()"); + test(externs("function f(){}"), srcs("const a = f();"), expected("f()")); } @Test public void testNoSideEffectAnnotation2() { - test( - externs("/**@nosideeffects*/function f(){}"), - srcs("var a = f();"), - expected("")); + test(externs("/** @nosideeffects */ function f(){}"), srcs("var a = f();"), expected("")); } @Test public void testNoSideEffectAnnotation3() { - test("var f = function(){}; var a = f();", "var f = function(){}; f();"); + test(externs("var f = function(){};"), srcs("var a = f();"), expected("f()")); } @Test public void testNoSideEffectAnnotation4() { test( - externs("var f = /**@nosideeffects*/function(){};"), - srcs("var a = f();"), - expected("")); + externs("var f = /** @nosideeffects */ function(){};"), srcs("var a = f();"), expected("")); + + test( + externs("/** @nosideeffects */ var f = function(){};"), srcs("var a = f();"), expected("")); } @Test public void testNoSideEffectAnnotation5() { - test("var f; f = function(){}; var a = f();", "var f; f = function(){}; f();"); + test( + "var f; f = function(){alert('a')}; var a = f();", + "var f; f = function(){alert('a')}; f();"); } @Test public void testNoSideEffectAnnotation6() { - test( - externs("f = /**@nosideeffects*/function(){};"), - srcs("var a = f();"), - expected("")); + test(externs("f = /** @nosideeffects */ function(){};"), srcs("var a = f();"), expected("")); } @Test public void testNoSideEffectAnnotation7() { test( - externs("var f = /**@nosideeffects*/function(){};"), + externs("var f = /** @nosideeffects */ function(){};"), srcs("f = function(){};var a = f();"), - expected("f = function(){}; f();")); + expected("f = function(){};")); + + test( + externs("function sideEffect() {}; var f = /** @nosideeffects */ function(){};"), + srcs("f = function(){sideEffect()};var a = f();"), + expected("f = function(){sideEffect()};f()")); } @Test public void testNoSideEffectAnnotation8() { test( - externs("var f = function(){}; f = /**@nosideeffects*/function(){};"), // preserve newline + externs("var f = function(){}; f = /** @nosideeffects */ function(){};"), srcs("var a = f();"), - expected(" f();")); + expected("f();")); } @Test public void testNoSideEffectAnnotation9() { test( externs( - "f = /**@nosideeffects*/function(){};" + "f = /**@nosideeffects*/function(){};"), + "f = /** @nosideeffects */ function(){};", "f = /** @nosideeffects */ function(){};"), srcs("var a = f();"), expected("")); test( - externs("f = /**@nosideeffects*/function(){};"), + externs("f = /** @nosideeffects */ function(){};"), // srcs("var a = f();"), expected("")); } @@ -508,28 +511,27 @@ public void testNoSideEffectAnnotation9() { @Test public void testNoSideEffectAnnotation10() { test( - "var o = {}; o.f = function(){}; var a = o.f();", "var o = {}; o.f = function(){}; o.f();"); + externs("var o = {}; o.f = function(){};"), // + srcs("var a = o.f();"), + expected("o.f();")); } @Test public void testNoSideEffectAnnotation11() { test( - externs("var o = {}; o.f = /**@nosideeffects*/function(){};"), + externs("var o = {}; o.f = /** @nosideeffects */ function(){};"), srcs("var a = o.f();"), expected("")); } @Test public void testNoSideEffectAnnotation12() { - test("function c(){} var a = new c", "function c(){} new c"); + test(externs("function c(){}"), srcs("var a = new c()"), expected("new c()")); } @Test public void testNoSideEffectAnnotation13() { - test( - externs("/**@nosideeffects*/function c(){}"), - srcs("var a = new c"), - expected("")); + test(externs("/** @nosideeffects */ function c(){}"), srcs("var a = new c"), expected("")); } @Test @@ -544,16 +546,17 @@ public void testNoSideEffectAnnotation14() { @Test public void testNoSideEffectAnnotation15() { test( - "function c(){}; c.prototype.f = function(){}; var a = (new c).f()", - "function c(){}; c.prototype.f = function(){}; (new c).f()"); + externs("/** @nosideeffects */ function c(){}", "c.prototype.f = function(){};"), + srcs("var a = (new c).f()"), + expected("(new c).f()")); } @Test public void testNoSideEffectAnnotation16() { test( externs( - "/**@nosideeffects*/function c(){}" - + "c.prototype.f = /**@nosideeffects*/function(){};"), + "/** @nosideeffects */ function c(){}", + "c.prototype.f = /** @nosideeffects */ function(){};"), srcs("var a = (new c).f()"), expected("")); } @@ -937,15 +940,15 @@ public void testAnonymous9() { @Test public void testFunctions1() { test( - "var foo = null; function baz() {} function bar() {foo=baz();} bar();", - " function baz() {} function bar() { baz();} bar();"); + "var foo = null; function baz() {sideEffect()} function bar() {foo=baz();} bar();", + " function baz() {sideEffect()} function bar() { baz();} bar();"); } @Test public void testFunctions2() { test( - "var foo; foo = function() {var a = bar()}; var bar = function(){}; foo();", - "var foo; foo = function() { bar()}; var bar = function(){}; foo();"); + "var foo; foo = function() {var a = bar()}; var bar = function(){sideEffect()}; foo();", + "var foo; foo = function() { bar()}; var bar = function(){sideEffect()}; foo();"); } @Test @@ -1833,8 +1836,8 @@ public void testAssign3() { @Test public void testAssign4() { test( - "function Foo(){} var foo = null; var f = {};" + "f.b = new Foo();", - "function Foo(){} new Foo()"); + "function Foo(){sideEffect()} var foo = null; var f = {};" + "f.b = new Foo();", + "function Foo(){sideEffect()} new Foo()"); } @Test @@ -2017,8 +2020,8 @@ public void testRefChain16() { @Test public void testRefChain17() { test( - "function f(){} var a = 1; var b = a; var c = f(); var d = c[b]", - "function f(){} f() "); + "function f(){sideEffect()} var a = 1; var b = a; var c = f(); var d = c[b]", + "function f(){sideEffect()} f() "); } @Test @@ -2434,10 +2437,10 @@ public void testObjectDefinePropertiesOnPrototype2() { public void testObjectDefinePropertiesOnPrototype3() { test( lines( - "var b = function() {};", + "var b = function() {sideEffect()};", "function Foo() {}", "Object.defineProperties(Foo.prototype, {prop: {value: b()}});"), - "var b = function() {}; ({prop: {value: b()}});"); + "var b = function() {sideEffect()}; ({prop: {value: b()}});"); } @Test