From e02df6b51f621e2b7f73c0d9ebeb1edd86b14903 Mon Sep 17 00:00:00 2001 From: dimvar Date: Thu, 10 Nov 2016 12:51:33 -0800 Subject: [PATCH] Rework the recent change that preserves cast information after inlining. Remove the methods from NodeUtil, as they aren't really general purpose, and add the functionality to FunctionInjector directly. Add an integration test to make sure that j2cl inlining and disambiguate-properties are working together as expected. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=138793388 --- .../javascript/jscomp/ConformanceRules.java | 3 +- .../javascript/jscomp/FunctionInjector.java | 13 ++++-- .../google/javascript/jscomp/NodeUtil.java | 11 ----- .../javascript/jscomp/IntegrationTest.java | 33 +++++++++++++ .../javascript/jscomp/NodeUtilTest.java | 46 +------------------ 5 files changed, 44 insertions(+), 62 deletions(-) diff --git a/src/com/google/javascript/jscomp/ConformanceRules.java b/src/com/google/javascript/jscomp/ConformanceRules.java index 4abf4ae55e4..3f0586af78e 100644 --- a/src/com/google/javascript/jscomp/ConformanceRules.java +++ b/src/com/google/javascript/jscomp/ConformanceRules.java @@ -311,8 +311,7 @@ protected boolean isAssertionCall(Node n) { } protected boolean isTypeImmediatelyTightened(Node n) { - Node parent = n.getParent(); - return NodeUtil.wasCasted(n) || isAssertionCall(parent); + return isAssertionCall(n.getParent()) || /* casted node */ n.getTypeIBeforeCast() != null; } protected boolean isUsed(Node n) { diff --git a/src/com/google/javascript/jscomp/FunctionInjector.java b/src/com/google/javascript/jscomp/FunctionInjector.java index aa3c9bc0296..5849d1f105a 100644 --- a/src/com/google/javascript/jscomp/FunctionInjector.java +++ b/src/com/google/javascript/jscomp/FunctionInjector.java @@ -22,7 +22,7 @@ import com.google.javascript.jscomp.ExpressionDecomposer.DecompositionType; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; - +import com.google.javascript.rhino.TypeI; import java.util.Collection; import java.util.HashSet; import java.util.Map; @@ -291,8 +291,11 @@ private Node inlineReturnValue(Reference ref, Node fnNode) { } // If the call site had a cast ensure it's persisted to the new expression that replaces it. - NodeUtil.maybePropagateCast(callNode, newExpression); - + TypeI typeBeforeCast = callNode.getTypeIBeforeCast(); + if (typeBeforeCast != null) { + newExpression.putProp(Node.TYPE_BEFORE_CAST, typeBeforeCast); + newExpression.setTypeI(callNode.getTypeI()); + } callParentNode.replaceChild(callNode, newExpression); return newExpression; } @@ -563,7 +566,9 @@ private Node inlineFunction( private static void removeConstantVarAnnotation(Scope scope, String name) { Var var = scope.getVar(name); Node nameNode = var == null ? null : var.getNameNode(); - if (nameNode == null) return; + if (nameNode == null) { + return; + } if (nameNode.getBooleanProp(Node.IS_CONSTANT_VAR)) { nameNode.removeProp(Node.IS_CONSTANT_VAR); diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index 18e03ec30d5..8e3a9b6b671 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -4868,15 +4868,4 @@ static boolean isGetterOrSetter(Node propNode) { static boolean isCallTo(Node n, String qualifiedName) { return n.isCall() && n.getFirstChild().matchesQualifiedName(qualifiedName); } - - static boolean wasCasted(Node node) { - return node.getProp(Node.TYPE_BEFORE_CAST) != null; - } - - static void maybePropagateCast(Node source, Node destination) { - if (wasCasted(source)) { - destination.putProp(Node.TYPE_BEFORE_CAST, source.getProp(Node.TYPE_BEFORE_CAST)); - destination.setJSType(source.getJSType()); - } - } } diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java index 7f0ba258990..b3ab92c1371 100644 --- a/test/com/google/javascript/jscomp/IntegrationTest.java +++ b/test/com/google/javascript/jscomp/IntegrationTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.javascript.jscomp.CompilerOptions.DisposalCheckingPolicy; +import com.google.javascript.jscomp.CompilerOptions.J2clPassMode; import com.google.javascript.jscomp.CompilerOptions.LanguageMode; import com.google.javascript.jscomp.CompilerOptions.Reach; import com.google.javascript.jscomp.deps.ModuleLoader; @@ -1768,6 +1769,38 @@ public void testDeadAssignmentsElimination() { test(options, code, "function f() { var x; 3; 4; x = 5; return x; } f();"); } + public void testPreservesCastInformation() { + // Only set the suffix instead of both prefix and suffix, because j2cl pass + // looks for that exact suffix, and IntegrationTestCase adds an input + // id number between prefix and suffix. + inputFileNameSuffix = "vmbootstrap/Arrays.impl.java.js"; + CompilerOptions options = new CompilerOptions(); + String code = LINE_JOINER.join( + "/** @constructor */", + "var Arrays = function() {};", + "Arrays.$create = function() { return {}; }", + "/** @constructor */", + "function Foo() { this.myprop = 1; }", + "/** @constructor */", + "function Bar() { this.myprop = 2; }", + "var x = /** @type {!Foo} */ (Arrays.$create()).myprop;"); + + options.setCheckTypes(true); + options.setJ2clPass(J2clPassMode.TRUE); + options.setDisambiguateProperties(true); + + test(options, code, + LINE_JOINER.join( + "/** @constructor */", + "var Arrays = function() {};", + "Arrays.$create = function() { return {}; }", + "/** @constructor */", + "function Foo() { this.Foo$myprop = 1; }", + "/** @constructor */", + "function Bar() { this.Bar$myprop = 2; }", + "var x = {}.Foo$myprop;")); + } + public void testInlineFunctions() { CompilerOptions options = createCompilerOptions(); String code = "function f() { return 3; } f(); "; diff --git a/test/com/google/javascript/jscomp/NodeUtilTest.java b/test/com/google/javascript/jscomp/NodeUtilTest.java index 789d68d756e..49eb0def2b6 100644 --- a/test/com/google/javascript/jscomp/NodeUtilTest.java +++ b/test/com/google/javascript/jscomp/NodeUtilTest.java @@ -25,8 +25,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.javascript.jscomp.CompilerOptions.LanguageMode; -import com.google.javascript.jscomp.type.ReverseAbstractInterpreter; -import com.google.javascript.jscomp.type.SemanticReverseAbstractInterpreter; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.JSTypeExpression; import com.google.javascript.rhino.Node; @@ -42,29 +40,15 @@ */ public final class NodeUtilTest extends TestCase { - private static Node parse(String js, boolean typeCheck) { + private static Node parse(String js) { Compiler compiler = new Compiler(); compiler.initCompilerOptionsIfTesting(); compiler.getOptions().setLanguageIn(LanguageMode.ECMASCRIPT6); Node n = compiler.parseTestCode(js); - if (typeCheck) { - ReverseAbstractInterpreter rai = - new SemanticReverseAbstractInterpreter(compiler.getTypeRegistry()); - new TypeCheck(compiler, rai, compiler.getTypeRegistry()) - .processForTesting(n.getFirstChild(), n.getLastChild()); - } assertThat(compiler.getErrors()).isEmpty(); return n; } - private static Node parse(String js) { - return parse(js, false); - } - - private static Node parseAndTypeCheck(String js) { - return parse(js, true); - } - static Node getNode(String js) { Node root = parse("var a=(" + js + ");"); Node expr = root.getFirstChild(); @@ -2459,34 +2443,6 @@ public void testIsObjectDefinePropertyDefinition() { getCallNode("Object.defineProperty();"))); } - public void testWasCasted() { - assertFalse(NodeUtil.wasCasted(getCallNode(parseAndTypeCheck("var x = foo();")))); - assertTrue( - NodeUtil.wasCasted( - getCallNode(parseAndTypeCheck("var x = /** @type {string} */ (foo());")))); - } - - public void testMaybePropagateCastTo() { - Node ast = parseAndTypeCheck("var x = /** @type {string} */ (foo());"); - Node x = getNameNode(ast, "x"); - Node call = getCallNode(ast); - - assertFalse(NodeUtil.wasCasted(x)); - NodeUtil.maybePropagateCast(call, x); - assertTrue(NodeUtil.wasCasted(x)); - } - - public void testMaybePropagateCastTo_noExistingCast() { - Node ast = parseAndTypeCheck("var x = foo();"); - Node x = getNameNode(ast, "x"); - Node call = getCallNode(ast); - - assertFalse(NodeUtil.wasCasted(x)); - NodeUtil.maybePropagateCast(call, x); - assertFalse(NodeUtil.wasCasted(x)); - } - - private boolean executedOnceTestCase(String code) { Node ast = parse(code); Node nameNode = getNameNode(ast, "x");