Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace some props on the object returned by getMode during compile time #3772

Merged
merged 4 commits into from Jun 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Binary file modified build-system/runner/dist/runner.jar
Binary file not shown.
13 changes: 10 additions & 3 deletions build-system/runner/src/org/ampproject/AmpCommandLineRunner.java
Expand Up @@ -35,6 +35,8 @@ public class AmpCommandLineRunner extends CommandLineRunner {
* Identifies if the runner only needs to do type checking.
*/
private boolean typecheck_only = false;

private boolean is_production_env = true;

/**
* List of string suffixes to eliminate from the AST.
Expand All @@ -52,7 +54,7 @@ protected AmpCommandLineRunner(String[] args) {
}
CompilerOptions options = super.createOptions();
options.setCollapseProperties(true);
AmpPass ampPass = new AmpPass(getCompiler(), suffixTypes);
AmpPass ampPass = new AmpPass(getCompiler(), is_production_env, suffixTypes);
options.addCustomPass(CustomPassExecutionTime.BEFORE_OPTIMIZATIONS, ampPass);
options.setDevirtualizePrototypeMethods(true);
options.setExtractPrototypeMemberDeclarations(true);
Expand Down Expand Up @@ -91,15 +93,20 @@ protected CompilerOptions createTypeCheckingOptions() {
protected void setTypeCheckOnly(boolean value) {
typecheck_only = value;
}

protected void setProductionFlag(boolean value) {
is_production_env = value;
}

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")) {
if (arg.contains("--define=TYPECHECK_ONLY=true")) {
runner.setTypeCheckOnly(true);
break;
} else if (arg.contains("--define=FORTESTING=true")) {
runner.setProductionFlag(false);
}
}

Expand Down
115 changes: 78 additions & 37 deletions build-system/runner/src/org/ampproject/AmpPass.java
Expand Up @@ -17,10 +17,12 @@

import java.util.Set;

import com.google.common.collect.ImmutableSet;
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;

/**
Expand All @@ -41,10 +43,12 @@ class AmpPass extends AbstractPostOrderCallback implements HotSwapCompilerPass {

final AbstractCompiler compiler;
private final Set<String> stripTypeSuffixes;
final boolean isProd;

public AmpPass(AbstractCompiler compiler, Set<String> stripTypeSuffixes) {
public AmpPass(AbstractCompiler compiler, boolean isProd, Set<String> stripTypeSuffixes) {
this.compiler = compiler;
this.stripTypeSuffixes = stripTypeSuffixes;
this.isProd = isProd;
}

@Override public void process(Node externs, Node root) {
Expand All @@ -56,56 +60,93 @@ public AmpPass(AbstractCompiler compiler, Set<String> stripTypeSuffixes) {
}

@Override public void visit(NodeTraversal t, Node n, Node parent) {
if (isDevAssertCall(n)) {
// Remove `dev.assert` calls and preserve first argument if any.
if (isNameStripType(n, ImmutableSet.of( "dev.assert"))) {
maybeEliminateCallExceptFirstParam(n, parent);
} else if (n.isExprResult()) {
maybeEliminateExpressionBySuffixName(n, parent);
// Remove any `stripTypes` passed in outright like `dev.warn`.
} else if (isNameStripType(n, stripTypeSuffixes)) {
removeExpression(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);
}
}

private boolean isDevAssertCall(Node n) {
if (n.isCall()) {
Node expression = n.getFirstChild();
if (expression == null) {
return false;
}

String name = expression.getQualifiedName();
if (name == null) {
return false;
}
/**
* Predicate for any <code>fnQualifiedName</code>.<code>props</code> call.
* example:
* isFunctionInvokeAndPropAccess(n, "getMode", "test"); // matches `getMode().test`
*/
private boolean isFunctionInvokeAndPropAccess(Node n, String fnQualifiedName, Set<String> 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;
}

if (name.endsWith("dev.assert")) {
return true;
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;
}

private void replaceWithBooleanExpression(boolean bool, Node n, Node parent) {
Node booleanNode = bool ? IR.trueNode() : IR.falseNode();
booleanNode.useSourceInfoIfMissingFrom(n);
parent.replaceChild(n, booleanNode);
compiler.reportCodeChange();
}

/**
* Checks if expression is a GETPROP() (method invocation) and the property
* name ends with one of the items in stripTypeSuffixes.
* This method does not do a deep check and will only do a shallow
* expression -> property -> call check.
*/
private void maybeEliminateExpressionBySuffixName(Node n, Node parent) {
// n = EXPRESSION_RESULT > CALL > GETPROP
Node call = n.getFirstChild();
if (call == null) {
return;
private boolean isNameStripType(Node n, Set<String> suffixes) {
if (!n.isCall()) {
return false;
}
Node expression = call.getFirstChild();
if (expression == null) {
return;
Node getprop = n.getFirstChild();
if (getprop == null) {
return false;
}
return qualifiedNameEndsWithStripType(getprop, suffixes);
}

if (qualifiedNameEndsWithStripType(expression)) {
if (parent.isExprResult()) {
Node grandparent = parent.getParent();
grandparent.removeChild(parent);
} else {
parent.removeChild(n);
}
compiler.reportCodeChange();
private void removeExpression(Node n, Node parent) {
if (parent.isExprResult()) {
Node grandparent = parent.getParent();
grandparent.removeChild(parent);
} else {
parent.removeChild(n);
}
compiler.reportCodeChange();
}

private void maybeEliminateCallExceptFirstParam(Node n, Node p) {
Expand All @@ -130,17 +171,17 @@ private void maybeEliminateCallExceptFirstParam(Node n, Node p) {
* Checks the nodes qualified name if it ends with one of the items in
* stripTypeSuffixes
*/
boolean qualifiedNameEndsWithStripType(Node n) {
boolean qualifiedNameEndsWithStripType(Node n, Set<String> suffixes) {
String name = n.getQualifiedName();
return qualifiedNameEndsWithStripType(name);
return qualifiedNameEndsWithStripType(name, suffixes);
}

/**
* Checks if the string ends with one of the items in stripTypeSuffixes
*/
boolean qualifiedNameEndsWithStripType(String name) {
boolean qualifiedNameEndsWithStripType(String name, Set<String> suffixes) {
if (name != null) {
for (String suffix : stripTypeSuffixes) {
for (String suffix : suffixes) {
if (name.endsWith(suffix)) {
return true;
}
Expand Down
99 changes: 98 additions & 1 deletion build-system/runner/test/org/ampproject/AmpPassTest.java
Expand Up @@ -17,7 +17,7 @@ public class AmpPassTest extends Es6CompilerTestCase {
"dev.fine");

@Override protected CompilerPass getProcessor(Compiler compiler) {
return new AmpPass(compiler, suffixTypes);
return new AmpPass(compiler, /* isProd */ true, suffixTypes);
}

@Override protected int getNumRepetitions() {
Expand Down Expand Up @@ -129,4 +129,101 @@ public void testShouldPreserveNoneCalls() throws Exception {
" console.log('this is preserved', someValue);",
"})()"));
}

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');",
" }",
"})()"));
}

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');",
" }",
"})()"));
}

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');",
" }",
"})()"));
}

public void testGetModePreserve() throws Exception {
test(
LINE_JOINER.join(
"(function() {",
"function getMode() { return { minified: false } }",
"var $mode = { getMode: getMode };",
" if ($mode.getMode()) {",
" console.log('hello world');",
" }",
"})()"),
LINE_JOINER.join(
"(function() {",
"function getMode() { return { minified: false }; }",
"var $mode = { getMode: getMode };",
" if ($mode.getMode()) {",
" console.log('hello world');",
" }",
"})()"));
test(
LINE_JOINER.join(
"(function() {",
"function getMode() { return { otherProp: true } }",
"var $mode = { getMode: getMode };",
" if ($mode.getMode().otherProp) {",
" console.log('hello world');",
" }",
"})()"),
LINE_JOINER.join(
"(function() {",
"function getMode() { return { otherProp: true }; }",
"var $mode = { getMode: getMode };",
" if ($mode.getMode().otherProp) {",
" console.log('hello world');",
" }",
"})()"));
}
}