Skip to content

Commit

Permalink
add custom closure compiler pass (#2593)
Browse files Browse the repository at this point in the history
* changes to third_party/closure-compiler
* add custom closure compiler pass
  • Loading branch information
erwinmombay committed Apr 22, 2016
1 parent 91c973c commit 656c3ec
Show file tree
Hide file tree
Showing 15 changed files with 929 additions and 45 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
.DS_Store
.g4ignore
build
build-system/build/classes
c
dist
dist.3p
Expand All @@ -12,3 +11,5 @@ node_modules
npm-debug.log
.idea
.tm_properties
.settings
build-system/runner/TESTS-TestSuites.xml
4 changes: 3 additions & 1 deletion build-system/runner/.classpath
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry including="**/*.java" kind="src" path="src"/>
<classpathentry including="**/*.java" kind="src" output="build/test-classes" path="test"/>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
<classpathentry kind="lib" path="lib/compiler.jar" sourcepath="/Users/erwinm/dev/java/closure-compiler"/>
<classpathentry kind="lib" path="./../../third_party/closure-compiler/compiler.jar"/>
<classpathentry kind="lib" path="./../../third_party/closure-compiler/compiler-and-tests.jar"/>
<classpathentry kind="output" path="build/classes"/>
</classpath>
90 changes: 86 additions & 4 deletions build-system/runner/build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@
<!--define folder properties-->
<property name="dir.buildfile" value="."/>
<property name="src.dir" value="${basedir}/src"/>
<property name="gen.dir" value="${basedir}/gen"/>
<property name="test.dir" value="${basedir}/test"/>
<property name="build.dir" value="${basedir}/build"/>
<property name="lib.dir" value="${basedir}/lib"/>
<property name="dist.dir" value="${basedir}/dist"/>
<property name="runner-jarfile" value="${build.dir}/${ant.project.name}.jar"/>
<property name="compiler.dir" value="${basedir}/../../third_party/closure-compiler"/>

<property name="classes.dir" value="${build.dir}/classes"/>
<property name="testClasses.dir" value="${build.dir}/test"/>
Expand All @@ -33,7 +34,7 @@
<jar destfile="${runner-jarfile}">
<zipfileset src="${lib.dir}/jar-in-jar-loader.zip"/>
<fileset dir="${classes.dir}"/>
<fileset dir="${lib.dir}">
<fileset dir="${compiler.dir}">
<include name="compiler.jar"/>
</fileset>
<manifest>
Expand All @@ -43,11 +44,12 @@
<attribute name="Rsrc-Class-Path" value="./ compiler.jar"/>
</manifest>
</jar>
<mkdir dir="${dist.dir}"/>
<copy file="${build.dir}/runner.jar" todir="${dist.dir}"/>
</target>

<target name="compile">
<mkdir dir="${classes.dir}"/>
<copy file="${src.dir}/../../../third_party/closure-compiler/compiler.jar" todir="${lib.dir}"/>
<javac srcdir="${src.dir}"
destdir="${classes.dir}"
excludes=".git,.DS_Store"
Expand All @@ -56,20 +58,100 @@
<classpath refid="srcclasspath.path"/>
<compilerarg value="-Xlint:unchecked"/>
</javac>
<!-- This copy is needed for tests -->
<copy file="${compiler.dir}/Messages.properties"
todir="${classes.dir}/com/google/javascript/rhino/"/>
<!-- This copy is needed for tests -->
<copy file="${compiler.dir}/ParserConfig.properties"
todir="${classes.dir}/com/google/javascript/jscomp/parsing"/>
</target>

<target name="clean" description="delete generated files">
<delete dir="${build.dir}"/>
</target>

<target name="test" depends="compile-tests">
<mkdir dir="build/testoutput"/>
<junit printsummary="on" fork="${test.fork}"
forkmode="once" showoutput="true"
dir="${basedir}"
failureproperty="junit.failure">
<classpath refid="allclasspath.path" />
<classpath>
<pathelement location="${build.dir}/test"/>
</classpath>
<batchtest todir="build/testoutput">
<formatter type="brief" usefile="false"/>
<formatter type="xml"/>
<fileset dir="${build.dir}/test">
<include name="**/${test.class}.class"/>
</fileset>
</batchtest>
</junit>
<junitreport>
<fileset dir="build/testoutput" includes="*.xml"/>
<report todir="build/testoutput"/>
</junitreport>
<fail if="junit.failure" message="Unit tests failed. See build/testoutput/index.html"/>
</target>

<target name="compile-tests" depends="compile">
<mkdir dir="${testClasses.dir}"/>
<javac srcdir="${src.dir}"
destdir="${testClasses.dir}"
excludes=".git,**/debugger/**,.DS_Store"
debug="on"
deprecation="on"
includeantruntime="false"
encoding="utf-8">
<classpath refid="allclasspath.path"/>
<compilerarg value="-Xlint:unchecked"/>
</javac>
<javac srcdir="${test.dir}"
destdir="${testClasses.dir}"
excludes=".git,.DS_Store"
debug="on"
deprecation="on"
includeantruntime="false"
encoding="utf-8">
<classpath refid="allclasspath.path"/>
</javac>
</target>

<target name="one-test" depends="compile-tests">
<mkdir dir="build/testoutput"/>
<junit printsummary="on" fork="${test.fork}"
forkmode="once" showoutput="true"
dir="${basedir}"
failureproperty="junit.failure">
<classpath refid="allclasspath.path"/>
<classpath>
<pathelement location="${build.dir}/test"/>
</classpath>
<test todir="build/testoutput" name="${test.class}" methods="${test.method}">
<formatter type="brief" usefile="false"/>
<formatter type="xml"/>
</test>
</junit>
<junitreport>
<fileset dir="build/testoutput" includes="*.xml"/>
<report todir="build/testoutput"/>
</junitreport>
<fail if="junit.failure"
message="Unit tests failed. See build/testoutput/index.html"/>
</target>

<path id="srcclasspath.path">
<pathelement location="${classes.dir}"/>
<fileset dir="${lib.dir}">
<fileset dir="${compiler.dir}">
<include name="compiler.jar"/>
</fileset>
</path>

<path id="allclasspath.path">
<pathelement location="${classes.dir}"/>
<fileset dir="${compiler.dir}">
<include name="compiler-and-tests.jar"/>
</fileset>
</path>
</project>
Binary file not shown.
13 changes: 13 additions & 0 deletions build-system/runner/runner.iml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
<module type="JAVA_MODULE" version="4">
<component name="NewModuleRootManager" inherit-compiler-output="true">
<exclude-output />
<content url="file://$MODULE_DIR$">
<sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
<sourceFolder url="file://$MODULE_DIR$/test" isTestSource="true" />
</content>
<orderEntry type="inheritedJdk" />
<orderEntry type="sourceFolder" forTests="false" />
</component>
</module>

19 changes: 17 additions & 2 deletions build-system/runner/src/org/ampproject/AmpCommandLineRunner.java
Original file line number Diff line number Diff line change
@@ -1,21 +1,36 @@
package org.ampproject;


import com.google.common.collect.ImmutableSet;
import com.google.javascript.jscomp.CommandLineRunner;
import com.google.javascript.jscomp.CompilerOptions;
import com.google.javascript.jscomp.CustomPassExecutionTime;


/**
* Adds a way to add custom options and custom compiler passes.
* Adds a custom pass for Tree shaking `dev.fine` and `dev.assert` calls.
*/
public class AmpCommandLineRunner extends CommandLineRunner {

/**
* List of string suffixes to eliminate from the AST.
*/
ImmutableSet<String> suffixTypes = ImmutableSet.of(
"dev.fine");

protected AmpCommandLineRunner(String[] args) {
super(args);
}

@Override protected CompilerOptions createOptions() {
CompilerOptions options = super.createOptions();
AmpPass ampPass = new AmpPass(getCompiler(), suffixTypes);
options.addCustomPass(CustomPassExecutionTime.BEFORE_OPTIMIZATIONS, ampPass);
return options;
}

public static void main(String[] args) {
AmpCommandLineRunner runner = new AmpCommandLineRunner(args);

if (runner.shouldRunCompiler()) {
runner.run();
}
Expand Down
132 changes: 132 additions & 0 deletions build-system/runner/src/org/ampproject/AmpPass.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package org.ampproject;

import java.util.Set;

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.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
*/
class AmpPass extends AbstractPostOrderCallback implements HotSwapCompilerPass {

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

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

@Override public void process(Node externs, Node root) {
hotSwapScript(root, null);
}

@Override public void hotSwapScript(Node scriptRoot, Node originalRoot) {
NodeTraversal.traverseEs6(compiler, scriptRoot, this);
}

@Override public void visit(NodeTraversal t, Node n, Node parent) {
if (isDevAssertCall(n)) {
maybeEliminateCallExceptFirstParam(n, parent);
} else if (n.isExprResult()) {
maybeEliminateExpressionBySuffixName(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;
}

if (name.endsWith("dev.assert")) {
return true;
}
}
return false;
}

/**
* Checks if expression is a GETPROP() (method invocation) and the property
* name ends with one of the items in stripTypeSuffixes.
*/
private void maybeEliminateExpressionBySuffixName(Node n, Node parent) {
// n = EXPRESSION_RESULT > CALL > GETPROP
Node call = n.getFirstChild();
if (call == null) {
return;
}
Node expression = call.getFirstChild();
if (expression == null) {
return;
}

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

private void maybeEliminateCallExceptFirstParam(Node n, Node p) {
Node functionName = n.getFirstChild();
if (functionName == null) {
return;
}
Node firstArg = functionName.getNext();
if (firstArg == null) {
return;
}
firstArg.detachFromParent();
p.replaceChild(n, firstArg);
compiler.reportCodeChange();
}

/**
* Checks the nodes qualified name if it ends with one of the items in
* stripTypeSuffixes
*/
boolean qualifiedNameEndsWithStripType(Node n) {
String name = n.getQualifiedName();
return qualifiedNameEndsWithStripType(name);
}

/**
* Checks if the string ends with one of the items in stripTypeSuffixes
*/
boolean qualifiedNameEndsWithStripType(String name) {
if (name != null) {
for (String suffix : stripTypeSuffixes) {
if (name.endsWith(suffix)) {
return true;
}
}
}
return false;
}
}
Loading

0 comments on commit 656c3ec

Please sign in to comment.