diff --git a/src/com/google/javascript/jscomp/PureFunctionIdentifier.java b/src/com/google/javascript/jscomp/PureFunctionIdentifier.java index bbc28255c92..755d26291c1 100644 --- a/src/com/google/javascript/jscomp/PureFunctionIdentifier.java +++ b/src/com/google/javascript/jscomp/PureFunctionIdentifier.java @@ -34,11 +34,11 @@ import com.google.javascript.jscomp.graph.FixedPointGraphTraversal; import com.google.javascript.jscomp.graph.FixedPointGraphTraversal.EdgeCallback; import com.google.javascript.jscomp.graph.LinkedDirectedGraph; +import com.google.javascript.rhino.FunctionTypeI; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; -import com.google.javascript.rhino.jstype.FunctionType; -import com.google.javascript.rhino.jstype.JSType; +import com.google.javascript.rhino.TypeI; import com.google.javascript.rhino.jstype.JSTypeNative; import java.io.File; import java.io.IOException; @@ -903,11 +903,11 @@ private void updateSideEffectsFromExtern(Node externFunction, AbstractCompiler c JSDocInfo info = NodeUtil.getBestJSDocInfo(externFunction); // Handle externs. - JSType jstype = externFunction.getJSType(); - FunctionType functionType = JSType.toMaybeFunctionType(jstype); + TypeI typei = externFunction.getTypeI(); + FunctionTypeI functionType = typei == null ? null : typei.toMaybeFunctionType(); if (functionType != null) { - JSType jstypeReturn = functionType.getReturnType(); - if (!PureFunctionIdentifier.isLocalValueType(jstypeReturn, compiler)) { + TypeI retType = functionType.getReturnType(); + if (!PureFunctionIdentifier.isLocalValueType(retType, compiler)) { setTaintsReturn(); } } @@ -937,14 +937,13 @@ private void updateSideEffectsFromExtern(Node externFunction, AbstractCompiler c * * @return Whether the jstype is something known to be a local value. */ - private static boolean isLocalValueType(JSType jstype, AbstractCompiler compiler) { - Preconditions.checkNotNull(jstype); - JSType subtype = - jstype.getGreatestSubtype( - (JSType) compiler.getTypeIRegistry().getNativeType(JSTypeNative.OBJECT_TYPE)); + private static boolean isLocalValueType(TypeI typei, AbstractCompiler compiler) { + Preconditions.checkNotNull(typei); + TypeI nativeObj = compiler.getTypeIRegistry().getNativeType(JSTypeNative.OBJECT_TYPE); + TypeI subtype = typei.meetWith(nativeObj); // If the type includes anything related to a object type, don't assume // anything about the locality of the value. - return subtype.isNoType(); + return subtype.isBottom(); } /** diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java index 383067701f4..39fbf875de4 100644 --- a/test/com/google/javascript/jscomp/CompilerTestCase.java +++ b/test/com/google/javascript/jscomp/CompilerTestCase.java @@ -217,6 +217,14 @@ public abstract class CompilerTestCase extends TestCase { "function String(arg) {}", "/** @param {number} sliceArg */", "String.prototype.slice = function(sliceArg) {};", + "/**", + " * @this {?String|string}", + " * @param {?} regex", + " * @param {?} str", + " * @param {string=} opt_flags", + " * @return {string}", + " */", + "String.prototype.replace = function(regex, str, opt_flags) {};", "/** @type {number} */ String.prototype.length;", "/**", " * @template T", @@ -246,6 +254,19 @@ public abstract class CompilerTestCase extends TestCase { "Arguments.prototype.length;", "/**", " * @constructor", + " * @param {*=} opt_pattern", + " * @param {*=} opt_flags", + " * @return {!RegExp}", + " * @nosideeffects", + " */", + "function RegExp(opt_pattern, opt_flags) {}", + "/**", + " * @param {*} str The string to search.", + " * @return {?Array}", + " */", + "RegExp.prototype.exec = function(str) {};", + "/**", + " * @constructor", " */", // TODO(bradfordcsmith): Copy fields for this from es5.js this when we have test cases // that depend on them. diff --git a/test/com/google/javascript/jscomp/CompilerTypeTestCase.java b/test/com/google/javascript/jscomp/CompilerTypeTestCase.java index c8a72637c73..ff778423697 100644 --- a/test/com/google/javascript/jscomp/CompilerTypeTestCase.java +++ b/test/com/google/javascript/jscomp/CompilerTypeTestCase.java @@ -30,38 +30,37 @@ */ abstract class CompilerTypeTestCase extends BaseJSTypeTestCase { - static final String CLOSURE_DEFS = - "var goog = {};" + - "goog.inherits = function(x, y) {};" + - "/** @type {!Function} */ goog.abstractMethod = function() {};" + - "goog.isArray = function(x) {};" + - "goog.isDef = function(x) {};" + - "goog.isFunction = function(x) {};" + - "goog.isNull = function(x) {};" + - "goog.isString = function(x) {};" + - "goog.isObject = function(x) {};" + - "goog.isDefAndNotNull = function(x) {};" + - "goog.array = {};" + + static final String CLOSURE_DEFS = LINE_JOINER.join( + "/** @const */ var goog = {};", + "goog.inherits = function(x, y) {};", + "/** @type {!Function} */ goog.abstractMethod = function() {};", + "goog.isArray = function(x) {};", + "goog.isDef = function(x) {};", + "goog.isFunction = function(x) {};", + "goog.isNull = function(x) {};", + "goog.isString = function(x) {};", + "goog.isObject = function(x) {};", + "goog.isDefAndNotNull = function(x) {};", + "/** @const */ goog.array = {};", // simplified ArrayLike definition - "/**\n" + - " * @typedef {Array|{length: number}}\n" + - " */\n" + - "goog.array.ArrayLike;" + - "/**\n" + - " * @param {Array.|{length:number}} arr\n" + - " * @param {function(this:S, T, number, goog.array.ArrayLike):boolean} f\n" + - " * @param {S=} opt_obj\n" + - " * @return {!Array.}\n" + - " * @template T,S\n" + - " */" + + "/**", + " * @typedef {Array|{length: number}}", + " */", + "goog.array.ArrayLike;", + "/**", + " * @param {Array.|{length:number}} arr", + " * @param {function(this:S, T, number, goog.array.ArrayLike):boolean} f", + " * @param {S=} opt_obj", + " * @return {!Array.}", + " * @template T,S", + " */", // return empty array to satisfy return type - "goog.array.filter = function(arr, f, opt_obj){ return []; };" + - "goog.asserts = {};" + - "/** @return {*} */ goog.asserts.assert = function(x) { return x; };"; + "goog.array.filter = function(arr, f, opt_obj){ return []; };", + "goog.asserts = {};", + "/** @return {*} */ goog.asserts.assert = function(x) { return x; };"); /** A default set of externs for testing. */ - static final String DEFAULT_EXTERNS = - CompilerTestCase.DEFAULT_EXTERNS; + static final String DEFAULT_EXTERNS = CompilerTestCase.DEFAULT_EXTERNS; protected Compiler compiler; diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java b/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java index 1c8662fe501..5fbaf10f55f 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java @@ -145,13 +145,6 @@ public abstract class NewTypeInferenceTestBase extends CompilerTypeTestCase { "function Error(opt_message, opt_file, opt_line) {}", "/** @type {string} */", "Error.prototype.stack;", - "/**", - " * @constructor", - " * @param {?=} opt_pattern", - " * @param {?=} opt_flags", - " * @return {!RegExp}", - " */", - "function RegExp(opt_pattern, opt_flags) {}", "/** @constructor */", "function Window() {}", "/** @type {boolean} */", diff --git a/test/com/google/javascript/jscomp/PureFunctionIdentifierTest.java b/test/com/google/javascript/jscomp/PureFunctionIdentifierTest.java index 29c2529bf1d..225e8dd72f0 100644 --- a/test/com/google/javascript/jscomp/PureFunctionIdentifierTest.java +++ b/test/com/google/javascript/jscomp/PureFunctionIdentifierTest.java @@ -32,155 +32,154 @@ * @author johnlenz@google.com (John Lenz) */ -public final class PureFunctionIdentifierTest extends CompilerTestCase { - private static final List NO_PURE_CALLS = ImmutableList.of(); - - List noSideEffectCalls = new ArrayList<>(); - List localResultCalls = new ArrayList<>(); +public final class PureFunctionIdentifierTest extends TypeICompilerTestCase { + List noSideEffectCalls; + List localResultCalls; boolean regExpHaveSideEffects = true; - private static final String TEST_EXTERNS = LINE_JOINER.join( - "var window; window.setTimeout;", - "/**@nosideeffects*/ function externSENone(){}", + private static final String TEST_EXTERNS = + CompilerTypeTestCase.DEFAULT_EXTERNS + LINE_JOINER.join( + "var window; window.setTimeout;", + "/**@nosideeffects*/ function externSENone(){}", - "/**@modifies{this}*/ function externSEThis(){}", + "/**@modifies{this}*/ function externSEThis(){}", - "/**@constructor", - " * @modifies{this}*/", - "function externObjSEThis(){}", + "/**@constructor", + " * @modifies{this}*/", + "function externObjSEThis(){}", - "/**", - " * @param {string} s id.", - " * @return {string}", - " * @modifies{this}", - " */", - "externObjSEThis.prototype.externObjSEThisMethod = function(s) {};", + "/**", + " * @param {string} s id.", + " * @return {string}", + " * @modifies{this}", + " */", + "externObjSEThis.prototype.externObjSEThisMethod = function(s) {};", - "/**", - " * @param {string} s id.", - " * @return {string}", - " * @modifies{arguments}", - " */", - "externObjSEThis.prototype.externObjSEThisMethod2 = function(s) {};", + "/**", + " * @param {string} s id.", + " * @return {string}", + " * @modifies{arguments}", + " */", + "externObjSEThis.prototype.externObjSEThisMethod2 = function(s) {};", - "/**@nosideeffects*/function Error(){}", + "/**@nosideeffects*/function Error(){}", - "function externSef1(){}", + "function externSef1(){}", - "/**@nosideeffects*/function externNsef1(){}", + "/**@nosideeffects*/function externNsef1(){}", - "var externSef2 = function(){};", + "var externSef2 = function(){};", - "/**@nosideeffects*/var externNsef2 = function(){};", + "/**@nosideeffects*/var externNsef2 = function(){};", - "var externNsef3 = /**@nosideeffects*/function(){};", + "var externNsef3 = /**@nosideeffects*/function(){};", - "var externObj;", + "var externObj;", - "externObj.sef1 = function(){};", + "externObj.sef1 = function(){};", - "/**@nosideeffects*/externObj.nsef1 = function(){};", + "/**@nosideeffects*/externObj.nsef1 = function(){};", - "externObj.nsef2 = /**@nosideeffects*/function(){};", + "externObj.nsef2 = /**@nosideeffects*/function(){};", - "externObj.partialFn;", + "externObj.partialFn;", - "externObj.partialSharedFn;", + "externObj.partialSharedFn;", - "var externObj2;", + "var externObj2;", - "externObj2.partialSharedFn = /**@nosideeffects*/function(){};", + "externObj2.partialSharedFn = /**@nosideeffects*/function(){};", - "/**@constructor*/function externSefConstructor(){}", + "/**@constructor*/function externSefConstructor(){}", - "externSefConstructor.prototype.sefFnOfSefObj = function(){};", + "externSefConstructor.prototype.sefFnOfSefObj = function(){};", - "externSefConstructor.prototype.nsefFnOfSefObj = ", - " /**@nosideeffects*/function(){};", + "externSefConstructor.prototype.nsefFnOfSefObj = ", + " /**@nosideeffects*/function(){};", - "externSefConstructor.prototype.externShared = function(){};", + "externSefConstructor.prototype.externShared = function(){};", - "/**@constructor@nosideeffects*/function externNsefConstructor(){}", + "/**@constructor@nosideeffects*/function externNsefConstructor(){}", - "externNsefConstructor.prototype.sefFnOfNsefObj = function(){};", + "externNsefConstructor.prototype.sefFnOfNsefObj = function(){};", - "externNsefConstructor.prototype.nsefFnOfNsefObj = ", - " /**@nosideeffects*/function(){};", + "externNsefConstructor.prototype.nsefFnOfNsefObj = ", + " /**@nosideeffects*/function(){};", - "externNsefConstructor.prototype.externShared = ", - " /**@nosideeffects*/function(){};", + "externNsefConstructor.prototype.externShared = ", + " /**@nosideeffects*/function(){};", - "/**@constructor @nosideeffects*/function externNsefConstructor2(){}", - "externNsefConstructor2.prototype.externShared = ", - " /**@nosideeffects*/function(){};", + "/**@constructor @nosideeffects*/function externNsefConstructor2(){}", + "externNsefConstructor2.prototype.externShared = ", + " /**@nosideeffects*/function(){};", - "externNsefConstructor.prototype.sharedPartialSef;", - "/**@nosideeffects*/externNsefConstructor.prototype.sharedPartialNsef;", + "externNsefConstructor.prototype.sharedPartialSef;", + "/**@nosideeffects*/externNsefConstructor.prototype.sharedPartialNsef;", - // An externs definition with a stub before. - "/**@constructor*/function externObj3(){}", + // An externs definition with a stub before. + "/**@constructor*/function externObj3(){}", - "externObj3.prototype.propWithStubBefore;", + "externObj3.prototype.propWithStubBefore;", - "/**", - " * @param {string} s id.", - " * @return {string}", - " * @nosideeffects", - " */", - "externObj3.prototype.propWithStubBefore = function(s) {};", + "/**", + " * @param {string} s id.", + " * @return {string}", + " * @nosideeffects", + " */", + "externObj3.prototype.propWithStubBefore = function(s) {};", - // useless JsDoc - "/**", - " * @see {foo}", - " */", - "externObj3.prototype.propWithStubBeforeWithJSDoc;", + // useless JsDoc + "/**", + " * @see {foo}", + " */", + "externObj3.prototype.propWithStubBeforeWithJSDoc;", - "/**", - " * @param {string} s id.", - " * @return {string}", - " * @nosideeffects", - " */", - "externObj3.prototype.propWithStubBeforeWithJSDoc = function(s) {};", + "/**", + " * @param {string} s id.", + " * @return {string}", + " * @nosideeffects", + " */", + "externObj3.prototype.propWithStubBeforeWithJSDoc = function(s) {};", - // An externs definition with a stub after. - "/**@constructor*/function externObj4(){}", + // An externs definition with a stub after. + "/**@constructor*/function externObj4(){}", - "/**", - " * @param {string} s id.", - " * @return {string}", - " * @nosideeffects", - " */", - "externObj4.prototype.propWithStubAfter = function(s) {};", + "/**", + " * @param {string} s id.", + " * @return {string}", + " * @nosideeffects", + " */", + "externObj4.prototype.propWithStubAfter = function(s) {};", - "externObj4.prototype.propWithStubAfter;", + "externObj4.prototype.propWithStubAfter;", - "/**", - " * @param {string} s id.", - " * @return {string}", - " * @nosideeffects", - " */", - "externObj4.prototype.propWithStubAfterWithJSDoc = function(s) {};", + "/**", + " * @param {string} s id.", + " * @return {string}", + " * @nosideeffects", + " */", + "externObj4.prototype.propWithStubAfterWithJSDoc = function(s) {};", - // useless JsDoc - "/**", - " * @see {foo}", - " */", - "externObj4.prototype.propWithStubAfterWithJSDoc;", - "var goog = {reflect: {}};", - "goog.reflect.cache = function(a, b, c, opt_d) {};", + // useless JsDoc + "/**", + " * @see {foo}", + " */", + "externObj4.prototype.propWithStubAfterWithJSDoc;", + "var goog = {};", + "goog.reflect = {};", + "goog.reflect.cache = function(a, b, c, opt_d) {};", - "/** @nosideeffects */", - "externObj.prototype.duplicateExternFunc = function() {}", - "externObj2.prototype.duplicateExternFunc = function() {}", + "/** @nosideeffects */", + "externObj.prototype.duplicateExternFunc = function() {};", + "externObj2.prototype.duplicateExternFunc = function() {};", - "externObj.prototype['weirdDefinition'] = function() {}" - ); + "externObj.prototype['weirdDefinition'] = function() {};" + ); public PureFunctionIdentifierTest() { - super(CompilerTypeTestCase.DEFAULT_EXTERNS + TEST_EXTERNS); - enableTypeCheck(); + super(TEST_EXTERNS); } @Override @@ -192,11 +191,79 @@ protected int getNumRepetitions() { @Override protected void tearDown() throws Exception { super.tearDown(); - noSideEffectCalls.clear(); - localResultCalls.clear(); regExpHaveSideEffects = true; } + /** + * Run PureFunctionIdentifier, then gather a list of calls that are + * marked as having no side effects. + */ + private class NoSideEffectCallEnumerator + extends AbstractPostOrderCallback implements CompilerPass { + private final Compiler compiler; + + NoSideEffectCallEnumerator(Compiler compiler) { + this.compiler = compiler; + } + + @Override + public void process(Node externs, Node root) { + noSideEffectCalls = new ArrayList<>(); + localResultCalls = new ArrayList<>(); + compiler.setHasRegExpGlobalReferences(regExpHaveSideEffects); + compiler.getOptions().setUseTypesForLocalOptimization(true); + NameBasedDefinitionProvider defFinder = new NameBasedDefinitionProvider(compiler, true); + defFinder.process(externs, root); + + PureFunctionIdentifier pureFunctionIdentifier = + new PureFunctionIdentifier(compiler, defFinder); + pureFunctionIdentifier.process(externs, root); + + // Ensure that debug report computation doesn't crash. + pureFunctionIdentifier.getDebugReport(); + + NodeTraversal.traverseEs6(compiler, externs, this); + NodeTraversal.traverseEs6(compiler, root, this); + } + + @Override + public void visit(NodeTraversal t, Node n, Node parent) { + if (n.isNew()) { + if (!NodeUtil.constructorCallHasSideEffects(n)) { + noSideEffectCalls.add(generateNameString(n.getFirstChild())); + } + } else if (n.isCall()) { + if (!NodeUtil.functionCallHasSideEffects(n, compiler)) { + noSideEffectCalls.add(generateNameString(n.getFirstChild())); + } + if (NodeUtil.callHasLocalResult(n)) { + localResultCalls.add(generateNameString(n.getFirstChild())); + } + } + } + + private String generateNameString(Node node) { + if (node.isOr()) { + return "(" + generateNameString(node.getFirstChild()) + + " || " + generateNameString(node.getLastChild()) + ")"; + } else if (node.isHook()) { + return "(" + generateNameString(node.getSecondChild()) + + " : " + generateNameString(node.getLastChild()) + ")"; + } else { + String result = node.getQualifiedName(); + if (result == null) { + if (node.isFunction()) { + result = node.toString(false, false, false).trim(); + } else { + result = node.getFirstChild().toString(false, false, false); + result += " " + node.getLastChild().toString(false, false, false); + } + } + return result; + } + } + } + public void testIssue303() throws Exception { String source = LINE_JOINER.join( "/** @constructor */ function F() {", @@ -210,7 +277,7 @@ public void testIssue303() throws Exception { "};", "(new F()).setLocation('http://www.google.com/');" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testIssue303b() throws Exception { @@ -228,7 +295,7 @@ public void testIssue303b() throws Exception { " (new F()).setLocation('http://www.google.com/');", "} window['x'] = x;" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAnnotationInExterns_new1() throws Exception { @@ -236,7 +303,7 @@ public void testAnnotationInExterns_new1() throws Exception { } public void testAnnotationInExterns_new2() throws Exception { - assertPureCallsMarked("externSEThis()", NO_PURE_CALLS); + assertNoPureCalls("externSEThis()"); } public void testAnnotationInExterns_new3() throws Exception { @@ -330,11 +397,11 @@ public void testAnnotationInExterns_new10() throws Exception { } public void testAnnotationInExterns1() throws Exception { - assertPureCallsMarked("externSef1()", NO_PURE_CALLS); + assertNoPureCalls("externSef1()"); } public void testAnnotationInExterns2() throws Exception { - assertPureCallsMarked("externSef2()", NO_PURE_CALLS); + assertNoPureCalls("externSef2()"); } public void testAnnotationInExterns3() throws Exception { @@ -350,7 +417,7 @@ public void testAnnotationInExterns5() throws Exception { } public void testNamespaceAnnotationInExterns1() throws Exception { - assertPureCallsMarked("externObj.sef1()", NO_PURE_CALLS); + assertNoPureCalls("externObj.sef1()"); } public void testNamespaceAnnotationInExterns2() throws Exception { @@ -362,7 +429,7 @@ public void testNamespaceAnnotationInExterns3() throws Exception { } public void testNamespaceAnnotationInExterns4() throws Exception { - assertPureCallsMarked("externObj.partialFn()", NO_PURE_CALLS); + assertNoPureCalls("externObj.partialFn()"); } public void testNamespaceAnnotationInExterns5() throws Exception { @@ -375,22 +442,22 @@ public void testNamespaceAnnotationInExterns5() throws Exception { assertPureCallsMarked( templateSrc.replace("", "notPartialFn"), ImmutableList.of("o.notPartialFn")); - assertPureCallsMarked(templateSrc.replace("", "partialFn"), NO_PURE_CALLS); + assertNoPureCalls(templateSrc.replace("", "partialFn")); } public void testNamespaceAnnotationInExterns6() throws Exception { - assertPureCallsMarked("externObj.partialSharedFn()", NO_PURE_CALLS); + assertNoPureCalls("externObj.partialSharedFn()"); } public void testConstructorAnnotationInExterns1() throws Exception { - assertPureCallsMarked("new externSefConstructor()", NO_PURE_CALLS); + assertNoPureCalls("new externSefConstructor()"); } public void testConstructorAnnotationInExterns2() throws Exception { String source = LINE_JOINER.join( "var a = new externSefConstructor();", "a.sefFnOfSefObj()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testConstructorAnnotationInExterns3() throws Exception { @@ -404,7 +471,7 @@ public void testConstructorAnnotationInExterns4() throws Exception { String source = LINE_JOINER.join( "var a = new externSefConstructor();", "a.externShared()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testConstructorAnnotationInExterns5() throws Exception { @@ -487,7 +554,7 @@ public void testAnnotationInExternStubs2() throws Exception { } public void testAnnotationInExternStubs3() throws Exception { - assertPureCallsMarked("propWithAnnotatedStubAfter('a');", NO_PURE_CALLS); + assertNoPureCalls("propWithAnnotatedStubAfter('a');"); } public void testAnnotationInExternStubs4() throws Exception { @@ -505,11 +572,19 @@ public void testAnnotationInExternStubs4() throws Exception { " */", "externObj5.prototype.propWithAnnotatedStubAfter;"); - testSame(externs, + this.mode = TypeInferenceMode.OTI_ONLY; + testSame( + externs, "o.prototype.propWithAnnotatedStubAfter", TypeValidator.DUP_VAR_DECLARATION_TYPE_MISMATCH, false); assertThat(noSideEffectCalls).isEmpty(); - noSideEffectCalls.clear(); + + this.mode = TypeInferenceMode.NTI_ONLY; + testSame( + TEST_EXTERNS + externs, + "o.prototype.propWithAnnotatedStubAfter", + GlobalTypeInfo.REDECLARED_PROPERTY, false); + assertThat(noSideEffectCalls).isEmpty(); } public void testAnnotationInExternStubs5() throws Exception { @@ -531,11 +606,17 @@ public void testAnnotationInExternStubs5() throws Exception { " */", "externObj5.prototype.propWithAnnotatedStubAfter;"); + this.mode = TypeInferenceMode.OTI_ONLY; testSame(externs, "o.prototype.propWithAnnotatedStubAfter", TypeValidator.DUP_VAR_DECLARATION, false); - assertEquals(NO_PURE_CALLS, noSideEffectCalls); - noSideEffectCalls.clear(); + assertThat(noSideEffectCalls).isEmpty(); + + this.mode = TypeInferenceMode.NTI_ONLY; + testSame(TEST_EXTERNS + externs, + "o.prototype.propWithAnnotatedStubAfter", + GlobalTypeInfo.REDECLARED_PROPERTY, false); + assertThat(noSideEffectCalls).isEmpty(); } public void testNoSideEffectsSimple() throws Exception { @@ -626,10 +707,10 @@ public void testResultLocalitySimple() throws Exception { prefix + "var a = {foo : 1}; return a.foo" + suffix, fReturnsNonLocal); // read from extern - checkLocalityOfMarkedCalls(prefix + "return externObj" + suffix, NO_PURE_CALLS); + checkLocalityOfMarkedCalls(prefix + "return externObj" + suffix, ImmutableList.of()); checkLocalityOfMarkedCalls( "function inner(x) { x.foo = 3; }" /* to suppress missing property */ + - prefix + "return externObj.foo" + suffix, NO_PURE_CALLS); + prefix + "return externObj.foo" + suffix, ImmutableList.of()); } /** @@ -670,21 +751,21 @@ public void testExternCalls() throws Exception { assertPureCallsMarked(prefix + "externObj.nsef1()" + suffix, ImmutableList.of("externObj.nsef1", "f")); - assertPureCallsMarked(prefix + "externSef1()" + suffix, NO_PURE_CALLS); - assertPureCallsMarked(prefix + "externObj.sef1()" + suffix, NO_PURE_CALLS); + assertNoPureCalls(prefix + "externSef1()" + suffix); + assertNoPureCalls(prefix + "externObj.sef1()" + suffix); } public void testApply() throws Exception { String source = LINE_JOINER.join( "function f() {return 42}", - "f.apply()"); + "f.apply(null)"); assertPureCallsMarked(source, ImmutableList.of("f.apply")); } public void testCall() throws Exception { String source = LINE_JOINER.join( "function f() {return 42}", - "f.call()"); + "f.call(null)"); assertPureCallsMarked(source, ImmutableList.of("f.call")); } @@ -692,17 +773,17 @@ public void testApplyToUnknownDefinition() throws Exception { String source = LINE_JOINER.join( "var dict = {'func': function() {}};", "function f() { var s = dict['func'];}", - "f.apply()" + "f.apply(null)" ); assertPureCallsMarked(source, ImmutableList.of("f.apply")); - // Not marked becuase the definition cannot be found so unknown side effects. + // Not marked because the definition cannot be found so unknown side effects. source = LINE_JOINER.join( "var dict = {'func': function() {}};", "function f() { var s = dict['func'].apply();}", - "f.apply()" + "f.apply(null)" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); // Not marked becuase the definition cannot be found so unknown side effects. source = LINE_JOINER.join( @@ -711,7 +792,7 @@ public void testApplyToUnknownDefinition() throws Exception { "function f() { var s = (dict['func'] || pure)();}", "f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); // Not marked becuase the definition cannot be found so unknown side effects. source = LINE_JOINER.join( @@ -720,7 +801,7 @@ public void testApplyToUnknownDefinition() throws Exception { , "function f() { var s = (condition ? dict['func'] : pure)();}" , "f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testInference1() throws Exception { @@ -739,7 +820,7 @@ public void testInference2() throws Exception { "function g() {a=2}", "f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testInference3() throws Exception { @@ -758,12 +839,11 @@ public void testInference4() throws Exception { "var g = function() {a=2};", "f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testInference5() throws Exception { String source = LINE_JOINER.join( - "var goog = {};", "goog.f = function() {return goog.g()};", "goog.g = function() {return 42};", "goog.f()" @@ -774,12 +854,11 @@ public void testInference5() throws Exception { public void testInference6() throws Exception { String source = LINE_JOINER.join( "var a = 1;", - "var goog = {};", "goog.f = function() {goog.g()};", "goog.g = function() {a=2};", "goog.f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testLocalizedSideEffects1() throws Exception { @@ -803,7 +882,7 @@ public void testLocalizedSideEffects2() throws Exception { "}", "f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testLocalizedSideEffects3() throws Exception { @@ -814,7 +893,7 @@ public void testLocalizedSideEffects3() throws Exception { "function f() {var x = g; x.foo++};", "f();" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testLocalizedSideEffects4() throws Exception { @@ -834,7 +913,7 @@ public void testLocalizedSideEffects5() throws Exception { "function f() {var x = g; x[0] = 1;};", "f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testLocalizedSideEffects6() throws Exception { @@ -943,7 +1022,7 @@ public void testUnaryOperators2() throws Exception { "var x = 1;", "function f() {x++}", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testUnaryOperators3() throws Exception { @@ -958,7 +1037,7 @@ public void testUnaryOperators4() throws Exception { "var x = {foo : 0};", "function f() {x.foo++}", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testUnaryOperators5() throws Exception { @@ -973,7 +1052,7 @@ public void testDeleteOperator1() throws Exception { "var x = {};", "function f() {delete x}", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testDeleteOperator2() throws Exception { @@ -987,77 +1066,77 @@ public void testOrOperator1() throws Exception { String source = LINE_JOINER.join( "var f = externNsef1 || externNsef2;", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testOrOperator2() throws Exception { String source = LINE_JOINER.join( "var f = function(){} || externNsef2;", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testOrOperator3() throws Exception { String source = LINE_JOINER.join( "var f = externNsef2 || function(){};", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testOrOperators4() throws Exception { String source = LINE_JOINER.join( "var f = function(){} || function(){};", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAndOperator1() throws Exception { String source = LINE_JOINER.join( "var f = externNsef1 && externNsef2;", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAndOperator2() throws Exception { String source = LINE_JOINER.join( "var f = function(){} && externNsef2;", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAndOperator3() throws Exception { String source = LINE_JOINER.join( "var f = externNsef2 && function(){};", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAndOperators4() throws Exception { String source = LINE_JOINER.join( "var f = function(){} && function(){};", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testHookOperator1() throws Exception { String source = LINE_JOINER.join( "var f = true ? externNsef1 : externNsef2;", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testHookOperator2() throws Exception { String source = LINE_JOINER.join( "var f = true ? function(){} : externNsef2;", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testHookOperator3() throws Exception { String source = LINE_JOINER.join( "var f = true ? externNsef2 : function(){};", "f()"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testHookOperators4() throws Exception { @@ -1158,7 +1237,7 @@ public void testAmbiguousDefinitions() throws Exception { "sideEffectCaller();" ); // Can't tell which f is being called so it assumes both. - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAmbiguousDefinitionsCall() throws Exception { @@ -1170,7 +1249,7 @@ public void testAmbiguousDefinitionsCall() throws Exception { "sideEffectCaller();" ); // Can't tell which f is being called so it assumes both. - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAmbiguousDefinitionsAllPropagationTypes() throws Exception { @@ -1190,12 +1269,22 @@ public void testAmbiguousDefinitionsCallWithThis() throws Exception { "var globalVar = 1;", "A.modifiesThis = function() { this.x = 5; };", "/**@constructor*/function Constructor() { Constructor.modifiesThis.call(this); };", - "Constructor.prototype.modifiesThis = function() {};", + "Constructor.modifiesThis = function() {};", "new Constructor();", "A.modifiesThis();" ); + // Can't tell which modifiesThis is being called so it assumes both. + + this.mode = TypeInferenceMode.OTI_ONLY; assertPureCallsMarked(source, ImmutableList.of("Constructor")); + + this.mode = TypeInferenceMode.NTI_ONLY; + assertPureCallsMarked( + source, + ImmutableList.of("Constructor"), + // modifiesThis not defined on Constructor + NewTypeInference.GLOBAL_THIS); } public void testAmbiguousDefinitionsBothCallThis() throws Exception { @@ -1211,7 +1300,12 @@ public void testAmbiguousDefinitionsBothCallThis() throws Exception { " this.x = 2;", "}", "new C();"); + + this.mode = TypeInferenceMode.OTI_ONLY; assertPureCallsMarked(source, ImmutableList.of("C")); + + this.mode = TypeInferenceMode.NTI_ONLY; + assertPureCallsMarked(source, ImmutableList.of("C"), NewTypeInference.GLOBAL_THIS); } public void testAmbiguousDefinitionsAllCallThis() throws Exception { @@ -1226,7 +1320,12 @@ public void testAmbiguousDefinitionsAllCallThis() throws Exception { "new h();", "i();" // With better locals tracking i could be identified as pure ); + + this.mode = TypeInferenceMode.OTI_ONLY; assertPureCallsMarked(source, ImmutableList.of("F.f.apply", "h")); + + this.mode = TypeInferenceMode.NTI_ONLY; + assertPureCallsMarked(source, ImmutableList.of("F.f.apply", "h"), NewTypeInference.GLOBAL_THIS); } public void testAmbiguousDefinitionsMutatesGlobalArgument() throws Exception { @@ -1240,7 +1339,7 @@ public void testAmbiguousDefinitionsMutatesGlobalArgument() throws Exception { "B.a = function() {};", "var b = function(x) {C.a(x)};", "b({});"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAmbiguousDefinitionsMutatesLocalArgument() throws Exception { @@ -1260,7 +1359,7 @@ public void testAmbiguousDefinitionsMutatesLocalArgument() throws Exception { } public void testAmbiguousExternDefinitions() { - assertPureCallsMarked("x.duplicateExternFunc()", NO_PURE_CALLS); + assertNoPureCalls("x.duplicateExternFunc()"); // nsef1 is defined as no side effect in the externs. String source = LINE_JOINER.join( @@ -1269,7 +1368,7 @@ public void testAmbiguousExternDefinitions() { "A.nsef1 = function () {global = 2;};", "externObj.nsef1();" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } /** @@ -1284,7 +1383,7 @@ public void testAmbiguousDefinitionsDoubleDefinition() { "B.x = function() {}", "B.x();" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAmbiguousDefinitionsDoubleDefinition2() { @@ -1294,7 +1393,7 @@ public void testAmbiguousDefinitionsDoubleDefinition2() { "a = function() {}", "B.x(); a();" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testAmbiguousDefinitionsDoubleDefinition3() { @@ -1334,12 +1433,20 @@ public void testAmbiguousDefinitionsDoubleDefinition6() { " x[dataName] = dataValue;", "}", "SetCustomData1(window, \"foo\", \"bar\");"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testCallBeforeDefinition() throws Exception { assertPureCallsMarked("f(); function f(){}", ImmutableList.of("f")); + + this.mode = TypeInferenceMode.OTI_ONLY; assertPureCallsMarked("var a = {}; a.f(); a.f = function (){}", ImmutableList.of("a.f")); + + this.mode = TypeInferenceMode.NTI_ONLY; + assertPureCallsMarked( + "var a = {}; a.f(); a.f = function (){}", + ImmutableList.of("a.f"), + NewTypeInference.INEXISTENT_PROPERTY); } public void testConstructorThatModifiesThis1() throws Exception { @@ -1380,7 +1487,12 @@ public void testConstructorThatModifiesThis4() throws Exception { "function f() {return new A}", "f()" ); + + this.mode = TypeInferenceMode.OTI_ONLY; assertPureCallsMarked(source, ImmutableList.of("A", "f")); + + this.mode = TypeInferenceMode.NTI_ONLY; + assertPureCallsMarked(source, ImmutableList.of("A", "f"), NewTypeInference.GLOBAL_THIS); } public void testConstructorThatModifiesGlobal1() throws Exception { @@ -1390,7 +1502,7 @@ public void testConstructorThatModifiesGlobal1() throws Exception { "function f() {return new A}", "f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testConstructorThatModifiesGlobal2() throws Exception { @@ -1400,7 +1512,7 @@ public void testConstructorThatModifiesGlobal2() throws Exception { "function f() {return new A}", "f()" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testCallFunctionThatModifiesThis() throws Exception { @@ -1425,7 +1537,7 @@ public void testMutatesArguments2() throws Exception { String source = LINE_JOINER.join( "function f(x) { x.y = 1; }", "f(window);"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testMutatesArguments3() throws Exception { @@ -1434,7 +1546,7 @@ public void testMutatesArguments3() throws Exception { "function f(x) { x.y = 1; }", "function g(x) { f(x); }", "g({});"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testMutatesArguments4() throws Exception { @@ -1454,7 +1566,7 @@ public void testMutatesArguments5() throws Exception { " g();", "}", "f(window);"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testMutatesArgumentsArray1() throws Exception { @@ -1469,14 +1581,14 @@ public void testMutatesArgumentsArray2() throws Exception { String source = LINE_JOINER.join( "function f(x) { arguments[0].y = 1; }", "f({});"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testMutatesArgumentsArray3() throws Exception { String source = LINE_JOINER.join( "function f(x) { arguments[0].y = 1; }", "f(x);"); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testCallFunctionFOrG() throws Exception { @@ -1547,7 +1659,7 @@ public void testCallRegExpWithSideEffects() throws Exception { ); regExpHaveSideEffects = true; - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); regExpHaveSideEffects = false; assertPureCallsMarked(source, ImmutableList.of( "REGEXP STRING exec", "k")); @@ -1577,13 +1689,13 @@ public void testAnonymousFunction4() throws Exception { ); // This should be "(Error || FUNCTION)" but isn't. - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); } public void testFunctionProperties1() throws Exception { String source = LINE_JOINER.join( "/** @constructor */", - "function F() {}", + "function F() { this.bar; }", "function g() {", " this.bar = function() { alert(3); };", "}", @@ -1591,14 +1703,24 @@ public void testFunctionProperties1() throws Exception { "g.call(x);", "x.bar();" ); - assertPureCallsMarked(source, ImmutableList.of("F")); + this.mode = TypeInferenceMode.OTI_ONLY; + assertPureCallsMarked(source, ImmutableList.of("F")); Node lastRoot = getLastCompiler().getRoot(); Node call = findQualifiedNameNode("g.call", lastRoot).getParent(); assertEquals( new Node.SideEffectFlags() .clearAllFlags().setMutatesArguments().valueOf(), call.getSideEffectFlags()); + + this.mode = TypeInferenceMode.NTI_ONLY; + assertPureCallsMarked(source, ImmutableList.of("F"), NewTypeInference.GLOBAL_THIS); + lastRoot = getLastCompiler().getRoot(); + call = findQualifiedNameNode("g.call", lastRoot).getParent(); + assertEquals( + new Node.SideEffectFlags() + .clearAllFlags().setMutatesArguments().valueOf(), + call.getSideEffectFlags()); } public void testCallCache() throws Exception { @@ -1640,7 +1762,7 @@ public void testCallCache_anonymousFn_hasSideEffects() throws Exception { "var x = 0;", "goog.reflect.cache(externObj, \"foo\", function(v) { return (x+=1) })" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); Node lastRoot = getLastCompiler().getRoot().getLastChild(); Node call = findQualifiedNameNode("goog.reflect.cache", lastRoot).getParent(); assertThat(call.isNoSideEffectsCall()).isFalse(); @@ -1653,7 +1775,7 @@ public void testCallCache_hasSideEffects() throws Exception { "var valueFn = function() { return (x+=1); };", "goog.reflect.cache(externObj, \"foo\", valueFn)" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); Node lastRoot = getLastCompiler().getRoot().getLastChild(); Node call = findQualifiedNameNode("goog.reflect.cache", lastRoot).getParent(); assertThat(call.isNoSideEffectsCall()).isFalse(); @@ -1667,7 +1789,7 @@ public void testCallCache_withKeyFn_hasSideEffects() throws Exception { "var valueFn = function(v) { return v };", "goog.reflect.cache(externObj, \"foo\", valueFn, keyFn)" ); - assertPureCallsMarked(source, NO_PURE_CALLS); + assertNoPureCalls(source); Node lastRoot = getLastCompiler().getRoot().getLastChild(); Node call = findQualifiedNameNode("goog.reflect.cache", lastRoot).getParent(); assertThat(call.isNoSideEffectsCall()).isFalse(); @@ -1691,20 +1813,28 @@ public void testCallCache_propagatesSideEffects() throws Exception { assertThat(helperCall.mayMutateGlobalStateOrThrow()).isFalse(); } + void assertNoPureCalls(String source) { + assertPureCallsMarked(source, ImmutableList.of(), null); + } + void assertPureCallsMarked(String source, List expected) { - assertPureCallsMarked(source, expected, LanguageMode.ECMASCRIPT6); - assertPureCallsMarked(source, expected, LanguageMode.ECMASCRIPT5); + assertPureCallsMarked(source, expected, null); + } + + void assertPureCallsMarked(String source, List expected, DiagnosticType warning) { + assertPureCallsMarked(source, expected, warning, LanguageMode.ECMASCRIPT_2015); + assertPureCallsMarked(source, expected, warning, LanguageMode.ECMASCRIPT5); } - void assertPureCallsMarked(String source, List expected, LanguageMode mode) { + void assertPureCallsMarked( + String source, List expected, DiagnosticType warning, LanguageMode mode) { setAcceptedLanguage(mode); - testSame(source); + testSame(source, warning); assertEquals(expected, noSideEffectCalls); - noSideEffectCalls.clear(); } void checkLocalityOfMarkedCalls(String source, List expected) { - checkLocalityOfMarkedCalls(source, expected, LanguageMode.ECMASCRIPT6); + checkLocalityOfMarkedCalls(source, expected, LanguageMode.ECMASCRIPT_2015); checkLocalityOfMarkedCalls(source, expected, LanguageMode.ECMASCRIPT5); } @@ -1712,79 +1842,10 @@ void checkLocalityOfMarkedCalls(String source, List expected, LanguageMo setAcceptedLanguage(mode); testSame(source); assertEquals(expected, localResultCalls); - localResultCalls.clear(); } @Override protected CompilerPass getProcessor(Compiler compiler) { return new NoSideEffectCallEnumerator(compiler); } - - /** - * Run PureFunctionIdentifier, then gather a list of calls that are - * marked as having no side effects. - */ - private class NoSideEffectCallEnumerator - extends AbstractPostOrderCallback implements CompilerPass { - private final Compiler compiler; - - NoSideEffectCallEnumerator(Compiler compiler) { - this.compiler = compiler; - } - - @Override - public void process(Node externs, Node root) { - compiler.setHasRegExpGlobalReferences(regExpHaveSideEffects); - compiler.getOptions().setUseTypesForLocalOptimization(true); - NameBasedDefinitionProvider defFinder = new NameBasedDefinitionProvider(compiler, true); - defFinder.process(externs, root); - - PureFunctionIdentifier pureFunctionIdentifier = - new PureFunctionIdentifier(compiler, defFinder); - pureFunctionIdentifier.process(externs, root); - - // Ensure that debug report computation doesn't crash. - pureFunctionIdentifier.getDebugReport(); - - NodeTraversal.traverseEs6(compiler, externs, this); - NodeTraversal.traverseEs6(compiler, root, this); - } - - @Override - public void visit(NodeTraversal t, Node n, Node parent) { - if (n.isNew()) { - if (!NodeUtil.constructorCallHasSideEffects(n)) { - noSideEffectCalls.add(generateNameString(n.getFirstChild())); - } - } else if (n.isCall()) { - if (!NodeUtil.functionCallHasSideEffects(n, compiler)) { - noSideEffectCalls.add(generateNameString(n.getFirstChild())); - } - if (NodeUtil.callHasLocalResult(n)) { - localResultCalls.add(generateNameString(n.getFirstChild())); - } - } - } - - private String generateNameString(Node node) { - if (node.isOr()) { - return "(" + generateNameString(node.getFirstChild()) + - " || " + generateNameString(node.getLastChild()) + ")"; - } else if (node.isHook()) { - return "(" + generateNameString(node.getSecondChild()) + - " : " + generateNameString(node.getLastChild()) + ")"; - } else { - String result = node.getQualifiedName(); - if (result == null) { - if (node.isFunction()) { - result = node.toString(false, false, false).trim(); - } else { - result = node.getFirstChild().toString(false, false, false); - result += " " + node.getLastChild().toString(false, false, false); - } - } - return result; - } - } - } }