From 30beedc1f06068ea61bab1696bcaa7acb02f22ec Mon Sep 17 00:00:00 2001 From: tbreisacher Date: Thu, 25 May 2017 14:26:45 -0700 Subject: [PATCH] Internally, keep track of the FeatureSet (i.e. which features are present in the tree) rather than the LanguageMode. This allows us to indicate, for instance, "The tree currently contains no ES6 modules but does contain other ES6 features" ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=157153453 --- .../javascript/jscomp/AbstractCompiler.java | 10 +-- .../javascript/jscomp/AstValidator.java | 65 ++++++------------- .../google/javascript/jscomp/Compiler.java | 54 +++++++-------- .../javascript/jscomp/CompilerOptions.java | 23 +++++++ .../javascript/jscomp/DefaultPassConfig.java | 17 +++-- .../jscomp/Es6TypedToEs6Converter.java | 3 + .../jscomp/parsing/parser/FeatureSet.java | 14 ++++ .../javascript/jscomp/CompilerTestCase.java | 2 +- 8 files changed, 99 insertions(+), 89 deletions(-) diff --git a/src/com/google/javascript/jscomp/AbstractCompiler.java b/src/com/google/javascript/jscomp/AbstractCompiler.java index 2d59d63088c..88843a96255 100644 --- a/src/com/google/javascript/jscomp/AbstractCompiler.java +++ b/src/com/google/javascript/jscomp/AbstractCompiler.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableMap; import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.jscomp.parsing.Config; +import com.google.javascript.jscomp.parsing.parser.FeatureSet; import com.google.javascript.jscomp.parsing.parser.trees.Comment; import com.google.javascript.jscomp.type.ReverseAbstractInterpreter; import com.google.javascript.rhino.ErrorReporter; @@ -396,14 +397,9 @@ boolean isNormalizedObfuscated() { abstract CompilerOptions getOptions(); - /** - * The language mode of the current root node. This will match the languageIn - * field of the {@link CompilerOptions} before transpilation happens, and - * match the languageOut field after transpilation. - */ - abstract CompilerOptions.LanguageMode getLanguageMode(); + abstract FeatureSet getFeatureSet(); - abstract void setLanguageMode(CompilerOptions.LanguageMode mode); + abstract void setFeatureSet(FeatureSet fs); // TODO(bashir) It would be good to extract a single dumb data object with // only getters and setters that keeps all global information we keep for a diff --git a/src/com/google/javascript/jscomp/AstValidator.java b/src/com/google/javascript/jscomp/AstValidator.java index c12274088b4..86ff26f0b18 100644 --- a/src/com/google/javascript/jscomp/AstValidator.java +++ b/src/com/google/javascript/jscomp/AstValidator.java @@ -18,7 +18,7 @@ import static com.google.common.base.Strings.isNullOrEmpty; -import com.google.javascript.jscomp.CompilerOptions.LanguageMode; +import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature; import com.google.javascript.rhino.InputId; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; @@ -357,7 +357,7 @@ public void validateExpression(Node n) { } private void validateYield(Node n) { - validateEs6Feature("yield", n); + validateFeature(Feature.GENERATORS, n); validateNodeType(Token.YIELD, n); validateChildCountIn(n, 0, 1); if (n.hasChildren()) { @@ -366,7 +366,7 @@ private void validateYield(Node n) { } private void validateAwait(Node n) { - validateEs6Feature("async function", n); + validateFeature(Feature.ASYNC_FUNCTIONS, n); validateNodeType(Token.AWAIT, n); validateWithinAsyncFunction(n); } @@ -379,7 +379,7 @@ private void validateWithinAsyncFunction(Node n) { } private void validateImport(Node n) { - validateEs6Feature("import statement", n); + validateFeature(Feature.MODULES, n); validateNodeType(Token.IMPORT, n); validateChildCount(n); @@ -457,7 +457,7 @@ private void validateExportSpecifier(Node n) { } private void validateTaggedTemplateLit(Node n) { - validateEs6Feature("template literal", n); + validateFeature(Feature.TEMPLATE_LITERALS, n); validateNodeType(Token.TAGGED_TEMPLATELIT, n); validateChildCount(n); validateExpression(n.getFirstChild()); @@ -465,7 +465,7 @@ private void validateTaggedTemplateLit(Node n) { } private void validateTemplateLit(Node n) { - validateEs6Feature("template literal", n); + validateFeature(Feature.TEMPLATE_LITERALS, n); validateNodeType(Token.TEMPLATELIT, n); if (!n.hasChildren()) { return; @@ -490,7 +490,7 @@ private void validateTemplateLitSub(Node n) { } private void validateInterface(Node n) { - validateEs6TypedFeature("interface", n); + validateFeature(Feature.INTERFACE, n); validateNodeType(Token.INTERFACE, n); validateChildCount(n); Node name = n.getFirstChild(); @@ -566,7 +566,7 @@ private void validateClass(Node n) { } private void validateClassHelper(Node n, boolean isAmbient) { - validateEs6Feature("classes", n); + validateFeature(Feature.CLASSES, n); validateNodeType(Token.CLASS, n); validateChildCount(n); @@ -724,7 +724,7 @@ private void validateFunctionExpressionHelper(Node n, boolean isAmbient) { Node name = n.getFirstChild(); Node body = n.getLastChild(); if (n.isArrowFunction()) { - validateEs6Feature("arrow functions", n); + validateFeature(Feature.ARROW_FUNCTIONS, n); validateEmptyName(name); if (body.isNormalBlock()) { validateBlock(body); @@ -747,21 +747,6 @@ private void validateFunctionBody(Node n, boolean noBlock) { private void validateParameters(Node n) { validateNodeType(Token.PARAM_LIST, n); - - if (isEs6OrHigher()) { - validateParametersEs6(n); - } else { - validateParametersEs5(n); - } - } - - private void validateParametersEs5(Node n) { - for (Node c = n.getFirstChild(); c != null; c = c.getNext()) { - validateName(c); - } - } - - private void validateParametersEs6(Node n) { for (Node c = n.getFirstChild(); c != null; c = c.getNext()) { if (c.isRest()) { if (c.getNext() != null) { @@ -783,6 +768,7 @@ private void validateParametersEs6(Node n) { } private void validateDefaultValue(Token type, Node n) { + validateFeature(Feature.DEFAULT_PARAMETERS, n); validateAssignmentExpression(n); Node lhs = n.getFirstChild(); @@ -810,6 +796,7 @@ private void validateCall(Node n) { * @param n */ private void validateRest(Token contextType, Node n) { + validateFeature(Feature.REST_PARAMETERS, n); validateNodeType(Token.REST, n); validateChildCount(n); validateLHS(contextType, n.getFirstChild()); @@ -886,6 +873,7 @@ private void validateLHS(Token contextType, Node n) { } private void validateArrayPattern(Token type, Node n) { + validateFeature(Feature.DESTRUCTURING, n); validateNodeType(Token.ARRAY_PATTERN, n); for (Node c = n.getFirstChild(); c != null; c = c.getNext()) { // When the array pattern is a direct child of a var/let/const node, @@ -905,6 +893,7 @@ private void validateArrayPattern(Token type, Node n) { } private void validateObjectPattern(Token type, Node n) { + validateFeature(Feature.DESTRUCTURING, n); validateNodeType(Token.OBJECT_PATTERN, n); for (Node c = n.getFirstChild(); c != null; c = c.getNext()) { // When the object pattern is a direct child of a var/let/const node, @@ -1253,11 +1242,7 @@ private void validateObjectLitStringKey(Node n) { validateNodeType(Token.STRING_KEY, n); validateObjectLiteralKeyName(n); - if (isEs6OrHigher()) { - validateChildCountIn(n, 0, 1); - } else { - validateChildCount(n, 1); - } + validateChildCountIn(n, 0, 1); if (n.hasOneChild()) { validateExpression(n.getFirstChild()); @@ -1342,13 +1327,13 @@ private void validateNamedType(Node n) { } private void validateTypeAlias(Node n) { - validateEs6TypedFeature("type alias", n); + validateFeature(Feature.TYPE_ALIAS, n); validateNodeType(Token.TYPE_ALIAS, n); validateChildCount(n); } private void validateAmbientDeclaration(Node n) { - validateEs6TypedFeature("ambient declaration", n); + validateFeature(Feature.AMBIENT_DECLARATION, n); validateNodeType(Token.DECLARE, n); validateAmbientDeclarationHelper(n.getFirstChild()); } @@ -1384,7 +1369,7 @@ private void validateAmbientDeclarationHelper(Node n) { } private void validateNamespace(Node n, boolean isAmbient) { - validateEs6TypedFeature("namespace", n); + validateFeature(Feature.NAMESPACE_DECLARATION, n); validateNodeType(Token.NAMESPACE, n); validateChildCount(n); validateNamespaceName(n.getFirstChild()); @@ -1476,19 +1461,9 @@ private void validateMaximumChildCount(Node n, int i) { } } - private void validateEs6Feature(String feature, Node n) { - if (!isEs6OrHigher()) { - violation("Feature '" + feature + "' is only allowed in ES6 mode.", n); - } - } - - private boolean isEs6OrHigher() { - return compiler.getLanguageMode().isEs6OrHigher(); - } - - private void validateEs6TypedFeature(String feature, Node n) { - if (!compiler.getLanguageMode().equals(LanguageMode.ECMASCRIPT6_TYPED)) { - violation("Feature '" + feature + "' is only allowed in ES6 Typed mode.", n); + private void validateFeature(Feature feature, Node n) { + if (!compiler.getFeatureSet().contains(feature)) { + violation("AST should not contain " + feature, n); } } } diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index b470bb797d3..db9fccd5785 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -37,6 +37,7 @@ import com.google.javascript.jscomp.deps.SortedDependencies.MissingProvideException; import com.google.javascript.jscomp.parsing.Config; import com.google.javascript.jscomp.parsing.ParserRunner; +import com.google.javascript.jscomp.parsing.parser.FeatureSet; import com.google.javascript.jscomp.parsing.parser.trees.Comment; import com.google.javascript.jscomp.type.ChainableReverseAbstractInterpreter; import com.google.javascript.jscomp.type.ClosureReverseAbstractInterpreter; @@ -159,9 +160,7 @@ public class Compiler extends AbstractCompiler implements ErrorHandler, SourceFi // Used for debugging; to see the compiled code between passes private String lastJsSource = null; - /** @see #getLanguageMode() */ - private CompilerOptions.LanguageMode languageMode = - CompilerOptions.LanguageMode.ECMASCRIPT3; + private FeatureSet featureSet; private final Map inputsById = new ConcurrentHashMap<>(); @@ -345,7 +344,7 @@ void setOriginalSourcesLoader(ExternalSourceLoader originalSourcesLoader) { */ public void initOptions(CompilerOptions options) { this.options = options; - this.languageMode = options.getLanguageIn(); + this.setFeatureSet(options.getLanguageIn().toFeatureSet()); if (errorManager == null) { if (this.outStream == null) { setErrorManager( @@ -468,8 +467,7 @@ protected void reconcileOptionsWithGuards() { CheckLevel.WARNING); } - if (options.checkGlobalThisLevel.isOn() && - !options.disables(DiagnosticGroups.GLOBAL_THIS)) { + if (options.checkGlobalThisLevel.isOn() && !options.disables(DiagnosticGroups.GLOBAL_THIS)) { options.setWarningLevel( DiagnosticGroups.GLOBAL_THIS, options.checkGlobalThisLevel); @@ -486,10 +484,8 @@ protected void reconcileOptionsWithGuards() { // checks the externs file for validity. If you don't want to warn // about missing variable declarations, we shut that specific // error off. - if (!options.checkSymbols && - !options.enables(DiagnosticGroups.CHECK_VARIABLES)) { - options.setWarningLevel( - DiagnosticGroups.CHECK_VARIABLES, CheckLevel.OFF); + if (!options.checkSymbols && !options.enables(DiagnosticGroups.CHECK_VARIABLES)) { + options.setWarningLevel(DiagnosticGroups.CHECK_VARIABLES, CheckLevel.OFF); } } @@ -1312,13 +1308,13 @@ public Node getRoot() { } @Override - CompilerOptions.LanguageMode getLanguageMode() { - return languageMode; + FeatureSet getFeatureSet() { + return featureSet; } @Override - void setLanguageMode(CompilerOptions.LanguageMode mode) { - languageMode = mode; + void setFeatureSet(FeatureSet fs) { + featureSet = fs; } /** @@ -1349,8 +1345,7 @@ public String get() { @Override boolean areNodesEqualForInlining(Node n1, Node n2) { - if (options.shouldAmbiguateProperties() || - options.shouldDisambiguateProperties()) { + if (options.shouldAmbiguateProperties() || options.shouldDisambiguateProperties()) { // The type based optimizations require that type information is preserved // during other optimizations. return n1.isEquivalentToTyped(n2); @@ -1863,10 +1858,10 @@ Node parseInputs() { // TODO(johnlenz): we shouldn't need to check both isExternExportsEnabled and // externExportsPath. - if (options.sourceMapOutputPath != null || - options.isExternExportsEnabled() || - options.externExportsPath != null || - !options.replaceStringsFunctionDescriptions.isEmpty()) { + if (options.sourceMapOutputPath != null + || options.isExternExportsEnabled() + || options.externExportsPath != null + || !options.replaceStringsFunctionDescriptions.isEmpty()) { // Annotate the nodes in the tree with information from the // input file. This information is used to construct the SourceMap. @@ -2092,6 +2087,8 @@ void processEs6Modules(List inputsToProcess, boolean forceRewrite } new ProcessEs6Modules(this).processFile(root, forceRewrite); } + + setFeatureSet(featureSet.withoutModules()); } /** @@ -3103,7 +3100,7 @@ public void addNewScript(JsAst ast) { } private void processNewScript(JsAst ast, Node originalRoot) { - languageMode = options.getLanguageIn(); + setFeatureSet(options.getLanguageIn().toFeatureSet()); Node js = ast.getAstRoot(this); checkNotNull(js); @@ -3287,6 +3284,7 @@ private static class CompilerState implements Serializable { private final Node externAndJsRoot; private final Node externsRoot; private final Node jsRoot; + private final FeatureSet featureSet; private final List externs; private final List inputs; private final Map inputsById; @@ -3309,12 +3307,13 @@ private static class CompilerState implements Serializable { CompilerState(Compiler compiler) { this.externsRoot = checkNotNull(compiler.externsRoot); - this.jsRoot = checkNotNull(compiler.jsRoot); - this.externAndJsRoot = checkNotNull(compiler.externAndJsRoot); - this.typeRegistry = compiler.typeRegistry; - this.externs = compiler.externs; - this.inputs = checkNotNull(compiler.inputs); - this.inputsById = checkNotNull(compiler.inputsById); + this.jsRoot = checkNotNull(compiler.jsRoot); + this.externAndJsRoot = checkNotNull(compiler.externAndJsRoot); + this.featureSet = checkNotNull(compiler.featureSet); + this.typeRegistry = compiler.typeRegistry; + this.externs = compiler.externs; + this.inputs = checkNotNull(compiler.inputs); + this.inputsById = checkNotNull(compiler.inputsById); this.mostRecentTypeChecker = compiler.mostRecentTypechecker; this.synthesizedExternsInput = compiler.synthesizedExternsInput; this.synthesizedExternsInputAtEnd = compiler.synthesizedExternsInputAtEnd; @@ -3360,6 +3359,7 @@ public CompilerState call() throws Exception { return compilerState; } }); + featureSet = compilerState.featureSet; externs = compilerState.externs; inputs = compilerState.inputs; inputsById.clear(); diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java index afa9a88ad72..6603b94b974 100644 --- a/src/com/google/javascript/jscomp/CompilerOptions.java +++ b/src/com/google/javascript/jscomp/CompilerOptions.java @@ -31,6 +31,7 @@ import com.google.common.primitives.Chars; import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.jscomp.parsing.Config; +import com.google.javascript.jscomp.parsing.parser.FeatureSet; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.SourcePosition; @@ -3068,6 +3069,28 @@ public static LanguageMode fromString(String value) { return null; // unknown name. } } + + FeatureSet toFeatureSet() { + switch (this) { + case ECMASCRIPT3: + return FeatureSet.ES3; + case ECMASCRIPT5: + case ECMASCRIPT5_STRICT: + return FeatureSet.ES5; + case ECMASCRIPT_2015: + return FeatureSet.ES6_MODULES; + case ECMASCRIPT_2016: + return FeatureSet.ES7_MODULES; + case ECMASCRIPT_2017: + case ECMASCRIPT_NEXT: + return FeatureSet.ES8_MODULES; + case ECMASCRIPT6_TYPED: + return FeatureSet.TYPESCRIPT; + case NO_TRANSPILE: + throw new IllegalStateException(); + } + throw new IllegalStateException(); + } } /** When to do the extra sanity checks */ diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index 335aee8b0bd..dfa799f464e 100644 --- a/src/com/google/javascript/jscomp/DefaultPassConfig.java +++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java @@ -17,7 +17,6 @@ package com.google.javascript.jscomp; import static com.google.javascript.jscomp.CompilerOptions.LanguageMode.ECMASCRIPT6_TYPED; -import static com.google.javascript.jscomp.CompilerOptions.LanguageMode.ECMASCRIPT_2016; import static com.google.javascript.jscomp.PassFactory.createEmptyPass; import com.google.common.annotations.VisibleForTesting; @@ -27,7 +26,6 @@ import com.google.javascript.jscomp.AbstractCompiler.LifeCycleStage; import com.google.javascript.jscomp.AbstractCompiler.MostRecentTypechecker; import com.google.javascript.jscomp.CompilerOptions.ExtractPrototypeMemberDeclarationsMode; -import com.google.javascript.jscomp.CompilerOptions.LanguageMode; import com.google.javascript.jscomp.CoverageInstrumentationPass.CoverageReach; import com.google.javascript.jscomp.CoverageInstrumentationPass.InstrumentOption; import com.google.javascript.jscomp.ExtractPrototypeMemberDeclarations.Pattern; @@ -47,6 +45,7 @@ import com.google.javascript.jscomp.lint.CheckUnusedLabels; import com.google.javascript.jscomp.lint.CheckUselessBlocks; import com.google.javascript.jscomp.parsing.ParserRunner; +import com.google.javascript.jscomp.parsing.parser.FeatureSet; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; import java.util.ArrayList; @@ -205,7 +204,7 @@ protected List getTranspileOnlyPasses() { if (options.getLanguageIn().isEs2017OrHigher() && !options.getLanguageOut().isEs2017OrHigher()) { TranspilationPasses.addEs2017Passes(passes); - passes.add(setLanguageMode(ECMASCRIPT_2016)); + passes.add(setFeatureSet(FeatureSet.ES7)); } if (options.getLanguageIn().isEs6OrHigher() && !options.skipTranspilationAndCrash) { @@ -215,7 +214,7 @@ protected List getTranspileOnlyPasses() { if (options.rewritePolyfills) { TranspilationPasses.addRewritePolyfillPass(passes); } - passes.add(setLanguageMode(options.getLanguageOut())); + passes.add(setFeatureSet(options.getLanguageOut().toFeatureSet())); } if (options.raiseToEs6Typed()) { @@ -384,7 +383,7 @@ protected List getChecks() { if (options.getLanguageIn().isEs2017OrHigher() && !options.getLanguageOut().isEs2017OrHigher()) { TranspilationPasses.addEs2017Passes(checks); - checks.add(setLanguageMode(ECMASCRIPT_2016)); + checks.add(setFeatureSet(FeatureSet.ES7)); } if (options.getLanguageIn().isEs6OrHigher() && !options.skipTranspilationAndCrash) { @@ -399,7 +398,7 @@ protected List getChecks() { } // TODO(bradfordcsmith): This marking is really about how variable scoping is handled during // type checking. It should really be handled in a more direct fashion. - checks.add(setLanguageMode(options.getLanguageOut())); + checks.add(setFeatureSet(options.getLanguageOut().toFeatureSet())); } if (options.raiseToEs6Typed()) { @@ -1422,14 +1421,14 @@ protected CompilerPass create(AbstractCompiler compiler) { } }; - private final PassFactory setLanguageMode(final LanguageMode mode) { - return new PassFactory("setLanguageMode:" + mode, true) { + private final PassFactory setFeatureSet(final FeatureSet featureSet) { + return new PassFactory("setFeatureSet:" + featureSet.toLanguageModeString(), true) { @Override protected CompilerPass create(final AbstractCompiler compiler) { return new CompilerPass() { @Override public void process(Node externs, Node root) { - compiler.setLanguageMode(mode); + compiler.setFeatureSet(featureSet); } }; } diff --git a/src/com/google/javascript/jscomp/Es6TypedToEs6Converter.java b/src/com/google/javascript/jscomp/Es6TypedToEs6Converter.java index e47f4df3956..f9ae06dab24 100644 --- a/src/com/google/javascript/jscomp/Es6TypedToEs6Converter.java +++ b/src/com/google/javascript/jscomp/Es6TypedToEs6Converter.java @@ -111,6 +111,9 @@ public void process(Node externs, Node scriptRoot) { NodeTraversal.traverseEs6(compiler, scriptRoot, scanner); NodeTraversal.traverseEs6(compiler, externs, this); NodeTraversal.traverseEs6(compiler, scriptRoot, this); + if (!compiler.hasHaltingErrors()) { + compiler.setFeatureSet(compiler.getFeatureSet().withoutTypes()); + } } @Override diff --git a/src/com/google/javascript/jscomp/parsing/parser/FeatureSet.java b/src/com/google/javascript/jscomp/parsing/parser/FeatureSet.java index 08512d30f72..c590a4487b4 100644 --- a/src/com/google/javascript/jscomp/parsing/parser/FeatureSet.java +++ b/src/com/google/javascript/jscomp/parsing/parser/FeatureSet.java @@ -179,6 +179,20 @@ public FeatureSet require(FeatureSet other) { return this.contains(other) ? this : this.union(other); } + public FeatureSet withoutModules() { + if (!es6Modules) { + return this; + } + return new FeatureSet(number, supported, false, typeScript); + } + + public FeatureSet withoutTypes() { + if (!typeScript) { + return this; + } + return new FeatureSet(number, supported, es6Modules, false); + } + /** * Returns a new {@link FeatureSet} including all features of both {@code this} and {@code other}. */ diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java index c247a5423bb..8d2a491d389 100644 --- a/test/com/google/javascript/jscomp/CompilerTestCase.java +++ b/test/com/google/javascript/jscomp/CompilerTestCase.java @@ -1947,7 +1947,7 @@ static JSModule[] createModules( Compiler createCompiler() { Compiler compiler = new Compiler(); - compiler.setLanguageMode(acceptedLanguage); + compiler.setFeatureSet(acceptedLanguage.toFeatureSet()); return compiler; }