Skip to content

Commit 84e1928

Browse files
rishipalcopybara-github
authored andcommitted
Run any transpilation pass only if the specific features worked on by that pass exist in the script
This CL completes go/avoid-unnecessary-transpile. PiperOrigin-RevId: 555691979
1 parent 7ee0b5e commit 84e1928

8 files changed

+57
-25
lines changed

src/com/google/javascript/jscomp/AstValidator.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.google.common.base.Preconditions.checkArgument;
2020
import static com.google.common.base.Preconditions.checkNotNull;
21+
import static com.google.common.base.Preconditions.checkState;
2122
import static com.google.common.base.Strings.isNullOrEmpty;
2223

2324
import com.google.common.collect.ImmutableSet;
@@ -307,6 +308,14 @@ public void validateStatement(Node n, boolean isAmbient) {
307308
violation("Expected statement but was " + n.getToken() + ".", n);
308309
return;
309310
default:
311+
if (n.isModuleBody() && compiler.getOptions().skipNonTranspilationPasses) {
312+
checkState(
313+
!compiler.getOptions().wrapGoogModulesForWhitespaceOnly,
314+
"Modules can exist in transpiler only if setWrapGoogModulesForWhitespaceOnly is"
315+
+ " false");
316+
validateModuleContents(n);
317+
return;
318+
}
310319
violation("Expected statement but was " + n.getToken() + ".", n);
311320
}
312321
}
@@ -2259,6 +2268,7 @@ private void validateMaximumChildCount(Node n, int i) {
22592268

22602269
private void validateFeature(Feature feature, Node n) {
22612270
FeatureSet allowbleFeatures = compiler.getAllowableFeatures();
2271+
// Checks that feature present in the AST is recorded in the compiler's featureSet.
22622272
if (!n.isFromExterns() && !allowbleFeatures.has(feature)) {
22632273
// Skip this check for externs because we don't need to complete transpilation on externs,
22642274
// and currently only transpile externs so that we can typecheck ES6+ features in externs.
@@ -2269,6 +2279,7 @@ private void validateFeature(Feature feature, Node n) {
22692279
return;
22702280
}
22712281
FeatureSet scriptFeatures = NodeUtil.getFeatureSetOfScript(currentScript);
2282+
// Checks that feature present in the AST is recorded in the SCRIPT node's featureSet.
22722283
if (scriptFeatures == null || !NodeUtil.getFeatureSetOfScript(currentScript).has(feature)) {
22732284
violation("SCRIPT node should be marked as containing feature " + feature, currentScript);
22742285
}

src/com/google/javascript/jscomp/Es6ConvertSuper.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ public final class Es6ConvertSuper extends NodeTraversal.AbstractPostOrderCallba
3535
private final AbstractCompiler compiler;
3636
private final AstFactory astFactory;
3737
private final SynthesizeExplicitConstructors constructorSynthesizer;
38-
private static final FeatureSet transpiledFeatures = FeatureSet.BARE_MINIMUM.with(Feature.SUPER);
38+
private static final FeatureSet FEATURES_TO_RUN_FOR =
39+
FeatureSet.BARE_MINIMUM.with(Feature.SUPER, Feature.CLASSES);
3940

4041
public Es6ConvertSuper(AbstractCompiler compiler) {
4142
this.compiler = compiler;
@@ -223,8 +224,8 @@ private void visitSuperPropertyAccess(Node node, Node parent, Node enclosingMemb
223224
@Override
224225
public void process(Node externs, Node root) {
225226
// Might need to synthesize constructors for ambient classes in .d.ts externs
226-
TranspilationPasses.processTranspile(compiler, externs, transpiledFeatures, this);
227-
TranspilationPasses.processTranspile(compiler, root, transpiledFeatures, this);
227+
TranspilationPasses.processTranspile(compiler, externs, FEATURES_TO_RUN_FOR, this);
228+
TranspilationPasses.processTranspile(compiler, root, FEATURES_TO_RUN_FOR, this);
229+
// TODO(rishipal): mark super feature as transpiled away
228230
}
229-
230231
}

src/com/google/javascript/jscomp/Es6RewriteDestructuring.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ public void process(Node externs, Node root) {
156156
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
157157
if (n.isScript()) {
158158
FeatureSet scriptFeatures = NodeUtil.getFeatureSetOfScript(n);
159+
// This will ensure that we run this only when features exist in the script
159160
return scriptFeatures.containsAtLeastOneOf(featuresToTriggerRunningPass);
160161
}
161162
switch (n.getToken()) {

src/com/google/javascript/jscomp/Es7RewriteExponentialOperator.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ public void process(Node externs, Node root) {
5959
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
6060
if (n.isScript()) {
6161
FeatureSet scriptFeatures = NodeUtil.getFeatureSetOfScript(n);
62+
// Runs only if features exist in the script
6263
return scriptFeatures == null || scriptFeatures.contains(Feature.EXPONENT_OP);
6364
}
6465
return true;

src/com/google/javascript/jscomp/RewriteLogicalAssignmentOperatorsPass.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public void process(Node externs, Node root) {
4444
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
4545
if (n.isScript()) {
4646
FeatureSet scriptFeatures = NodeUtil.getFeatureSetOfScript(n);
47+
// runs only if logical assignment present
4748
return scriptFeatures == null || scriptFeatures.contains(Feature.LOGICAL_ASSIGNMENT);
4849
}
4950
return true;

src/com/google/javascript/jscomp/RewriteNullishCoalesceOperator.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ private class ArrowRewriteCallBack implements NodeTraversal.Callback {
104104
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
105105
if (n.isScript()) {
106106
FeatureSet scriptFeatures = NodeUtil.getFeatureSetOfScript(n);
107+
// ensures we don't run unless script contains the feature
107108
return scriptFeatures == null || scriptFeatures.contains(Feature.NULL_COALESCE_OP);
108109
}
109110
return true;

src/com/google/javascript/jscomp/RewriteOptionalChainingOperator.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
7171
// Set the TmpVarNameCreator to be used when rewriting optional chains in this script.
7272
rewriterBuilder.setTmpVarNameCreator(getTmpVarNameCreatorForInput.apply(t.getInput()));
7373
FeatureSet scriptFeatures = NodeUtil.getFeatureSetOfScript(n);
74+
// ensures that the pass early exits if script does not contain the feature
7475
return scriptFeatures == null || scriptFeatures.contains(Feature.OPTIONAL_CHAINING);
7576
}
7677
return true;

src/com/google/javascript/jscomp/TranspilationPasses.java

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.javascript.jscomp;
1818

19+
import static com.google.common.base.Preconditions.checkState;
1920

2021
import com.google.javascript.jscomp.CompilerOptions.ChunkOutputType;
2122
import com.google.javascript.jscomp.Es6RewriteDestructuring.ObjectDestructuringRewriteMode;
@@ -62,7 +63,12 @@ public static void addEs6RewriteImportPathPass(PassListBuilder passes) {
6263
passes.maybeAdd(es6RelativizeImportPaths);
6364
}
6465

65-
/** Adds transpilation passes that should run at the beginning of the optimization phase */
66+
/**
67+
* Adds transpilation passes that should run at the beginning of the optimization phase. Passes
68+
* added in this method either use {@code TranspilationPasses.processTranspile} or early-exit by
69+
* checking their feature in the script's featureset. So they will only get run if the feature
70+
* they're responsible for removing exists in the script.
71+
*/
6672
public static void addEarlyOptimizationTranspilationPasses(
6773
PassListBuilder passes, CompilerOptions options) {
6874

@@ -224,14 +230,21 @@ public static void addEarlyOptimizationTranspilationPasses(
224230
}
225231
}
226232

227-
/** Adds transpilation passes that should not be run until after normalization has been done. */
233+
/**
234+
* Adds transpilation passes that should not be run until after normalization has been done.
235+
* Passes added in this method either use {@code TranspilationPasses.processTranspile} or
236+
* early-exit by checking their feature in the script's featureset. So they will only get run if
237+
* the feature they're responsible for removing exists in the script.
238+
*/
228239
public static void addPostNormalizationTranspilationPasses(
229240
PassListBuilder passes, CompilerOptions options) {
230241
// TODO(b/197349249): Move passes from `addEarlyOptimizationTranspilationPasses()` to here
231242
// until that method can be deleted as a no-op.
232243
if (options.needsTranspilationOf(Feature.GENERATORS)) {
233244
passes.maybeAdd(rewriteGenerators);
234245
}
246+
// This pass must run at the end of all transpiler passes. It validates that only supported
247+
// features remain in the compiler's featureSet
235248
passes.maybeAdd(
236249
TranspilationPasses.createPostTranspileUnsupportedFeaturesRemovedCheck(
237250
"postTranspileUnsupportedFeaturesRemovedCheck"));
@@ -441,44 +454,46 @@ static final PassFactory getEs6RewriteDestructuring(ObjectDestructuringRewriteMo
441454
.build();
442455

443456
/**
444-
* @param script The SCRIPT node representing a JS file
445-
* @return If the file has any features not in {@code supportedFeatures}
457+
* Returns true if the script's featureSet contains any feature from the given featureSet.
458+
*
459+
* <p>The script's featureSet gets accurately maintained during transpile. It can be relied upon
460+
* to check if a feature exists in the AST or not.
446461
*/
447-
static boolean doesScriptHaveUnsupportedFeatures(Node script, FeatureSet supportedFeatures) {
462+
static boolean doesScriptHaveAnyOfTheseFeatures(Node script, FeatureSet featureSet) {
448463
FeatureSet features = NodeUtil.getFeatureSetOfScript(script);
449-
return features != null && !supportedFeatures.contains(features);
464+
return features != null && features.containsAtLeastOneOf(featureSet);
450465
}
451466

452467
/**
453-
* Process transpilations if the input language needs transpilation from certain features, on any
454-
* JS file that has features not present in the compiler's output language mode.
468+
* Runs the given transpilation callbacks on every source script if the given {@code
469+
* featuresToRunFor} are unsupported in the output language and exist in that script.
455470
*
456471
* @param compiler An AbstractCompiler
457-
* @param combinedRoot The combined root for all JS files.
458-
* @param featureSet Ignored
459-
* @param callbacks The callbacks that should be invoked if a file has ES2015 features.
472+
* @param combinedRoot The combined root for all JS files
473+
* @param featuresToRunFor features for which these callbacks run
474+
* @param callbacks The callbacks that should be invoked if a file has output-unsupported
475+
* features.
460476
* @deprecated Please use a regular NodeTraversal object directly, using `shouldTraverse` to skip
461477
* SCRIPT node if desired.
462478
*/
463479
@Deprecated
464480
static void processTranspile(
465481
AbstractCompiler compiler,
466482
Node combinedRoot,
467-
FeatureSet featureSet,
483+
FeatureSet featuresToRunFor,
468484
NodeTraversal.Callback... callbacks) {
469485
FeatureSet languageOutFeatures = compiler.getOptions().getOutputFeatureSet();
486+
if (languageOutFeatures.contains(featuresToRunFor)) {
487+
// all featuresToRunFor are supported by languageOut and don't need to get transpiled.
488+
return;
489+
}
490+
470491
for (Node singleRoot = combinedRoot.getFirstChild();
471492
singleRoot != null;
472493
singleRoot = singleRoot.getNext()) {
473-
474-
// Only run the transpilation if this file has features not in the compiler's target output
475-
// language. For example, if this file is purely ES6 and the output language is ES6, don't
476-
// run any transpilation passes on it.
477-
// TODO(lharker): We could save time by being more selective about what files we transpile.
478-
// e.g. if a file has async functions but not `**`, don't run `**` transpilation on it.
479-
// Right now we know what features were in a file at parse time, but not what features were
480-
// added to that file by other transpilation passes.
481-
if (doesScriptHaveUnsupportedFeatures(singleRoot, languageOutFeatures)) {
494+
checkState(singleRoot.isScript());
495+
// only run the callbacks if any feature from the given featureSet exists in the script
496+
if (doesScriptHaveAnyOfTheseFeatures(singleRoot, featuresToRunFor)) {
482497
for (NodeTraversal.Callback callback : callbacks) {
483498
NodeTraversal.traverse(compiler, singleRoot, callback);
484499
}

0 commit comments

Comments
 (0)