Skip to content

Commit

Permalink
Handle interop between goog.provides and goog.modules and fix typedef…
Browse files Browse the repository at this point in the history
… bug

 - declare legacy namespace exports in the global scope
 - goog.modules can require a goog.provide'd name

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=244976803
  • Loading branch information
lauraharker authored and blickly committed Apr 25, 2019
1 parent ccc1f68 commit d5ccde0
Show file tree
Hide file tree
Showing 6 changed files with 377 additions and 69 deletions.
15 changes: 8 additions & 7 deletions src/com/google/javascript/jscomp/AstValidator.java
Expand Up @@ -466,16 +466,17 @@ private void validateNameType(Node nameNode) {
private void validateCallType(Node callNode) { private void validateCallType(Node callNode) {
// TODO(b/74537281): Shouldn't CALL nodes always have a type, even if it is unknown? // TODO(b/74537281): Shouldn't CALL nodes always have a type, even if it is unknown?
Node callee = callNode.getFirstChild(); Node callee = callNode.getFirstChild();
JSType calleeTypeI = JSType calleeType =
checkNotNull(callee.getJSType(), "Callee of\n\n%s\nhas no type.", callNode.toStringTree()); checkNotNull(callee.getJSType(), "Callee of\n\n%s\nhas no type.", callNode.toStringTree());


if (calleeTypeI.isFunctionType()) { if (calleeType.isFunctionType()) {
FunctionType calleeFunctionTypeI = calleeTypeI.toMaybeFunctionType(); FunctionType calleeFunctionType = calleeType.toMaybeFunctionType();
JSType returnTypeI = calleeFunctionTypeI.getReturnType(); JSType returnType = calleeFunctionType.getReturnType();
// Skip this check if the call node was originally in a cast, because the cast type may be // Skip this check if the call node was originally in a cast, because the cast type may be
// narrower than the return type. // narrower than the return type. Also skip the check if the function's return type is the
if (callNode.getJSTypeBeforeCast() == null) { // any (formerly unknown) type, since we may have inferred a better type.
expectMatchingTypeInformation(callNode, returnTypeI); if (callNode.getJSTypeBeforeCast() == null && !returnType.isUnknownType()) {
expectMatchingTypeInformation(callNode, returnType);
} }
} // TODO(b/74537281): What other cases should be covered? } // TODO(b/74537281): What other cases should be covered?
} }
Expand Down
38 changes: 26 additions & 12 deletions src/com/google/javascript/jscomp/ModuleImportResolver.java
Expand Up @@ -15,6 +15,7 @@
*/ */
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import static com.google.common.base.Preconditions.checkArgument;


import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.javascript.jscomp.modules.Module; import com.google.javascript.jscomp.modules.Module;
Expand Down Expand Up @@ -74,23 +75,36 @@ ScopedName getClosureNamespaceTypeFromCall(Node googRequire) {
if (module == null) { if (module == null) {
return null; return null;
} }
switch (module.metadata().moduleType()) {
case GOOG_PROVIDE:
// Expect this to be a global variable
Node provide = module.metadata().rootNode();
if (provide != null && provide.isScript()) {
return ScopedName.of(moduleId, provide.getGrandparent());
} else {
// Unknown module requires default to 'goog provides', but we don't want to type them.
return null;
}


Node scopeRoot = getModuleScopeRoot(module); case GOOG_MODULE:
if (scopeRoot != null) { case LEGACY_GOOG_MODULE:
return ScopedName.of("exports", scopeRoot); // TODO(b/124919359): Fix getGoogModuleScopeRoot to never return null.
Node scopeRoot = getGoogModuleScopeRoot(module);
return scopeRoot != null ? ScopedName.of("exports", scopeRoot) : null;
case ES6_MODULE:
throw new IllegalStateException("Type checking ES modules not yet supported");
case COMMON_JS:
throw new IllegalStateException("Type checking CommonJs modules not yet supported");
case SCRIPT:
throw new IllegalStateException("Cannot import a name from a SCRIPT");
} }
// TODO(b/124919359): assert that this is non-nullable once getModuleScopeRoot handles modules throw new AssertionError();
// other than goog.modules.
return null;
} }


/** Converts a {@link Module} reference to a {@link Node} scope root. */ /** Returns the corresponding scope root Node from a goog.module. */
@Nullable @Nullable
private Node getModuleScopeRoot(@Nullable Module module) { private Node getGoogModuleScopeRoot(@Nullable Module module) {
if (!module.metadata().isGoogModule()) { checkArgument(module.metadata().isGoogModule(), module.metadata());
// TODO(b/124919359): also handle ES modules and goog.provides
return null;
}
Node scriptNode = module.metadata().rootNode(); Node scriptNode = module.metadata().rootNode();


if (scriptNode.isScript() if (scriptNode.isScript()
Expand Down
Expand Up @@ -54,6 +54,9 @@
* <p>This also annotates all provided namespace definitions `a.b = {};` with {@link * <p>This also annotates all provided namespace definitions `a.b = {};` with {@link
* Node#IS_NAMESPACE} so that later passes know they refer to a goog.provide'd namespace. * Node#IS_NAMESPACE} so that later passes know they refer to a goog.provide'd namespace.
* *
* <p>If modules have not been rewritten, this pass also includes legacy Closure module namespaces
* in the list of {@link ProvidedName}s.
*
* @author chrisn@google.com (Chris Nokleberg) * @author chrisn@google.com (Chris Nokleberg)
*/ */
class ProcessClosureProvidesAndRequires { class ProcessClosureProvidesAndRequires {
Expand Down Expand Up @@ -151,6 +154,20 @@ private void checkForLateOrMissingProvide(UnrecognizedRequire r) {
private class CollectDefinitions implements NodeTraversal.Callback { private class CollectDefinitions implements NodeTraversal.Callback {
@Override @Override
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
// Don't recurse into modules, which cannot have goog.provides. We do need to handle legacy
// goog.modules, but we do that quickly here rather than descending all the way into them.
// We completely ignore ES modules and CommonJS modules.
if ((n.isModuleBody() && n.getParent().getBooleanProp(Node.GOOG_MODULE))
|| NodeUtil.isBundledGoogModuleScopeRoot(n)) {
Node googModuleCall = n.getFirstChild();
String closureNamespace = googModuleCall.getFirstChild().getSecondChild().getString();
Node maybeLegacyNamespaceCall = googModuleCall.getNext();
if (maybeLegacyNamespaceCall != null
&& NodeUtil.isGoogModuleDeclareLegacyNamespaceCall(maybeLegacyNamespaceCall)) {
processLegacyModuleCall(closureNamespace, googModuleCall, t.getModule());
}
return false;
}
return !n.isModuleBody(); return !n.isModuleBody();
} }


Expand Down Expand Up @@ -322,6 +339,19 @@ private void processRequireCall(NodeTraversal t, Node n, Node parent) {
} }
} }


/** Handles a goog.module that is a legacy namespace. */
private void processLegacyModuleCall(String namespace, Node googModuleCall, JSModule module) {
registerAnyProvidedPrefixes(namespace, googModuleCall, module);
providedNames.put(
namespace,
new ProvidedName(
namespace,
googModuleCall,
module,
/* explicit= */ true,
/* fromPreviousProvide= */ false));
}

/** Handles a goog.provide call. */ /** Handles a goog.provide call. */
private void processProvideCall(NodeTraversal t, Node n, Node parent) { private void processProvideCall(NodeTraversal t, Node n, Node parent) {
checkState(n.isCall()); checkState(n.isCall());
Expand Down Expand Up @@ -700,8 +730,8 @@ private boolean hasCandidateDefinitionNotFromPreviousPass() {
} }


/** /**
* Returns the `goog.provide` call that created this name, if any, or otherwise the first * Returns the `goog.provide` or legacy namespace `goog.module` call that created this name, if
* 'previous provide' assignmetn that created this name. * any, or otherwise the first 'previous provide' assignment that created this name.
*/ */
Node getFirstProvideCall() { Node getFirstProvideCall() {
return firstNode; return firstNode;
Expand Down

0 comments on commit d5ccde0

Please sign in to comment.