Skip to content

Commit

Permalink
When resolving template types match only against the request type par…
Browse files Browse the repository at this point in the history 
…ameters

and not the entire hierarchy.  This avoids resolving against template types
that are ambiguous in OTI unnecessarily (such as unions of template types).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=161546684
  • Loading branch information
@concavelenz
concavelenz authored and Tyler Breisacher committed Jul 11, 2017
1 parent 1421862 commit 1f80a2f
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 36 deletions.
74 changes: 38 additions & 36 deletions src/com/google/javascript/jscomp/TypeInference.java
View file
Expand Up @@ -53,6 +53,7 @@
import com.google.javascript.rhino.jstype.TemplateType;
import com.google.javascript.rhino.jstype.TemplateTypeMap;
import com.google.javascript.rhino.jstype.TemplateTypeMapReplacer;
import com.google.javascript.rhino.jstype.TemplatizedType;
import com.google.javascript.rhino.jstype.UnionType;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -1215,11 +1216,11 @@ private void maybeResolveTemplatedType(
JSType argType,
Map<TemplateType, JSType> resolvedTypes, Set<JSType> seenTypes) {
if (paramType.isTemplateType()) {
// @param {T}
// example: @param {T}
resolvedTemplateType(
resolvedTypes, paramType.toMaybeTemplateType(), argType);
} else if (paramType.isUnionType()) {
// @param {Array.<T>|NodeList|Arguments|{length:number}}
// example: @param {Array.<T>|NodeList|Arguments|{length:number}}
UnionType unionType = paramType.toMaybeUnionType();
for (JSType alernative : unionType.getAlternates()) {
maybeResolveTemplatedType(alernative, argType, resolvedTypes, seenTypes);
Expand All @@ -1245,7 +1246,7 @@ private void maybeResolveTemplatedType(
argFunctionType.getParameters(), resolvedTypes, seenTypes);
}
} else if (paramType.isRecordType() && !paramType.isNominalType()) {
// @param {{foo:T}}
// example: @param {{foo:T}}
if (seenTypes.add(paramType)) {
ObjectType paramRecordType = paramType.toObjectType();
ObjectType argObjectType = argType.restrictByNotNullOrUndefined().toObjectType();
Expand All @@ -1262,25 +1263,31 @@ private void maybeResolveTemplatedType(
seenTypes.remove(paramType);
}
} else if (paramType.isTemplatizedType()) {
// @param {Array<T>}
ObjectType referencedParamType = paramType
.toMaybeTemplatizedType()
.getReferencedType();
JSType argObjectType = argType
.restrictByNotNullOrUndefined()
.collapseUnion();


if (argObjectType.isSubtype(referencedParamType)) {
// If the argument type is a subtype of the parameter type, resolve any
// template types amongst their templatized types.
TemplateTypeMap paramTypeMap = paramType.getTemplateTypeMap();
TemplateTypeMap argTypeMap = argObjectType.getTemplateTypeMap();
for (TemplateType key : paramTypeMap.getTemplateKeys()) {
maybeResolveTemplatedType(
paramTypeMap.getResolvedTemplateType(key),
argTypeMap.getResolvedTemplateType(key),
resolvedTypes, seenTypes);
// example: @param {Array<T>}
TemplatizedType templatizedParamType = paramType.toMaybeTemplatizedType();
int keyCount = templatizedParamType.getTemplateTypes().size();
// TODO(johnlenz): determine why we are creating TemplatizedTypes for
// types with no type arguments.
if (keyCount > 0) {
ObjectType referencedParamType = templatizedParamType.getReferencedType();
JSType argObjectType = argType
.restrictByNotNullOrUndefined()
.collapseUnion();

if (argObjectType.isSubtype(referencedParamType)) {
// If the argument type is a subtype of the parameter type, resolve any
// template types amongst their templatized types.
TemplateTypeMap paramTypeMap = paramType.getTemplateTypeMap();

ImmutableList<TemplateType> keys = paramTypeMap.getTemplateKeys();
TemplateTypeMap argTypeMap = argObjectType.getTemplateTypeMap();
for (int index = keys.size() - keyCount; index < keys.size(); index++) {
TemplateType key = keys.get(index);
maybeResolveTemplatedType(
paramTypeMap.getResolvedTemplateType(key),
argTypeMap.getResolvedTemplateType(key),
resolvedTypes, seenTypes);
}
}
}
}
Expand Down Expand Up @@ -1402,14 +1409,13 @@ private Map<TemplateType, JSType> evaluateTypeTransformations(
}

/**
* For functions with function(this: T, ...) and T as parameters, type
* inference will set the type of this on a function literal argument to the
* the actual type of T.
* For functions that use template types, specialize the function type for
* the call target based on the call-site specific arguments.
* Specifically, this enables inference to set the type of any function
* literal parameters based on these inferred types.
*/
private boolean inferTemplatedTypesForCall(
Node n, FunctionType fnType) {
final ImmutableList<TemplateType> keys = fnType.getTemplateTypeMap()
.getTemplateKeys();
private boolean inferTemplatedTypesForCall(Node n, FunctionType fnType) {
ImmutableList<TemplateType> keys = fnType.getTemplateTypeMap().getTemplateKeys();
if (keys.isEmpty()) {
return false;
}
Expand All @@ -1426,22 +1432,18 @@ private boolean inferTemplatedTypesForCall(
}

// Try to infer the template types using the type transformations
Map<TemplateType, JSType> typeTransformations =
evaluateTypeTransformations(keys, inferred);
Map<TemplateType, JSType> typeTransformations = evaluateTypeTransformations(keys, inferred);
if (typeTransformations != null) {
inferred.putAll(typeTransformations);
}

// Replace all template types. If we couldn't find a replacement, we
// replace it with UNKNOWN.
TemplateTypeReplacer replacer = new TemplateTypeReplacer(
registry, inferred);
TemplateTypeReplacer replacer = new TemplateTypeReplacer(registry, inferred);
Node callTarget = n.getFirstChild();

FunctionType replacementFnType = fnType.visit(replacer)
.toMaybeFunctionType();
FunctionType replacementFnType = fnType.visit(replacer).toMaybeFunctionType();
checkNotNull(replacementFnType);

callTarget.setJSType(replacementFnType);
n.setJSType(replacementFnType.getReturnType());

Expand Down
34 changes: 34 additions & 0 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
View file
Expand Up @@ -17735,6 +17735,40 @@ public void testNoResolvedTypeDoesntCauseInfiniteLoop() throws Exception {
null);
}

public void testb38182645() throws Exception {
testTypes(
LINE_JOINER.join("",
"/**",
" * @interface",
" * @template VALUE",
" */",
"function MyI() {}",
"",
"",
"/**",
" * @constructor",
" * @implements {MyI<K|V>}",
" * @template K, V",
" */",
"function MyMap() {}",
"",
"",
"/**",
" * @param {!MyMap<string, T>} map",
" * @return {T}",
" * @template T",
" */",
"function getValueFromNameAndMap(map) {",
" return /** @type {?} */ (123);",
"}",
"var m = /** @type {!MyMap<string,number>} */ (new MyMap());",
"var /** null */ n = getValueFromNameAndMap(m);"),
LINE_JOINER.join(
"initializing variable",
"found : number",
"required: null"));
}

private void testTypes(String js) {
testTypes(js, (String) null);
}
Expand Down

0 comments on commit 1f80a2f

Please sign in to comment.