diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index 75ac9483cd9..79f05f9849c 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -1722,13 +1722,16 @@ Node parseInputs() { externsRoot.addChildToBack(n); } + if (options.transformAMDToCJSModules) { + processAMDModules(); + } + if (options.getLanguageIn().toFeatureSet().has(FeatureSet.Feature.MODULES) - || options.transformAMDToCJSModules || options.processCommonJSModules) { this.moduleLoader = new ModuleLoader( - this, + null, options.moduleRoots, inputs, ModuleLoader.PathResolver.RELATIVE, @@ -1740,7 +1743,7 @@ Node parseInputs() { // so we redefine it afterwards with the package.json inputs this.moduleLoader = new ModuleLoader( - this, + null, options.moduleRoots, inputs, ModuleLoader.PathResolver.RELATIVE, @@ -1755,20 +1758,13 @@ Node parseInputs() { if (options.getDependencyOptions().needsManagement()) { findDependenciesFromEntryPoints( options.getLanguageIn().toFeatureSet().has(Feature.MODULES), - options.processCommonJSModules, - options.transformAMDToCJSModules); - } else if (options.needsTranspilationOf(Feature.MODULES) - || options.transformAMDToCJSModules + options.processCommonJSModules); + } else if (options.needsTranspilationFrom(FeatureSet.ES6_MODULES) || options.processCommonJSModules) { if (options.getLanguageIn().toFeatureSet().has(Feature.MODULES)) { parsePotentialModules(inputs); } - // Modules inferred in ProcessCommonJS pass. - if (options.transformAMDToCJSModules || options.processCommonJSModules) { - processAMDAndCommonJSModules(); - } - // Build a map of module identifiers for any input which provides no namespace. // These files could be imported modules which have no exports, but do have side effects. Map inputModuleIdentifiers = new HashMap<>(); @@ -1793,13 +1789,14 @@ Node parseInputs() { } for (CompilerInput input : inputsToRewrite.values()) { - forceInputToPathBasedModule( - input, - options.getLanguageIn().toFeatureSet().has(Feature.MODULES), - options.processCommonJSModules); + input.setJsModuleType(CompilerInput.ModuleType.IMPORTED_SCRIPT); } } + if (this.moduleLoader != null) { + this.moduleLoader.setErrorHandler(this); + } + orderInputs(); // If in IDE mode, we ignore the error and keep going. @@ -1915,8 +1912,7 @@ void orderInputs() { *

If the dependency mode is set to LOOSE, inputs for which the deps package did not find a * provide statement or detect as a module will be treated as entry points. */ - void findDependenciesFromEntryPoints( - boolean supportEs6Modules, boolean supportCommonJSModules, boolean supportAmdModules) { + void findDependenciesFromEntryPoints(boolean supportEs6Modules, boolean supportCommonJSModules) { hoistExterns(); List entryPoints = new ArrayList<>(); Map inputsByProvide = new HashMap<>(); @@ -1951,16 +1947,7 @@ void findDependenciesFromEntryPoints( inputsByIdentifier, inputsByProvide, supportEs6Modules, - supportCommonJSModules, - supportAmdModules)); - } - - // TODO(ChadKillingsworth) Move this into the standard compilation passes - if (supportCommonJSModules) { - for (CompilerInput input : orderedInputs) { - new ProcessCommonJSModules(this) - .process(/* externs */ null, input.getAstRoot(this), /* forceModuleDetection */ false); - } + supportCommonJSModules)); } } @@ -1972,24 +1959,19 @@ private List depthFirstDependenciesFromInput( Map inputsByIdentifier, Map inputsByProvide, boolean supportEs6Modules, - boolean supportCommonJSModules, - boolean supportAmdModules) { + boolean supportCommonJSModules) { List orderedInputs = new ArrayList<>(); if (!inputs.remove(input)) { // It's possible for a module to be included as both a script // and a module in the same compilation. In these cases, it should // be forced to be a module. if (wasImportedByModule && input.getJsModuleType() == CompilerInput.ModuleType.NONE) { - forceInputToPathBasedModule(input, supportEs6Modules, supportCommonJSModules); + input.setJsModuleType(CompilerInput.ModuleType.IMPORTED_SCRIPT); } return orderedInputs; } - if (supportAmdModules) { - new TransformAMDToCJSModule(this).process(null, input.getAstRoot(this)); - } - FindModuleDependencies findDeps = new FindModuleDependencies(this, supportEs6Modules, supportCommonJSModules); findDeps.process(input.getAstRoot(this)); @@ -1997,7 +1979,7 @@ private List depthFirstDependenciesFromInput( // If this input was imported by another module, it is itself a module // so we force it to be detected as such. if (wasImportedByModule && input.getJsModuleType() == CompilerInput.ModuleType.NONE) { - forceInputToPathBasedModule(input, supportEs6Modules, supportCommonJSModules); + input.setJsModuleType(CompilerInput.ModuleType.IMPORTED_SCRIPT); } for (String requiredNamespace : input.getRequires()) { @@ -2019,28 +2001,13 @@ private List depthFirstDependenciesFromInput( inputsByIdentifier, inputsByProvide, supportEs6Modules, - supportCommonJSModules, - supportAmdModules)); + supportCommonJSModules)); } } orderedInputs.add(input); return orderedInputs; } - private void forceInputToPathBasedModule( - CompilerInput input, boolean supportEs6Modules, boolean supportCommonJSModules) { - - if (supportEs6Modules) { - FindModuleDependencies findDeps = - new FindModuleDependencies(this, supportEs6Modules, supportCommonJSModules); - findDeps.convertToEs6Module(input.getAstRoot(this)); - input.setJsModuleType(CompilerInput.ModuleType.ES6); - } else if (supportCommonJSModules) { - new ProcessCommonJSModules(this).process(null, input.getAstRoot(this), true); - input.setJsModuleType(CompilerInput.ModuleType.COMMONJS); - } - } - /** * Hoists inputs with the @externs annotation into the externs list. */ @@ -2145,6 +2112,7 @@ Map processJsonInputs(List inputsToProcess) { if (root == null) { continue; } + input.setJsModuleType(CompilerInput.ModuleType.JSON); rewriteJson.process(null, root); } return rewriteJson.getPackageJsonMainEntries(); @@ -2172,25 +2140,15 @@ private List parsePotentialModules(List inputsToPr return filteredInputs; } - /** - * Transforms AMD and CJS modules to something closure compiler can - * process and creates JSModules and the corresponding dependency tree - * on the way. - */ - void processAMDAndCommonJSModules() { + /** Transforms AMD to CJS modules */ + void processAMDModules() { for (CompilerInput input : inputs) { input.setCompiler(this); Node root = input.getAstRoot(this); if (root == null) { continue; } - if (options.transformAMDToCJSModules) { - new TransformAMDToCJSModule(this).process(null, root); - } - if (options.processCommonJSModules) { - ProcessCommonJSModules cjs = new ProcessCommonJSModules(this); - cjs.process(null, root); - } + new TransformAMDToCJSModule(this).process(null, root); } } diff --git a/src/com/google/javascript/jscomp/CompilerInput.java b/src/com/google/javascript/jscomp/CompilerInput.java index 5fc667a7490..97c394b4db3 100644 --- a/src/com/google/javascript/jscomp/CompilerInput.java +++ b/src/com/google/javascript/jscomp/CompilerInput.java @@ -521,8 +521,10 @@ public void reset() { enum ModuleType { NONE, - GOOG_MODULE, + GOOG, ES6, - COMMONJS + COMMONJS, + JSON, + IMPORTED_SCRIPT } } diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index 4c159a9a421..aebc0a74162 100644 --- a/src/com/google/javascript/jscomp/DefaultPassConfig.java +++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java @@ -203,6 +203,13 @@ protected List getTranspileOnlyPasses() { @Override protected List getWhitespaceOnlyPasses() { List passes = new ArrayList<>(); + + if (options.processCommonJSModules) { + passes.add(rewriteCommonJsModules); + } else if (options.getLanguageIn().toFeatureSet().has(FeatureSet.Feature.MODULES)) { + passes.add(rewriteScriptsToEs6Modules); + } + if (options.wrapGoogModulesForWhitespaceOnly) { passes.add(whitespaceWrapGoogModules); } @@ -251,6 +258,12 @@ protected List getChecks() { checks.add(createEmptyPass("beforeStandardChecks")); + if (options.processCommonJSModules) { + checks.add(rewriteCommonJsModules); + } else if (options.getLanguageIn().toFeatureSet().has(FeatureSet.Feature.MODULES)) { + checks.add(rewriteScriptsToEs6Modules); + } + // Note: ChromePass can rewrite invalid @type annotations into valid ones, so should run before // JsDoc checks. if (options.isChromePassEnabled()) { @@ -3432,4 +3445,30 @@ protected CompilerPass create(AbstractCompiler compiler) { return new RemoveSuperMethodsPass(compiler); } }; + + private final PassFactory rewriteCommonJsModules = + new PassFactory(PassNames.REWRITE_COMMON_JS_MODULES, true) { + @Override + protected CompilerPass create(AbstractCompiler compiler) { + return new ProcessCommonJSModules(compiler); + } + + @Override + public FeatureSet featureSet() { + return ES8_MODULES; + } + }; + + private final PassFactory rewriteScriptsToEs6Modules = + new PassFactory(PassNames.REWRITE_SCRIPTS_TO_ES6_MODULES, true) { + @Override + protected CompilerPass create(AbstractCompiler compiler) { + return new Es6RewriteScriptsToModules(compiler); + } + + @Override + protected FeatureSet featureSet() { + return ES8_MODULES; + } + }; } diff --git a/src/com/google/javascript/jscomp/Es6RewriteScriptsToModules.java b/src/com/google/javascript/jscomp/Es6RewriteScriptsToModules.java new file mode 100644 index 00000000000..0e908dc58fb --- /dev/null +++ b/src/com/google/javascript/jscomp/Es6RewriteScriptsToModules.java @@ -0,0 +1,76 @@ +/* + * Copyright 2017 The Closure Compiler Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.javascript.jscomp; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.javascript.jscomp.NodeTraversal.AbstractPreOrderCallback; +import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.Token; + +/** Rewrites a script which was imported as a module into an ES6 module. */ +public final class Es6RewriteScriptsToModules extends AbstractPreOrderCallback + implements HotSwapCompilerPass { + private final AbstractCompiler compiler; + + /** + * Creates a new Es6RewriteModules instance which can be used to rewrite ES6 modules to a + * concatenable form. + */ + public Es6RewriteScriptsToModules(AbstractCompiler compiler) { + this.compiler = compiler; + } + + /** + * Force rewriting of a script into an ES6 module, such as for imported files that contain no + * "import" or "export" statements. + */ + void forceToEs6Module(Node root) { + if (Es6RewriteModules.isEs6ModuleRoot(root)) { + return; + } + Node moduleNode = new Node(Token.MODULE_BODY).srcref(root); + moduleNode.addChildrenToBack(root.removeChildren()); + root.addChildToBack(moduleNode); + } + + @Override + public void process(Node externs, Node root) { + for (Node file = root.getFirstChild(); file != null; file = file.getNext()) { + hotSwapScript(file, null); + } + } + + @Override + public void hotSwapScript(Node scriptNode, Node originalRoot) { + checkArgument(scriptNode.isScript()); + NodeTraversal.traverseEs6(compiler, scriptNode, this); + } + + @Override + public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) { + if (n.isScript()) { + CompilerInput.ModuleType moduleType = compiler.getInput(n.getInputId()).getJsModuleType(); + + if (moduleType == CompilerInput.ModuleType.IMPORTED_SCRIPT) { + forceToEs6Module(n); + compiler.reportChangeToChangeScope(n); + } + } + + return false; + } +} diff --git a/src/com/google/javascript/jscomp/FindModuleDependencies.java b/src/com/google/javascript/jscomp/FindModuleDependencies.java index 69a1920128b..cb66cbad9df 100644 --- a/src/com/google/javascript/jscomp/FindModuleDependencies.java +++ b/src/com/google/javascript/jscomp/FindModuleDependencies.java @@ -55,7 +55,7 @@ public class FindModuleDependencies extends NodeTraversal.AbstractPostOrderCallb public void process(Node root) { checkArgument(root.isScript()); - if (FindModuleDependencies.isEs6ModuleRoot(root)) { + if (Es6RewriteModules.isEs6ModuleRoot(root)) { moduleType = ModuleType.ES6; } CompilerInput input = compiler.getInput(root.getInputId()); @@ -81,9 +81,22 @@ public void process(Node root) { @Override public void visit(NodeTraversal t, Node n, Node parent) { + if (parent == null + || NodeUtil.isControlStructure(parent) + || NodeUtil.isStatementBlock(parent)) { + if (n.isExprResult()) { + Node maybeGetProp = n.getFirstFirstChild(); + if (maybeGetProp != null + && (maybeGetProp.matchesQualifiedName("goog.provide") + || maybeGetProp.matchesQualifiedName("goog.module"))) { + moduleType = ModuleType.GOOG; + return; + } + } + } + if (supportsEs6Modules && n.isExport()) { moduleType = ModuleType.ES6; - } else if (supportsEs6Modules && n.isImport()) { moduleType = ModuleType.ES6; String moduleName; @@ -111,7 +124,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { } t.getInput().addOrderedRequire(moduleName); } else if (supportsCommonJsModules) { - if (ProcessCommonJSModules.isCommonJsExport(t, n)) { + if (moduleType != ModuleType.GOOG && ProcessCommonJSModules.isCommonJsExport(t, n)) { moduleType = ModuleType.COMMONJS; } else if (ProcessCommonJSModules.isCommonJsImport(n)) { String path = ProcessCommonJSModules.getCommonJsImportPath(n); @@ -143,15 +156,6 @@ public void visit(NodeTraversal t, Node n, Node parent) { } } - /** Return whether or not the given script node represents an ES6 module file. */ - public static boolean isEs6ModuleRoot(Node scriptNode) { - checkArgument(scriptNode.isScript()); - if (scriptNode.getBooleanProp(Node.GOOG_MODULE)) { - return false; - } - return scriptNode.hasChildren() && scriptNode.getFirstChild().isModuleBody(); - } - /** * Convert a script into a module by marking it's root node as a module body. This allows a script * which is imported as a module to be scoped as a module even without "import" or "export" @@ -164,7 +168,7 @@ public boolean convertToEs6Module(Node root) { } private boolean convertToEs6Module(Node root, boolean skipGoogProvideModuleCheck) { - if (isEs6ModuleRoot(root)) { + if (Es6RewriteModules.isEs6ModuleRoot(root)) { return true; } if (!skipGoogProvideModuleCheck) { diff --git a/src/com/google/javascript/jscomp/PassNames.java b/src/com/google/javascript/jscomp/PassNames.java index a8b1625e424..20028cfb2b1 100644 --- a/src/com/google/javascript/jscomp/PassNames.java +++ b/src/com/google/javascript/jscomp/PassNames.java @@ -77,6 +77,8 @@ public final class PassNames { public static final String OPTIMIZE_CALLS = "optimizeCalls"; public static final String PARSE_INPUTS = "parseInputs"; public static final String PEEPHOLE_OPTIMIZATIONS = "peepholeOptimizations"; + public static final String REWRITE_COMMON_JS_MODULES = "rewriteCommonJsModules"; + public static final String REWRITE_SCRIPTS_TO_ES6_MODULES = "rewriteScriptsToEs6Modules"; public static final String REMOVE_SUPER_METHODS = "removeSuperMethods"; public static final String REMOVE_UNREACHABLE_CODE = "removeUnreachableCode"; public static final String REMOVE_UNUSED_CLASS_PROPERTIES = "removeUnusedClassProperties"; diff --git a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java index 02c11570451..ff2810f5de9 100644 --- a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java +++ b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; @@ -29,17 +30,16 @@ import com.google.javascript.rhino.Node; import java.util.ArrayList; import java.util.List; +import java.util.Objects; /** - * Rewrites a CommonJS module http://wiki.commonjs.org/wiki/Modules/1.1.1 - * into a form that can be safely concatenated. - * Does not add a function around the module body but instead adds suffixes - * to global variables to avoid conflicts. - * Calls to require are changed to reference the required module directly. - * goog.provide and goog.require are emitted for closure compiler automatic - * ordering. + * Rewrites a CommonJS module http://wiki.commonjs.org/wiki/Modules/1.1.1 into a form that can be + * safely concatenated. Does not add a function around the module body but instead adds suffixes to + * global variables to avoid conflicts. Calls to require are changed to reference the required + * module directly. */ -public final class ProcessCommonJSModules implements CompilerPass { +public final class ProcessCommonJSModules extends NodeTraversal.AbstractPreOrderCallback + implements CompilerPass { private static final String EXPORTS = "exports"; private static final String MODULE = "module"; private static final String REQUIRE = "require"; @@ -54,80 +54,90 @@ public final class ProcessCommonJSModules implements CompilerPass { "Suspicious re-assignment of \"exports\" variable." + " Did you actually intend to export something?"); - private final Compiler compiler; + private final AbstractCompiler compiler; /** - * Creates a new ProcessCommonJSModules instance which can be used to - * rewrite CommonJS modules to a concatenable form. + * Creates a new ProcessCommonJSModules instance which can be used to rewrite CommonJS modules to + * a concatenable form. * * @param compiler The compiler */ - public ProcessCommonJSModules(Compiler compiler) { + public ProcessCommonJSModules(AbstractCompiler compiler) { this.compiler = compiler; } - - /** - * Module rewriting is done a on per-file basis prior to main compilation. The pass must handle - * ES6+ syntax and the root node for each file is a SCRIPT - not the typical jsRoot of other - * passes. - */ @Override public void process(Node externs, Node root) { - process(externs, root, false); + NodeTraversal.traverseEs6(compiler, root, this); } - public void process(Node externs, Node root, boolean forceModuleDetection) { - checkState(root.isScript()); - FindImportsAndExports finder = new FindImportsAndExports(); - NodeTraversal.traverseEs6(compiler, root, finder); - - ImmutableList.Builder exports = ImmutableList.builder(); - if (finder.isCommonJsModule() || forceModuleDetection) { - finder.reportModuleErrors(); - - if (!finder.umdPatterns.isEmpty()) { - boolean needsRetraverse = finder.replaceUmdPatterns(); + @Override + public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { + if (n.isRoot()) { + return true; + } else if (n.isScript()) { + FindImportsAndExports finder = new FindImportsAndExports(); + NodeTraversal.traverseEs6(compiler, n, finder); + + CompilerInput.ModuleType moduleType = compiler.getInput(n.getInputId()).getJsModuleType(); + boolean forceModuleDetection = moduleType == CompilerInput.ModuleType.IMPORTED_SCRIPT; + boolean isCommonJsModule = finder.isCommonJsModule(); + ImmutableList.Builder exports = ImmutableList.builder(); + if (isCommonJsModule || forceModuleDetection) { + finder.reportModuleErrors(); + + if (!finder.umdPatterns.isEmpty()) { + boolean needsRetraverse = finder.replaceUmdPatterns(); + + // Removing the IIFE rewrites vars. We need to re-traverse + // to get the new references. + if (removeIIFEWrapper(n)) { + needsRetraverse = true; + } - if (!needsRetraverse) { - needsRetraverse = removeIIFEWrapper(root); + if (needsRetraverse) { + finder = new FindImportsAndExports(); + NodeTraversal.traverseEs6(compiler, n, finder); + } } - // Inlining functions rewrites vars. We need to re-traverse - // to get the new references. - if (needsRetraverse) { - finder = new FindImportsAndExports(); - NodeTraversal.traverseEs6(compiler, root, finder); - } - } + finder.initializeModule(); - //UMD pattern replacement can leave detached export references - don't include those - for (ExportInfo export : finder.getModuleExports()) { - if (NodeUtil.getEnclosingScript(export.node) != null) { - exports.add(export); + //UMD pattern replacement can leave detached export references - don't include those + for (ExportInfo export : finder.getModuleExports()) { + if (NodeUtil.getEnclosingScript(export.node) != null) { + exports.add(export); + } } - } - for (ExportInfo export : finder.getExports()) { - if (NodeUtil.getEnclosingScript(export.node) != null) { - exports.add(export); + for (ExportInfo export : finder.getExports()) { + if (NodeUtil.getEnclosingScript(export.node) != null) { + exports.add(export); + } } } - finder.initializeModule(); + NodeTraversal.traverseEs6( + compiler, + n, + new RewriteModule(isCommonJsModule || forceModuleDetection, exports.build())); } + return false; + } - NodeTraversal.traverseEs6( - compiler, - root, - new RewriteModule(finder.isCommonJsModule() || forceModuleDetection, exports.build())); + public static String getModuleName(CompilerInput input) { + ModulePath modulePath = input.getPath(); + if (modulePath == null) { + return null; + } + + return getModuleName(modulePath); } - /** - * Recognize if a node is a module import. We recognize two forms: - * - *

- require("something"); - __webpack_require__(4); // only when the module resolution is - * WEBPACK - */ + public static String getModuleName(ModulePath input) { + return input.toModuleName(); + } + + /** Recognize if a node is a module import: require("something"); */ public static boolean isCommonJsImport(Node requireCall) { if (requireCall.isCall() && requireCall.hasTwoChildren()) { if (requireCall.getFirstChild().matchesQualifiedName(REQUIRE) @@ -145,9 +155,11 @@ public static String getCommonJsImportPath(Node requireCall) { /** * Recognize if a node is a module export. We recognize several forms: * - *

- module.exports = something; - module.exports.something = something; - exports.something = - * something; - __webpack_exports__["something"] = something; // only when the module resolution - * is WEBPACK + *

* *

In addition, we only recognize an export if the base export object is not defined or is * defined in externs. @@ -269,6 +281,7 @@ private boolean removeIIFEWrapper(Node root) { Node block = mutator.unwrapIifeInModule(iifeLabel, fnc, call); root.removeChildren(); root.addChildrenToFront(block.removeChildren()); + reportNestedScopesDeleted(fnc); compiler.reportChangeToEnclosingScope(root); return true; @@ -409,7 +422,9 @@ private void visitRequireCall(NodeTraversal t, Node require, Node parent) { if (!NodeUtil.isExpressionResultUsed(require) && parent.isExprResult() && NodeUtil.isStatementBlock(parent.getParent())) { + Node grandparent = parent.getParent(); parent.detach(); + compiler.reportChangeToEnclosingScope(grandparent); } } @@ -489,6 +504,10 @@ void reportModuleErrors() { * variable as a namespace. */ void initializeModule() { + if (exports.isEmpty() && moduleExports.isEmpty()) { + return; + } + CompilerInput ci = compiler.getInput(this.script.getInputId()); ModulePath modulePath = ci.getPath(); if (modulePath == null) { @@ -572,24 +591,32 @@ boolean replaceUmdPatterns() { boolean needsRetraverse = false; Node changeScope; for (UmdPattern umdPattern : umdPatterns) { + if (NodeUtil.getEnclosingScript(umdPattern.ifRoot) == null) { + reportNestedScopesDeleted(umdPattern.ifRoot); + continue; + } + Node parent = umdPattern.ifRoot.getParent(); Node newNode = umdPattern.activeBranch; if (newNode == null) { parent.removeChild(umdPattern.ifRoot); + reportNestedScopesDeleted(umdPattern.ifRoot); compiler.reportChangeToEnclosingScope(parent); + needsRetraverse = true; continue; } // Remove redundant block node. Not strictly necessary, but makes tests more legible. if (umdPattern.activeBranch.isNormalBlock() && umdPattern.activeBranch.getChildCount() == 1) { - newNode = umdPattern.activeBranch.getFirstChild(); - umdPattern.activeBranch.detachChildren(); + newNode = umdPattern.activeBranch.removeFirstChild(); } else { - umdPattern.ifRoot.detachChildren(); + newNode.detach(); } + needsRetraverse = true; parent.replaceChild(umdPattern.ifRoot, newNode); + reportNestedScopesDeleted(umdPattern.ifRoot); changeScope = NodeUtil.getEnclosingChangeScopeRoot(newNode); if (changeScope != null) { compiler.reportChangeToEnclosingScope(newNode); @@ -618,12 +645,13 @@ boolean replaceUmdPatterns() { continue; } needsRetraverse = true; - String factoryLabel = modulePath.toModuleName() + "_factory"; + String factoryLabel = + modulePath.toModuleName() + "_factory" + compiler.getUniqueNameIdSupplier().get(); - FunctionToBlockMutator mutator = new FunctionToBlockMutator( - compiler, compiler.getUniqueNameIdSupplier()); - Node newStatements = mutator.mutate( - factoryLabel, fn, enclosingFnCall, null, false, false); + FunctionToBlockMutator mutator = + new FunctionToBlockMutator(compiler, compiler.getUniqueNameIdSupplier()); + Node newStatements = + mutator.mutate(factoryLabel, fn, enclosingFnCall, null, false, false); // Check to see if the returned block is of the form: // { @@ -631,6 +659,13 @@ boolean replaceUmdPatterns() { // jscomp$inline(); // } // + // or + // + // { + // var jscomp$inline = function() {}; + // module.exports = jscomp$inline(); + // } + // // If so, inline again if (newStatements.isNormalBlock() && newStatements.hasTwoChildren() @@ -641,21 +676,25 @@ boolean replaceUmdPatterns() { Node inlinedFn = newStatements.getFirstFirstChild().getFirstChild(); Node expr = newStatements.getSecondChild().getFirstChild(); Node call = null; + String assignedName = null; if (expr.isAssign() && expr.getSecondChild().isCall()) { call = expr.getSecondChild(); + assignedName = + modulePath.toModuleName() + "_iife" + compiler.getUniqueNameIdSupplier().get(); } else if (expr.isCall()) { call = expr; } if (call != null) { - newStatements = mutator.mutate( - factoryLabel, inlinedFn, call, null, false, false); - if (expr.isAssign() && newStatements.hasOneChild() - && newStatements.getFirstChild().isExprResult()) { - expr.replaceChild( - expr.getSecondChild(), - newStatements.getFirstFirstChild().detach()); - newStatements = expr.getParent().detach(); + newStatements = + mutator.mutate(factoryLabel, inlinedFn, call, assignedName, false, false); + if (assignedName != null) { + Node newName = + NodeUtil.newName( + compiler, assignedName, fn, expr.getFirstChild().getQualifiedName()); + newStatements.addChildToFront(IR.var(newName).useSourceInfoFromForTree(fn)); + expr.replaceChild(expr.getSecondChild(), newName.cloneNode()); + newStatements.addChildToBack(expr.getParent().detach()); } } } @@ -665,20 +704,14 @@ boolean replaceUmdPatterns() { callRoot = callRoot.getParent(); } if (callRoot.isExprResult()) { - callRoot = callRoot.getParent(); - - callRoot.detachChildren(); - callRoot.addChildToFront(newStatements); - changeScope = NodeUtil.getEnclosingChangeScopeRoot(callRoot); - if (changeScope != null) { - compiler.reportChangeToEnclosingScope(callRoot); - } + callRoot.replaceWith(newStatements); + reportNestedScopesChanged(newStatements); + compiler.reportChangeToEnclosingScope(newStatements); + reportNestedScopesDeleted(enclosingFnCall); } else { parent.replaceChild(umdPattern.ifRoot, newNode); - changeScope = NodeUtil.getEnclosingChangeScopeRoot(newNode); - if (changeScope != null) { - compiler.reportChangeToEnclosingScope(newNode); - } + compiler.reportChangeToEnclosingScope(newNode); + reportNestedScopesDeleted(umdPattern.ifRoot); } } } @@ -686,6 +719,34 @@ boolean replaceUmdPatterns() { } } + private void reportNestedScopesDeleted(Node n) { + NodeUtil.visitPreOrder( + n, + new NodeUtil.Visitor() { + @Override + public void visit(Node n) { + if (n.isFunction()) { + compiler.reportFunctionDeleted(n); + } + } + }, + Predicates.alwaysTrue()); + } + + private void reportNestedScopesChanged(Node n) { + NodeUtil.visitPreOrder( + n, + new NodeUtil.Visitor() { + @Override + public void visit(Node n) { + if (n.isFunction()) { + compiler.reportChangeToChangeScope(n); + } + } + }, + Predicates.alwaysTrue()); + } + private static boolean umdPatternsContains(List umdPatterns, Node n) { for (UmdPattern umd : umdPatterns) { if (umd.ifRoot == n) { @@ -796,7 +857,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { Var nameDeclaration = t.getScope().getVar(qName); if (nameDeclaration != null && nameDeclaration.getNode() != null - && nameDeclaration.getNode().getInputId() == n.getInputId()) { + && Objects.equals(nameDeclaration.getNode().getInputId(), n.getInputId())) { maybeUpdateName(t, n, nameDeclaration); } @@ -1141,7 +1202,7 @@ private void updateNameReference( if (NodeUtil.isFunctionExpression(parent)) { // Don't refactor if the parent is a named function expression. // e.g. var foo = function foo() {}; - break; + return; } Node newNameRef = NodeUtil.newQName(compiler, newName, nameRef, originalName); Node grandparent = parent.getParent(); @@ -1241,7 +1302,7 @@ private void updateNameReference( * is actually the export target itself, return null; */ private String getExportedName(NodeTraversal t, Node n, Var var) { - if (var == null || var.getNode().getInputId() != n.getInputId()) { + if (var == null || !Objects.equals(var.getNode().getInputId(), n.getInputId())) { return n.getQualifiedName(); } @@ -1445,7 +1506,7 @@ private void fixTypeNode(NodeTraversal t, Node typeNode) { // Make sure we can find a variable declaration (and it's in this file) if (typeDeclaration != null - && typeDeclaration.getNode().getInputId() == typeNode.getInputId()) { + && Objects.equals(typeDeclaration.getNode().getInputId(), typeNode.getInputId())) { String importedModuleName = getModuleImportName(t, typeDeclaration.getNode()); // If the name is an import alias, rewrite it to be a reference to the diff --git a/src/com/google/javascript/jscomp/deps/ModuleLoader.java b/src/com/google/javascript/jscomp/deps/ModuleLoader.java index 7acdcb2f9bb..8261eb8594e 100644 --- a/src/com/google/javascript/jscomp/deps/ModuleLoader.java +++ b/src/com/google/javascript/jscomp/deps/ModuleLoader.java @@ -58,7 +58,7 @@ public final class ModuleLoader { DiagnosticType.error( "JSC_INVALID_MODULE_PATH", "Invalid module path \"{0}\" for resolution mode \"{1}\""); - private final ErrorHandler errorHandler; + private ErrorHandler errorHandler; /** Root URIs to match module roots against. */ private final ImmutableList moduleRootPaths; @@ -306,6 +306,19 @@ private static String normalize(String path, Iterable moduleRootPaths) { return path; } + public void setErrorHandler(ErrorHandler errorHandler) { + if (errorHandler == null) { + this.errorHandler = new NoopErrorHandler(); + } else { + this.errorHandler = errorHandler; + } + this.moduleResolver.setErrorHandler(this.errorHandler); + } + + public ErrorHandler getErrorHandler() { + return this.errorHandler; + } + /** An enum indicating whether to absolutize paths. */ public enum PathResolver implements Function { RELATIVE { diff --git a/src/com/google/javascript/jscomp/deps/ModuleResolver.java b/src/com/google/javascript/jscomp/deps/ModuleResolver.java index c0a66d238e5..efd52b275da 100644 --- a/src/com/google/javascript/jscomp/deps/ModuleResolver.java +++ b/src/com/google/javascript/jscomp/deps/ModuleResolver.java @@ -31,7 +31,7 @@ public abstract class ModuleResolver { /** Root URIs to match module roots against. */ protected final ImmutableList moduleRootPaths; - protected final ErrorHandler errorHandler; + protected ErrorHandler errorHandler; public ModuleResolver( ImmutableSet modulePaths, @@ -98,4 +98,8 @@ protected String canonicalizePath(String scriptAddress, String moduleAddress) { } return path; } + + public void setErrorHandler(ErrorHandler errorHandler) { + this.errorHandler = errorHandler; + } } diff --git a/test/com/google/javascript/jscomp/CommandLineRunnerTest.java b/test/com/google/javascript/jscomp/CommandLineRunnerTest.java index 746005ead89..eaa70a278a2 100644 --- a/test/com/google/javascript/jscomp/CommandLineRunnerTest.java +++ b/test/com/google/javascript/jscomp/CommandLineRunnerTest.java @@ -1839,6 +1839,7 @@ public void testES6ImportOfFileWithoutImportsOrExports() { args.add("--dependency_mode=STRICT"); args.add("--entry_point='./app.js'"); args.add("--language_in=ECMASCRIPT6"); + args.add("--language_out=ECMASCRIPT5"); setFilename(0, "foo.js"); setFilename(1, "app.js"); test( diff --git a/test/com/google/javascript/jscomp/CompilerTest.java b/test/com/google/javascript/jscomp/CompilerTest.java index 0d1fa2b9c9e..cd03413d58e 100644 --- a/test/com/google/javascript/jscomp/CompilerTest.java +++ b/test/com/google/javascript/jscomp/CompilerTest.java @@ -28,7 +28,6 @@ import com.google.debugging.sourcemap.SourceMapGeneratorV3; import com.google.debugging.sourcemap.proto.Mapping.OriginalMapping; import com.google.javascript.jscomp.CompilerOptions.LanguageMode; -import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.InputId; import com.google.javascript.rhino.Node; @@ -140,19 +139,6 @@ public void testLocalUndefined() throws Exception { compiler.compile(externs, input, options); } - public void testCommonJSMissingRequire() throws Exception { - List inputs = ImmutableList.of( - SourceFile.fromCode("/gin.js", "require('missing')")); - Compiler compiler = initCompilerForCommonJS( - inputs, ImmutableList.of(ModuleIdentifier.forFile("/gin"))); - - ErrorManager manager = compiler.getErrorManager(); - JSError[] errors = manager.getErrors(); - assertThat(errors).hasLength(1); - String error = errors[0].toString(); - assertThat(error).contains("Failed to load module \"missing\" at /gin.js"); - } - private static String normalize(String path) { return path.replace(File.separator, "/"); } @@ -340,21 +326,6 @@ public void testApplyInputSourceMaps() throws Exception { assertThat(mapping.getIdentifier()).isEqualTo("testSymbolName"); } - private Compiler initCompilerForCommonJS( - List inputs, List entryPoints) - throws Exception { - CompilerOptions options = new CompilerOptions(); - options.setIdeMode(true); - options.dependencyOptions.setDependencyPruning(true); - options.dependencyOptions.setMoocherDropping(true); - options.dependencyOptions.setEntryPoints(entryPoints); - options.setProcessCommonJSModules(true); - options.setModuleResolutionMode(ModuleLoader.ResolutionMode.NODE); - Compiler compiler = new Compiler(); - compiler.init(new ArrayList(), inputs, options); - compiler.parseInputs(); - return compiler; - } private static final ImmutableList EMPTY_EXTERNS = ImmutableList.of(SourceFile.fromCode("externs", "")); diff --git a/test/com/google/javascript/jscomp/Es6RewriteScriptsToModulesTest.java b/test/com/google/javascript/jscomp/Es6RewriteScriptsToModulesTest.java new file mode 100644 index 00000000000..2738c2c35c1 --- /dev/null +++ b/test/com/google/javascript/jscomp/Es6RewriteScriptsToModulesTest.java @@ -0,0 +1,68 @@ +/* + * Copyright 2017 The Closure Compiler Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.javascript.jscomp; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.javascript.rhino.Node; + +/** Unit tests for {@link Es6RewriteScriptsToModules} */ +public final class Es6RewriteScriptsToModulesTest extends CompilerTestCase { + + @Override + protected CompilerPass getProcessor(Compiler compiler) { + return new Es6RewriteScriptsToModules(compiler); + } + + @Override + protected CompilerOptions getOptions() { + CompilerOptions options = super.getOptions(); + options.setLanguageIn(CompilerOptions.LanguageMode.ECMASCRIPT_2015); + return options; + } + + @Override + protected int getNumRepetitions() { + return 1; + } + + public void testImportedScript() { + Compiler compiler = createCompiler(); + CompilerOptions options = getOptions(); + compiler.init( + ImmutableList.of(), + ImmutableList.of( + SourceFile.fromCode("/script.js", ""), + SourceFile.fromCode("/module.js", "import '/script.js';")), + options); + Node root = compiler.parseInputs(); + String errorMsg = LINE_JOINER.join(compiler.getErrors()); + assertThat(errorMsg).isEmpty(); + Node externsRoot = root.getFirstChild(); + Node mainRoot = externsRoot.getNext(); + getProcessor(compiler).process(externsRoot, mainRoot); + assertTrue(mainRoot.getFirstFirstChild().isModuleBody()); + } + + public void testNonImportedScript() { + testSame( + srcs( + SourceFile.fromCode("/script.js", ""), + SourceFile.fromCode("/module.js", "export default 'foo';"))); + } +} diff --git a/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java b/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java index 9615191c2b6..05b3552a39b 100644 --- a/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java +++ b/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java @@ -30,7 +30,6 @@ public final class ProcessCommonJSModulesTest extends CompilerTestCase { @Override protected CompilerOptions getOptions() { CompilerOptions options = super.getOptions(); - // Trigger module processing after parsing. options.setProcessCommonJSModules(true); options.setModuleResolutionMode(ModuleLoader.ResolutionMode.NODE); @@ -43,9 +42,7 @@ protected CompilerOptions getOptions() { @Override protected CompilerPass getProcessor(Compiler compiler) { - // CommonJS module handling is done directly after parsing, so not included here. - // It also depends on es6 module rewriting, however, so that must be explicitly included. - return new Es6RewriteModules(compiler); + return new ProcessCommonJSModules(compiler); } @Override @@ -507,20 +504,27 @@ public void testClassRewriting() { public void testMultipleAssignments() { setLanguage( CompilerOptions.LanguageMode.ECMASCRIPT_2015, CompilerOptions.LanguageMode.ECMASCRIPT5); - setExpectParseWarningsThisTest(); - testModules( - "test.js", - lines( - "/** @constructor */ function Hello() {}", - "module.exports = Hello;", - "/** @constructor */ function Bar() {} ", - "Bar.prototype.foobar = function() { alert('foobar'); };", - "exports = Bar;"), - lines( - "var module$test = /** @constructor */ function(){};", - "/** @constructor */ function Bar$$module$test(){}", - "Bar$$module$test.prototype.foobar = function() { alert('foobar'); };", - "exports = Bar$$module$test;")); + + JSModule module = new JSModule("out"); + module.add(SourceFile.fromCode("other.js", "goog.provide('module$other');")); + module.add(SourceFile.fromCode("yet_another.js", "goog.provide('module$yet_another');")); + module.add( + SourceFile.fromCode( + "test", + lines( + "/** @constructor */ function Hello() {}", + "module.exports = Hello;", + "/** @constructor */ function Bar() {} ", + "Bar.prototype.foobar = function() { alert('foobar'); };", + "exports = Bar;"))); + JSModule[] modules = {module}; + test( + modules, + null, + new Diagnostic( + ProcessCommonJSModules.SUSPICIOUS_EXPORTS_ASSIGNMENT.level, + ProcessCommonJSModules.SUSPICIOUS_EXPORTS_ASSIGNMENT, + null)); } public void testDestructuringImports() { @@ -656,7 +660,7 @@ public void testUMDRemoveIIFE() { " this.foobar = foobar;", "}}.call(window))"), lines( - "var module$test = {};", + "var module$test;", "(function(){", " module$test={foo:\"bar\"};", "}).call(window);")); @@ -676,7 +680,7 @@ public void testUMDRemoveIIFE() { "}})();", "alert('foo');"), lines( - "var module$test = {};", + "var module$test;", "(function(){", " module$test={foo:\"bar\"};", "})();", @@ -697,7 +701,7 @@ public void testUMDRemoveIIFE() { " this.foobar = foobar;", "}})();"), lines( - "var module$test = {};", + "var module$test;", "alert('foo');", "(function(){", " module$test={foo:\"bar\"};", @@ -878,16 +882,16 @@ public void testLeafletUMDWrapper() { lines( "/** @const */ var module$test={};", "{", - " var exports$jscomp$inline_3$$module$test=module$test;", - " var userAgentContains$jscomp$inline_5$$module$test=", - " function(str$jscomp$inline_6){", + " var exports$jscomp$inline_4$$module$test=module$test;", + " var userAgentContains$jscomp$inline_6$$module$test=", + " function(str$jscomp$inline_7){", " return navigator.userAgent.toLowerCase().indexOf(", - " str$jscomp$inline_6)>=0;", + " str$jscomp$inline_7)>=0;", " };", - " var webkit$jscomp$inline_4$$module$test=", - " userAgentContains$jscomp$inline_5$$module$test('webkit');", - " exports$jscomp$inline_3$$module$test.webkit=", - " webkit$jscomp$inline_4$$module$test;", + " var webkit$jscomp$inline_5$$module$test=", + " userAgentContains$jscomp$inline_6$$module$test('webkit');", + " exports$jscomp$inline_4$$module$test.webkit=", + " webkit$jscomp$inline_5$$module$test;", "}")); } @@ -905,8 +909,10 @@ public void testBowserUMDWrapper() { " return {foo: 'bar'};", "});"), lines( - "/** @const */ var module$test={};", - "module$test.foo = 'bar';")); + "{", + " var module$test;", + " module$test = {foo: 'bar'};", + "}")); } public void testDontSplitVarsInFor() { @@ -932,4 +938,8 @@ public void testIssue2616() { " return 1;", "};")); } + + public void testMissingRequire() { + ModulesTestUtils.testModulesError(this, "require('missing');", ModuleLoader.LOAD_WARNING); + } }