diff --git a/build-system/runner/dist/runner.jar b/build-system/runner/dist/runner.jar index 187da8887913..cefdefa08a70 100644 Binary files a/build-system/runner/dist/runner.jar and b/build-system/runner/dist/runner.jar differ diff --git a/build-system/runner/src/org/ampproject/AmpCommandLineRunner.java b/build-system/runner/src/org/ampproject/AmpCommandLineRunner.java index d7f330ade278..f2aaeca987ba 100644 --- a/build-system/runner/src/org/ampproject/AmpCommandLineRunner.java +++ b/build-system/runner/src/org/ampproject/AmpCommandLineRunner.java @@ -15,8 +15,6 @@ */ package org.ampproject; - -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.javascript.jscomp.CommandLineRunner; import com.google.javascript.jscomp.CompilerOptions; @@ -24,11 +22,8 @@ import com.google.javascript.jscomp.FlagUsageException; import com.google.javascript.jscomp.PropertyRenamingPolicy; import com.google.javascript.jscomp.VariableRenamingPolicy; -import com.google.javascript.rhino.IR; -import com.google.javascript.rhino.Node; import java.io.IOException; -import java.util.Set; /** @@ -45,8 +40,6 @@ public class AmpCommandLineRunner extends CommandLineRunner { private boolean is_production_env = true; - private String amp_version = ""; - /** * List of string suffixes to eliminate from the AST. */ @@ -72,7 +65,7 @@ protected AmpCommandLineRunner(String[] args) { } CompilerOptions options = super.createOptions(); options.setCollapsePropertiesLevel(CompilerOptions.PropertyCollapseLevel.ALL); - AmpPass ampPass = new AmpPass(getCompiler(), is_production_env, suffixTypes, amp_version); + AmpPass ampPass = new AmpPass(getCompiler(), is_production_env, suffixTypes); options.addCustomPass(CustomPassExecutionTime.BEFORE_OPTIMIZATIONS, ampPass); options.setDevirtualizePrototypeMethods(true); options.setExtractPrototypeMemberDeclarations(true); @@ -111,7 +104,6 @@ protected CompilerOptions createTypeCheckingOptions() { public static void main(String[] args) { AmpCommandLineRunner runner = new AmpCommandLineRunner(args); - // Scan for TYPECHECK_ONLY string which we pass in as a --define for (String arg : args) { if (arg.contains("TYPECHECK_ONLY=true")) { runner.typecheck_only = true; @@ -119,8 +111,6 @@ public static void main(String[] args) { runner.is_production_env = false; } else if (arg.contains("PSEUDO_NAMES=true")) { runner.pseudo_names = true; - } else if (arg.contains("VERSION=")) { - runner.amp_version = arg.substring(arg.lastIndexOf("=") + 1); } } diff --git a/build-system/runner/src/org/ampproject/AmpPass.java b/build-system/runner/src/org/ampproject/AmpPass.java index 3d2490b60380..8913b799119f 100644 --- a/build-system/runner/src/org/ampproject/AmpPass.java +++ b/build-system/runner/src/org/ampproject/AmpPass.java @@ -15,47 +15,29 @@ */ package org.ampproject; -import java.util.LinkedList; -import java.util.Map; import java.util.Set; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableMap; import com.google.javascript.jscomp.AbstractCompiler; import com.google.javascript.jscomp.HotSwapCompilerPass; import com.google.javascript.jscomp.NodeTraversal; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; -import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; /** - * Does a `stripTypeSuffix` which currently can't be done through - * the normal `strip` mechanisms provided by closure compiler. - * Some of the known mechanisms we tried before writing our own compiler pass - * are setStripTypes, setStripTypePrefixes, setStripNameSuffixes, setStripNamePrefixes. - * The normal mechanisms found in closure compiler can't strip the expressions we want because - * they are either prefix based and/or operate on the es6 translated code which would mean they - * operate on a qualifier string name that looks like - * "module$__$__$__$extensions$amp_test$0_1$log.dev.fine". - * - * Other custom pass examples found inside closure compiler src: - * https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PolymerPass.java - * https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/AngularPass.java + * Eliminates removable calls like `assert`, `assertElement`, `assertString`, etc. */ class AmpPass extends AbstractPostOrderCallback implements HotSwapCompilerPass { final AbstractCompiler compiler; private final ImmutableSet stripTypeSuffixes; final boolean isProd; - private final String amp_version; public AmpPass(AbstractCompiler compiler, boolean isProd, - ImmutableSet stripTypeSuffixes, - String amp_version) { + ImmutableSet stripTypeSuffixes) { this.compiler = compiler; this.stripTypeSuffixes = stripTypeSuffixes; this.isProd = isProd; - this.amp_version = amp_version; } @Override public void process(Node externs, Node root) { @@ -69,95 +51,7 @@ public AmpPass(AbstractCompiler compiler, boolean isProd, @Override public void visit(NodeTraversal t, Node n, Node parent) { if (isCallRemovable(n)) { maybeEliminateCallExceptFirstParam(n, parent); - } else if (isAmpExtensionCall(n)) { - inlineAmpExtensionCall(n, parent); - // Remove any `getMode().localDev` and `getMode().test` calls and replace it with `false`. - } else if (isProd && isFunctionInvokeAndPropAccess(n, "$mode.getMode", - ImmutableSet.of("localDev", "test"))) { - replaceWithBooleanExpression(false, n, parent); - // Remove any `getMode().minified` calls and replace it with `true`. - } else if (isProd && isFunctionInvokeAndPropAccess(n, "$mode.getMode", - ImmutableSet.of("minified"))) { - replaceWithBooleanExpression(true, n, parent); - } - } - - /** - * We don't care about the deep GETPROP. What we care about is finding a - * call which has an `extension` name which then has `AMP` as its - * previous getprop or name, and has a function as the 2nd argument. - * - * CALL 3 [length: 96] [source_file: input0] - * GETPROP 3 [length: 37] [source_file: input0] - * GETPROP 3 [length: 24] [source_file: input0] - * GETPROP 3 [length: 20] [source_file: input0] - * NAME self 3 [length: 4] [source_file: input0] - * STRING someproperty 3 [length: 15] [source_file: input0] - * STRING AMP 3 [length: 3] [source_file: input0] - * STRING extension 3 [length: 12] [source_file: input0] - * STRING some-string 3 [length: 9] [source_file: input0] - * FUNCTION 3 [length: 46] [source_file: input0] - */ - private boolean isAmpExtensionCall(Node n) { - if (n != null && n.isCall()) { - Node getprop = n.getFirstChild(); - - // The AST has the last getprop higher in the hierarchy. - if (isGetPropName(getprop, "extension")) { - Node firstChild = getprop.getFirstChild(); - // We have to handle both explicit/implicit top level `AMP` - if ((firstChild != null && firstChild.isName() && - firstChild.getString() == "AMP") || - isGetPropName(firstChild, "AMP")) { - // Child at index 1 should be the "string" value (first argument) - Node func = getAmpExtensionCallback(n); - return func != null && func.isFunction(); - } - } - } - return false; - } - - private boolean isGetPropName(Node n, String name) { - if (n != null && n.isGetProp()) { - Node nodeName = n.getSecondChild(); - return nodeName != null && nodeName.isString() && - nodeName.getString() == name; - } - return false; - } - - /** - * This operation should be guarded stringently by `isAmpExtensionCall` - * predicate. - * - * AMP.extension('some-name', '0.1', function(AMP) { - * // BODY... - * }); - * - * is turned into: - * (function(AMP) { - * // BODY... - * })(self.AMP); - */ - private void inlineAmpExtensionCall(Node n, Node expr) { - if (expr == null || !expr.isExprResult()) { - return; } - Node func = getAmpExtensionCallback(n); - func.detachFromParent(); - Node arg1 = IR.getprop(IR.name("self"), IR.string("AMP")); - arg1.setLength("self.AMP".length()); - arg1.useSourceInfoIfMissingFromForTree(func); - Node newcall = IR.call(func); - newcall.putBooleanProp(Node.FREE_CALL, true); - newcall.addChildToBack(arg1); - expr.replaceChild(n, newcall); - compiler.reportChangeToEnclosingScope(expr); - } - - private Node getAmpExtensionCallback(Node n) { - return n.getLastChild(); } /** @@ -176,75 +70,6 @@ private boolean isCallRemovable(Node n) { return false; } - private void maybeReplaceCallWithVersion(Node n, Node parent) { - if (n == null || !n.isCall() || amp_version.isEmpty()) { - return; - } - - String name = buildQualifiedName(n); - if (!name.equals("internalRuntimeVersion$$module$src$internal_version()")) { - return; - } - - Node version = IR.string(amp_version); - version.useSourceInfoIfMissingFrom(n); - parent.replaceChild(n, version); - compiler.reportChangeToEnclosingScope(parent); - } - - private void maybeReplaceRValueInVar(Node n, Map map) { - if (n != null && (n.isVar() || n.isLet() || n.isConst())) { - Node varNode = n.getFirstChild(); - if (varNode != null) { - for (Map.Entry mapping : map.entrySet()) { - if (varNode.getString() == mapping.getKey()) { - varNode.replaceChild(varNode.getFirstChild(), mapping.getValue()); - compiler.reportChangeToEnclosingScope(varNode); - return; - } - } - } - } - } - - /** - * Predicate for any fnQualifiedName.props call. - * example: - * isFunctionInvokeAndPropAccess(n, "getMode", "test"); // matches `getMode().test` - */ - private boolean isFunctionInvokeAndPropAccess(Node n, String fnQualifiedName, Set props) { - // mode.getMode().localDev - // mode [property] -> - // getMode [call] - // ${property} [string] - if (!n.isGetProp()) { - return false; - } - Node call = n.getFirstChild(); - if (!call.isCall()) { - return false; - } - Node fullQualifiedFnName = call.getFirstChild(); - if (fullQualifiedFnName == null) { - return false; - } - - String qualifiedName = fullQualifiedFnName.getQualifiedName(); - if (qualifiedName != null && qualifiedName.endsWith(fnQualifiedName)) { - Node maybeProp = n.getSecondChild(); - if (maybeProp != null && maybeProp.isString()) { - String name = maybeProp.getString(); - for (String prop : props) { - if (prop == name) { - return true; - } - } - } - } - - return false; - } - /** * Builds a string representation of MemberExpression and CallExpressions. */ @@ -274,13 +99,6 @@ private void buildQualifiedNameInternal(Node n, StringBuilder sb) { } } - private void replaceWithBooleanExpression(boolean bool, Node n, Node parent) { - Node booleanNode = bool ? IR.trueNode() : IR.falseNode(); - booleanNode.useSourceInfoIfMissingFrom(n); - parent.replaceChild(n, booleanNode); - compiler.reportChangeToEnclosingScope(parent); - } - private void removeExpression(Node n, Node parent) { Node scope = parent; if (parent.isExprResult()) { diff --git a/build-system/runner/test/org/ampproject/AmpPassTest.java b/build-system/runner/test/org/ampproject/AmpPassTest.java index 95878e41db2a..90a7e81641ab 100644 --- a/build-system/runner/test/org/ampproject/AmpPassTest.java +++ b/build-system/runner/test/org/ampproject/AmpPassTest.java @@ -1,8 +1,6 @@ package org.ampproject; - - import com.google.common.collect.ImmutableSet; import com.google.javascript.jscomp.Compiler; import com.google.javascript.jscomp.CompilerPass; @@ -22,7 +20,7 @@ public class AmpPassTest extends CompilerTestCase { ); @Override protected CompilerPass getProcessor(Compiler compiler) { - return new AmpPass(compiler, /* isProd */ true, suffixTypes, "123"); + return new AmpPass(compiler, /* isProd */ true, suffixTypes); } @Override protected int getNumRepetitions() { @@ -162,110 +160,6 @@ public class AmpPassTest extends CompilerTestCase { "})()")); } - @Test public void testGetModeLocalDevPropertyReplacement() throws Exception { - test( - LINE_JOINER.join( - "(function() {", - "function getMode() { return { localDev: true } }", - "var $mode = { getMode: getMode };", - " if ($mode.getMode().localDev) {", - " console.log('hello world');", - " }", - "})()"), - LINE_JOINER.join( - "(function() {", - "function getMode() { return { localDev: true }; }", - "var $mode = { getMode: getMode };", - " if (false) {", - " console.log('hello world');", - " }", - "})()")); - } - - @Test public void testGetModeTestPropertyReplacement() throws Exception { - test( - LINE_JOINER.join( - "(function() {", - "function getMode() { return { test: true } }", - "var $mode = { getMode: getMode };", - " if ($mode.getMode().test) {", - " console.log('hello world');", - " }", - "})()"), - LINE_JOINER.join( - "(function() {", - "function getMode() { return { test: true }; }", - "var $mode = { getMode: getMode };", - " if (false) {", - " console.log('hello world');", - " }", - "})()")); - } - - @Test public void testGetModeMinifiedPropertyReplacement() throws Exception { - test( - LINE_JOINER.join( - "(function() {", - "function getMode() { return { minified: false } }", - "var $mode = { getMode: getMode };", - " if ($mode.getMode().minified) {", - " console.log('hello world');", - " }", - "})()"), - LINE_JOINER.join( - "(function() {", - "function getMode() { return { minified: false }; }", - "var $mode = { getMode: getMode };", - " if (true) {", - " console.log('hello world');", - " }", - "})()")); - } - - @Test public void testGetModeWinTestPropertyReplacement() throws Exception { - test( - LINE_JOINER.join( - "(function() {", - "function getMode() { return { test: true } }", - "var win = {};", - "var $mode = { getMode: getMode };", - " if ($mode.getMode(win).test) {", - " console.log('hello world');", - " }", - "})()"), - LINE_JOINER.join( - "(function() {", - "function getMode() { return { test: true }; }", - "var win = {};", - "var $mode = { getMode: getMode };", - " if (false) {", - " console.log('hello world');", - " }", - "})()")); - } - - @Test public void testGetModeWinMinifiedPropertyReplacement() throws Exception { - test( - LINE_JOINER.join( - "(function() {", - "function getMode() { return { minified: false } }", - "var win = {};", - "var $mode = { getMode: getMode };", - " if ($mode.getMode(win).minified) {", - " console.log('hello world');", - " }", - "})()"), - LINE_JOINER.join( - "(function() {", - "function getMode() { return { minified: false }; }", - "var win = {};", - "var $mode = { getMode: getMode };", - " if (true) {", - " console.log('hello world');", - " }", - "})()")); - } - @Test public void testGetModePreserve() throws Exception { test( LINE_JOINER.join( @@ -302,40 +196,4 @@ public class AmpPassTest extends CompilerTestCase { " }", "})()")); } - - @Test public void testRemoveAmpAddExtensionCallWithExplicitContext() throws Exception { - test( - LINE_JOINER.join( - "var a = 'hello';", - "self.AMP.extension('hello', '0.1', function(AMP) {", - " var a = 'world';", - " console.log(a);", - "});", - "console.log(a);"), - LINE_JOINER.join( - "var a = 'hello';", - "(function(AMP) {", - " var a = 'world';", - " console.log(a);", - "})(self.AMP);", - "console.log(a);")); - } - - @Test public void testRemoveAmpAddExtensionCallWithNoContext() throws Exception { - test( - LINE_JOINER.join( - "var a = 'hello';", - "AMP.extension('hello', '0.1', function(AMP) {", - " var a = 'world';", - " console.log(a);", - "});", - "console.log(a);"), - LINE_JOINER.join( - "var a = 'hello';", - "(function(AMP) {", - " var a = 'world';", - " console.log(a);", - "})(self.AMP);", - "console.log(a);")); - } } diff --git a/build-system/runner/test/org/ampproject/AmpPassTestEnvTest.java b/build-system/runner/test/org/ampproject/AmpPassTestEnvTest.java index 6f984846b77c..d8b8e93d3700 100644 --- a/build-system/runner/test/org/ampproject/AmpPassTestEnvTest.java +++ b/build-system/runner/test/org/ampproject/AmpPassTestEnvTest.java @@ -1,9 +1,6 @@ package org.ampproject; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.javascript.rhino.IR; -import com.google.javascript.rhino.Node; import com.google.javascript.jscomp.Compiler; import com.google.javascript.jscomp.CompilerPass; import com.google.javascript.jscomp.CompilerTestCase; @@ -18,7 +15,7 @@ public class AmpPassTestEnvTest extends CompilerTestCase { ImmutableSet suffixTypes = ImmutableSet.of(); @Override protected CompilerPass getProcessor(Compiler compiler) { - return new AmpPass(compiler, /* isProd */ false, suffixTypes, "123"); + return new AmpPass(compiler, /* isProd */ false, suffixTypes); } @Override protected int getNumRepetitions() {