Skip to content

Commit

Permalink
Scope name lookups in the JSTypeRegistry.
Browse files Browse the repository at this point in the history 
This addresses a variety of issues around declaring types in local scopes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=193740597
  • Loading branch information
@concavelenz @brad4d
concavelenz authored and brad4d committed Apr 23, 2018
1 parent d02b3e1 commit d8b84be
Show file tree
Hide file tree
Showing 14 changed files with 434 additions and 183 deletions.
19 changes: 13 additions & 6 deletions src/com/google/javascript/jscomp/FunctionTypeBuilder.java
View file
Expand Up @@ -799,7 +799,9 @@ private FunctionType getOrCreateConstructor() {
returnType, returnType,
classTemplateTypeNames, classTemplateTypeNames,
isAbstract); isAbstract);
JSType existingType = typeRegistry.getType(fnName); // We use "getTypeForScope" to specifically check if this was defined for getScopeDeclaredIn()
// so we don't pick up types that are going to be shadowed.
JSType existingType = typeRegistry.getTypeForScope(getScopeDeclaredIn(), fnName);


if (makesStructs) { if (makesStructs) {
fnType.setStruct(); fnType.setStruct();
Expand Down Expand Up @@ -833,16 +835,20 @@ private FunctionType getOrCreateConstructor() {


maybeSetBaseType(fnType); maybeSetBaseType(fnType);


if (getScopeDeclaredIn().isGlobal() && !fnName.isEmpty()) { // TODO(johnlenz): determine what we are supposed to do for:
typeRegistry.declareType(fnName, fnType.getInstanceType()); // @constructor
// this.Foo = ...
//
if (!fnName.isEmpty() && !fnName.startsWith("this.")) {
typeRegistry.declareTypeForExactScope(getScopeDeclaredIn(), fnName, fnType.getInstanceType());
} }
return fnType; return fnType;
} }


private FunctionType getOrCreateInterface() { private FunctionType getOrCreateInterface() {
FunctionType fnType = null; FunctionType fnType = null;


JSType type = typeRegistry.getType(fnName); JSType type = typeRegistry.getType(getScopeDeclaredIn(), fnName);
if (type != null && type.isInstanceType()) { if (type != null && type.isInstanceType()) {
FunctionType ctor = type.toMaybeObjectType().getConstructor(); FunctionType ctor = type.toMaybeObjectType().getConstructor();
if (ctor.isInterface()) { if (ctor.isInterface()) {
Expand All @@ -854,8 +860,9 @@ private FunctionType getOrCreateInterface() {
if (fnType == null) { if (fnType == null) {
fnType = typeRegistry.createInterfaceType( fnType = typeRegistry.createInterfaceType(
fnName, contents.getSourceNode(), classTemplateTypeNames, makesStructs); fnName, contents.getSourceNode(), classTemplateTypeNames, makesStructs);
if (getScopeDeclaredIn().isGlobal() && !fnName.isEmpty()) { if (!fnName.isEmpty()) {
typeRegistry.declareType(fnName, fnType.getInstanceType()); typeRegistry.declareTypeForExactScope(
getScopeDeclaredIn(), fnName, fnType.getInstanceType());
} }
maybeSetBaseType(fnType); maybeSetBaseType(fnType);
} }
Expand Down
6 changes: 0 additions & 6 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
View file
Expand Up @@ -228,12 +228,6 @@ public JSType getGlobalType(String typeName) {
return getType(null, typeName); return getType(null, typeName);
} }


@Override
public JSType getType(String typeName) {
return getType(null, typeName);
}

@SuppressWarnings("unchecked")
@Override @Override
public JSType getType(StaticScope scope, String typeName) { public JSType getType(StaticScope scope, String typeName) {
// Primitives are not present in the global scope, so hardcode them // Primitives are not present in the global scope, so hardcode them
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/LinkedFlowScope.java
View file
Expand Up @@ -309,7 +309,7 @@ private boolean optimizesToSameScope(LinkedFlowScope that) {


@Override @Override
public TypedScope getDeclarationScope() { public TypedScope getDeclarationScope() {
return this.syntacticScope; return syntacticScope;
} }


/** Join the two FlowScopes. */ /** Join the two FlowScopes. */
Expand Down
17 changes: 13 additions & 4 deletions src/com/google/javascript/jscomp/TypeInference.java
View file
Expand Up @@ -595,7 +595,15 @@ private FlowScope traverseAssign(Node n, FlowScope scope) {
return scope; return scope;
} }


private boolean isPossibleMixinApplication(Node lvalue, Node rvalue) { private static boolean isInExternFile(Node n) {
return NodeUtil.getSourceFile(n).isExtern();
}

private static boolean isPossibleMixinApplication(Node lvalue, Node rvalue) {
if (isInExternFile(lvalue)) {
return true;
}

JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(lvalue); JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(lvalue);
return jsdoc != null return jsdoc != null
&& jsdoc.isConstructor() && jsdoc.isConstructor()
Expand All @@ -609,8 +617,8 @@ private boolean isPossibleMixinApplication(Node lvalue, Node rvalue) {
* The constructor implements at least one interface. If the constructor is missing some * The constructor implements at least one interface. If the constructor is missing some
* properties of the inherited interfaces, this method declares these properties. * properties of the inherited interfaces, this method declares these properties.
*/ */
private void addMissingInterfaceProperties(JSType constructor) { private static void addMissingInterfaceProperties(JSType constructor) {
if (constructor.isConstructor()) { if (constructor != null && constructor.isConstructor()) {
FunctionType f = constructor.toMaybeFunctionType(); FunctionType f = constructor.toMaybeFunctionType();
ObjectType proto = f.getPrototype(); ObjectType proto = f.getPrototype();
for (ObjectType interf : f.getImplementedInterfaces()) { for (ObjectType interf : f.getImplementedInterfaces()) {
Expand Down Expand Up @@ -1655,7 +1663,8 @@ private JSType getPropertyType(JSType objType, String propName,
if ((propertyType == null || propertyType.isUnknownType()) if ((propertyType == null || propertyType.isUnknownType())
&& qualifiedName != null) { && qualifiedName != null) {
// If we find this node in the registry, then we can infer its type. // If we find this node in the registry, then we can infer its type.
ObjectType regType = ObjectType.cast(registry.getType(qualifiedName)); ObjectType regType = ObjectType.cast(
registry.getType(scope.getDeclarationScope(), qualifiedName));
if (regType != null) { if (regType != null) {
propertyType = regType.getConstructor(); propertyType = regType.getConstructor();
} }
Expand Down
62 changes: 27 additions & 35 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
View file
Expand Up @@ -373,8 +373,8 @@ void patchGlobalScope(TypedScope globalScope, Node scriptRoot) {
String typeName = var.getName(); String typeName = var.getName();
globalScope.undeclare(var); globalScope.undeclare(var);
globalScope.getTypeOfThis().toObjectType().removeProperty(typeName); globalScope.getTypeOfThis().toObjectType().removeProperty(typeName);
if (typeRegistry.getType(typeName) != null) { if (typeRegistry.getType(globalScope, typeName) != null) {
typeRegistry.removeType(typeName); typeRegistry.removeType(globalScope, typeName);
} }
} }


Expand Down Expand Up @@ -664,8 +664,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
case CONST: case CONST:
defineVar(n); defineVar(n);
// Handle typedefs. // Handle typedefs.
if ((n.isVar() ? currentHoistScope : currentScope).isGlobal() && n.hasOneChild()) { if (n.hasOneChild()) {
// TODO(bradfordcsmith): Defining typedefs in local scopes should be possible.
checkForTypedef(n.getFirstChild(), n.getJSDocInfo()); checkForTypedef(n.getFirstChild(), n.getJSDocInfo());
} }
break; break;
Expand Down Expand Up @@ -769,8 +768,7 @@ private void defineObjectLiteral(Node objectLit) {
String lValueName = NodeUtil.getBestLValueName(lValue); String lValueName = NodeUtil.getBestLValueName(lValue);
boolean createdEnumType = false; boolean createdEnumType = false;
if (info != null && info.hasEnumParameterType()) { if (info != null && info.hasEnumParameterType()) {
boolean global = isLValueRootedInGlobalScope(lValue); type = createEnumTypeFromNodes(objectLit, lValueName, info);
type = createEnumTypeFromNodes(objectLit, lValueName, info, global);
createdEnumType = true; createdEnumType = true;
} }


Expand Down Expand Up @@ -918,8 +916,7 @@ void defineFunctionLiteral(Node n) {
Node lValue = NodeUtil.getBestLValue(n); Node lValue = NodeUtil.getBestLValue(n);
JSDocInfo info = NodeUtil.getBestJSDocInfo(n); JSDocInfo info = NodeUtil.getBestJSDocInfo(n);
String functionName = NodeUtil.getBestLValueName(lValue); String functionName = NodeUtil.getBestLValueName(lValue);
FunctionType functionType = FunctionType functionType = createFunctionTypeFromNodes(n, functionName, info, lValue);
createFunctionTypeFromNodes(n, functionName, info, lValue);


// Assigning the function type to the function node // Assigning the function type to the function node
setDeferredType(n, functionType); setDeferredType(n, functionType);
Expand Down Expand Up @@ -967,6 +964,7 @@ private boolean shouldUseFunctionLiteralType(
if (lValue != null && NodeUtil.isObjectLitKey(lValue)) { if (lValue != null && NodeUtil.isObjectLitKey(lValue)) {
return false; return false;
} }
// TODO(johnlenz): consider unifying global and local behavior
return isLValueRootedInGlobalScope(lValue) || !type.isReturnTypeInferred(); return isLValueRootedInGlobalScope(lValue) || !type.isReturnTypeInferred();
} }


Expand Down Expand Up @@ -1000,9 +998,7 @@ private FunctionType createFunctionTypeFromNodes(
FunctionType functionType = null; FunctionType functionType = null;
if (rValue != null if (rValue != null
&& rValue.isQualifiedName() && rValue.isQualifiedName()
&& lvalueNode != null && lvalueNode != null) {
// TODO(sdh): Try deleting this global scope check and see what happens.
&& isLValueRootedInGlobalScope(lvalueNode)) {
TypedVar var = currentScope.getVar(rValue.getQualifiedName()); TypedVar var = currentScope.getVar(rValue.getQualifiedName());
if (var != null && var.getType() != null && var.getType().isFunctionType()) { if (var != null && var.getType() != null && var.getType().isFunctionType()) {
FunctionType aliasedType = var.getType().toMaybeFunctionType(); FunctionType aliasedType = var.getType().toMaybeFunctionType();
Expand All @@ -1012,7 +1008,7 @@ && isLValueRootedInGlobalScope(lvalueNode)) {


// TODO(nick): Remove this. This should already be handled by normal type resolution. // TODO(nick): Remove this. This should already be handled by normal type resolution.
if (name != null) { if (name != null) {
typeRegistry.declareType(name, functionType.getInstanceType()); typeRegistry.declareType(currentScope, name, functionType.getInstanceType());
} }
} }
} }
Expand Down Expand Up @@ -1187,10 +1183,8 @@ private FunctionType findOverriddenFunction(
* @param rValue The node of the enum. * @param rValue The node of the enum.
* @param name The enum's name * @param name The enum's name
* @param info The {@link JSDocInfo} attached to the enum definition. * @param info The {@link JSDocInfo} attached to the enum definition.
* @param global Whether to declare the type in the global type registry.
*/ */
private EnumType createEnumTypeFromNodes( private EnumType createEnumTypeFromNodes(Node rValue, String name, JSDocInfo info) {
Node rValue, String name, JSDocInfo info, boolean global) {
checkNotNull(info); checkNotNull(info);
checkState(info.hasEnumParameterType()); checkState(info.hasEnumParameterType());


Expand Down Expand Up @@ -1219,8 +1213,8 @@ private EnumType createEnumTypeFromNodes(
} }
} }


if (name != null && global) { if (name != null) {
typeRegistry.declareType(name, enumType.getElementsType()); typeRegistry.declareType(currentScope, name, enumType.getElementsType());
} }


return enumType; return enumType;
Expand Down Expand Up @@ -1512,8 +1506,7 @@ && shouldUseFunctionLiteralType(
if (rValue != null && rValue.isObjectLit()) { if (rValue != null && rValue.isObjectLit()) {
return rValue.getJSType(); return rValue.getJSType();
} else { } else {
return createEnumTypeFromNodes( return createEnumTypeFromNodes(rValue, lValue.getQualifiedName(), info);
rValue, lValue.getQualifiedName(), info, isLValueRootedInGlobalScope(lValue));
} }
} else if (info.isConstructorOrInterface()) { } else if (info.isConstructorOrInterface()) {
return createFunctionTypeFromNodes(rValue, lValue.getQualifiedName(), info, lValue); return createFunctionTypeFromNodes(rValue, lValue.getQualifiedName(), info, lValue);
Expand All @@ -1531,7 +1524,8 @@ && shouldUseFunctionLiteralType(
&& rValueType.isFunctionType() && rValueType.isFunctionType()
&& rValueType.toMaybeFunctionType().hasInstanceType()) { && rValueType.toMaybeFunctionType().hasInstanceType()) {
FunctionType functionType = rValueType.toMaybeFunctionType(); FunctionType functionType = rValueType.toMaybeFunctionType();
typeRegistry.declareType(lValue.getQualifiedName(), functionType.getInstanceType()); typeRegistry.declareType(
currentScope, lValue.getQualifiedName(), functionType.getInstanceType());
} }
return rValueType; return rValueType;
} }
Expand Down Expand Up @@ -1669,11 +1663,10 @@ private void checkForClassDefiningCalls(Node n) {
} }
} }


String singletonGetterClassName = String singletonGetterClassName = codingConvention.getSingletonGetterClassName(n);
codingConvention.getSingletonGetterClassName(n);
if (singletonGetterClassName != null) { if (singletonGetterClassName != null) {
ObjectType objectType = ObjectType.cast( ObjectType objectType = ObjectType.cast(
typeRegistry.getType(singletonGetterClassName)); typeRegistry.getType(currentScope, singletonGetterClassName));
if (objectType != null) { if (objectType != null) {
FunctionType functionType = objectType.getConstructor(); FunctionType functionType = objectType.getConstructor();


Expand All @@ -1696,7 +1689,7 @@ private void checkForClassDefiningCalls(Node n) {
if (objectLiteralCast != null) { if (objectLiteralCast != null) {
if (objectLiteralCast.diagnosticType == null) { if (objectLiteralCast.diagnosticType == null) {
ObjectType type = ObjectType.cast( ObjectType type = ObjectType.cast(
typeRegistry.getType(objectLiteralCast.typeName)); typeRegistry.getType(currentScope, objectLiteralCast.typeName));
if (type != null && type.getConstructor() != null) { if (type != null && type.getConstructor() != null) {
setDeferredType(objectLiteralCast.objectNode, type); setDeferredType(objectLiteralCast.objectNode, type);
objectLiteralCast.objectNode.putBooleanProp( objectLiteralCast.objectNode.putBooleanProp(
Expand All @@ -1716,12 +1709,14 @@ private void checkForClassDefiningCalls(Node n) {
private void applyDelegateRelationship( private void applyDelegateRelationship(
DelegateRelationship delegateRelationship) { DelegateRelationship delegateRelationship) {
ObjectType delegatorObject = ObjectType.cast( ObjectType delegatorObject = ObjectType.cast(
typeRegistry.getType(delegateRelationship.delegator)); typeRegistry.getType(currentScope, delegateRelationship.delegator));
ObjectType delegateBaseObject = ObjectType.cast( ObjectType delegateBaseObject = ObjectType.cast(
typeRegistry.getType(delegateRelationship.delegateBase)); typeRegistry.getType(currentScope, delegateRelationship.delegateBase));
ObjectType delegateSuperObject = ObjectType.cast( ObjectType delegateSuperObject = ObjectType.cast(
typeRegistry.getType(codingConvention.getDelegateSuperclassName())); typeRegistry.getType(currentScope, codingConvention.getDelegateSuperclassName()));
if (delegatorObject != null && delegateBaseObject != null && delegateSuperObject != null) { if (delegatorObject != null
&& delegateBaseObject != null
&& delegateSuperObject != null) {
FunctionType delegatorCtor = delegatorObject.getConstructor(); FunctionType delegatorCtor = delegatorObject.getConstructor();
FunctionType delegateBaseCtor = delegateBaseObject.getConstructor(); FunctionType delegateBaseCtor = delegateBaseObject.getConstructor();
FunctionType delegateSuperCtor = delegateSuperObject.getConstructor(); FunctionType delegateSuperCtor = delegateSuperObject.getConstructor();
Expand Down Expand Up @@ -1768,9 +1763,7 @@ private void applyDelegateRelationship(
*/ */
void maybeDeclareQualifiedName(NodeTraversal t, JSDocInfo info, void maybeDeclareQualifiedName(NodeTraversal t, JSDocInfo info,
Node n, Node parent, Node rhsValue) { Node n, Node parent, Node rhsValue) {
if (currentHoistScope.isGlobal()) { checkForTypedef(n, info);
checkForTypedef(n, info);
}


Node ownerNode = n.getFirstChild(); Node ownerNode = n.getFirstChild();
String ownerName = ownerNode.getQualifiedName(); String ownerName = ownerNode.getQualifiedName();
Expand Down Expand Up @@ -2055,17 +2048,16 @@ private void checkForTypedef(Node candidate, JSDocInfo info) {
// TODO(nicksantos|user): This is a terrible, terrible hack // TODO(nicksantos|user): This is a terrible, terrible hack
// to bail out on recursive typedefs. We'll eventually need // to bail out on recursive typedefs. We'll eventually need
// to handle these properly. // to handle these properly.
typeRegistry.declareType(typedef, unknownType); typeRegistry.declareType(currentScope, typedef, unknownType);


JSType realType = info.getTypedefType().evaluate(currentScope, typeRegistry); JSType realType = info.getTypedefType().evaluate(currentScope, typeRegistry);
if (realType == null) { if (realType == null) {
report(JSError.make(candidate, MALFORMED_TYPEDEF, typedef)); report(JSError.make(candidate, MALFORMED_TYPEDEF, typedef));
} }


typeRegistry.overwriteDeclaredType(typedef, realType); typeRegistry.overwriteDeclaredType(currentScope, typedef, realType);
if (candidate.isGetProp()) { if (candidate.isGetProp()) {
defineSlot(candidate, candidate.getParent(), defineSlot(candidate, candidate.getParent(), getNativeType(NO_TYPE), false);
getNativeType(NO_TYPE), false);
} }
} }
} }
Expand Down
14 changes: 0 additions & 14 deletions src/com/google/javascript/rhino/TypeIRegistry.java
View file
Expand Up @@ -96,20 +96,6 @@ public interface TypeIRegistry extends Serializable {
*/ */
TypeI getType(StaticScope scope, String typeName); TypeI getType(StaticScope scope, String typeName);


/**
* Returns the type represented by typeName or null if not found.
*
* If you pass Foo to this method, and Foo can be an instance or a constructor,
* you get the Foo instance, in contrast to TypeIEnv#getNamespaceType,
* where you'd get the Foo constructor.
*
* If Foo is not a nominal type, returns the namespace type.
*
* @deprecated Use #getGlobalType instead.
*/
@Deprecated
TypeI getType(String typeName);

/** /**
* Returns the type represented by typeName or null if not found. * Returns the type represented by typeName or null if not found.
* *
Expand Down

0 comments on commit d8b84be

Please sign in to comment.