Skip to content

Commit

Permalink
[NTI] When calling a method of a generic class using .call or .apply,…
Browse files Browse the repository at this point in the history 
… don't use

the uninstantiated type variables; they result in wrong warnings.

Fixes issue #2445 on github.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=153797877
  • Loading branch information
@dimvar
dimvar committed Apr 21, 2017
1 parent 320b3f8 commit fd9db66
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 9 deletions.
52 changes: 45 additions & 7 deletions src/com/google/javascript/jscomp/newtypes/FunctionType.java
View file
Expand Up @@ -16,7 +16,10 @@

package com.google.javascript.jscomp.newtypes;

import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
Expand Down Expand Up @@ -329,12 +332,33 @@ public JSType getThisType() {
return this.receiverType != null ? this.receiverType : this.nominalType;
}

/**
* Say a method f is defined on a generic type Foo>T<.
* When doing Foo.prototype.f.call (or also .apply), we transform the method type to a
* function F, which includes the type variables of Foo, in this case T.
*/
private FunctionTypeBuilder transformCallApplyHelper() {
FunctionTypeBuilder builder = new FunctionTypeBuilder(this.commonTypes);
if (this.receiverType == null) {
builder.addReqFormal(this.commonTypes.UNKNOWN);
return builder;
}
NominalType nt = this.receiverType.getNominalTypeIfSingletonObj();
if (nt != null && nt.isUninstantiatedGenericType()) {
builder.addTypeParameters(nt.getTypeParameters());
NominalType ntWithIdentity = nt.instantiateGenericsWithIdentity();
builder.addReqFormal(JSType.fromObjectType(ObjectType.fromNominalType(ntWithIdentity)));
} else {
builder.addReqFormal(this.receiverType);
}
return builder;
}

public FunctionType transformByCallProperty() {
if (isTopFunction() || isQmarkFunction() || isLoose) {
return this.commonTypes.QMARK_FUNCTION;
}
FunctionTypeBuilder builder = new FunctionTypeBuilder(this.commonTypes);
builder.addReqFormal(this.receiverType == null ? this.commonTypes.UNKNOWN : this.receiverType);
FunctionTypeBuilder builder = transformCallApplyHelper();
for (JSType type : this.requiredFormals) {
builder.addReqFormal(type);
}
Expand All @@ -343,7 +367,7 @@ public FunctionType transformByCallProperty() {
}
builder.addRestFormals(this.restFormals);
builder.addRetType(this.returnType);
builder.addTypeParameters(this.typeParameters);
builder.appendTypeParameters(this.typeParameters);
builder.addAbstract(this.isAbstract);
return builder.buildFunction();
}
Expand All @@ -358,8 +382,7 @@ public FunctionType transformByApplyProperty() {
if (isGeneric()) {
return instantiateGenericsWithUnknown().transformByApplyProperty();
}
FunctionTypeBuilder builder = new FunctionTypeBuilder(this.commonTypes);
builder.addReqFormal(this.receiverType == null ? this.commonTypes.UNKNOWN : this.receiverType);
FunctionTypeBuilder builder = transformCallApplyHelper();
JSType arrayContents;
if (getMaxArityWithoutRestFormals() == 0 && hasRestFormals()) {
arrayContents = getRestFormalsType();
Expand Down Expand Up @@ -1045,8 +1068,7 @@ FunctionType substituteGenerics(Map<String, JSType> concreteTypes) {
return substituteNominalGenerics(concreteTypes);
}
ImmutableMap.Builder<String, JSType> builder = ImmutableMap.builder();
for (Map.Entry<String, JSType> concreteTypeEntry
: concreteTypes.entrySet()) {
for (Map.Entry<String, JSType> concreteTypeEntry : concreteTypes.entrySet()) {
if (!typeParameters.contains(concreteTypeEntry.getKey())) {
builder.put(concreteTypeEntry);
}
Expand Down Expand Up @@ -1139,6 +1161,17 @@ public String toString() {
return appendTo(new StringBuilder()).toString();
}

private static Collection<String> getPrettyTypeParams(List<String> typeParams) {
return Collections2.transform(
typeParams,
new Function<String, String>() {
@Override
public String apply(String typeParam) {
return UniqueNameGenerator.getOriginalName(typeParam);
}
});
}

public StringBuilder appendTo(StringBuilder builder) {
if (this == this.commonTypes.LOOSE_TOP_FUNCTION) {
return builder.append("LOOSE_TOP_FUNCTION");
Expand All @@ -1147,6 +1180,11 @@ public StringBuilder appendTo(StringBuilder builder) {
} else if (isQmarkFunction()) {
return builder.append("Function");
}
if (!this.typeParameters.isEmpty()) {
builder.append("<");
builder.append(Joiner.on(",").join(getPrettyTypeParams(this.typeParameters)));
builder.append(">");
}
builder.append("function(");
if (nominalType != null) {
builder.append("new:");
Expand Down
12 changes: 10 additions & 2 deletions src/com/google/javascript/jscomp/newtypes/FunctionTypeBuilder.java
View file
Expand Up @@ -129,14 +129,22 @@ public FunctionTypeBuilder addNominalType(JSType t) {
return this;
}

public FunctionTypeBuilder addTypeParameters(
ImmutableList<String> typeParameters) {
public FunctionTypeBuilder addTypeParameters(ImmutableList<String> typeParameters) {
Preconditions.checkNotNull(typeParameters);
Preconditions.checkState(this.typeParameters.isEmpty());
this.typeParameters = typeParameters;
return this;
}

public FunctionTypeBuilder appendTypeParameters(ImmutableList<String> typeParameters) {
Preconditions.checkNotNull(typeParameters);
ImmutableList.Builder<String> newTypeParams = ImmutableList.builder();
newTypeParams.addAll(this.typeParameters);
newTypeParams.addAll(typeParameters);
this.typeParameters = newTypeParams.build();
return this;
}

public FunctionTypeBuilder addReceiverType(JSType t) {
// this.receiverType is not always null here, because of prototype methods
// with an explicit @this annotation
Expand Down
13 changes: 13 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/NominalType.java
View file
Expand Up @@ -98,6 +98,10 @@ Map<String, JSType> getTypeMap() {
return typeMap;
}

ImmutableList<String> getTypeParameters() {
return this.rawType.getTypeParameters();
}

JSType getIndexType() {
if (isIObject()) {
return this.typeMap.get(this.rawType.getTypeParameters().get(0));
Expand Down Expand Up @@ -255,6 +259,15 @@ NominalType instantiateGenericsWithUnknown() {
return thisWithoutTypemap.instantiateGenerics(getCommonTypes().MAP_TO_UNKNOWN);
}

NominalType instantiateGenericsWithIdentity() {
Preconditions.checkState(isUninstantiatedGenericType());
Map<String, JSType> m = new LinkedHashMap<>();
for (String typeParam : this.getTypeParameters()) {
m.put(typeParam, JSType.fromTypeVar(this.getCommonTypes(), typeParam));
}
return instantiateGenerics(m);
}

public String getName() {
return this.rawType.name;
}
Expand Down
67 changes: 67 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
View file
Expand Up @@ -19925,4 +19925,71 @@ public void testAllowConstructorWithReturnToBeTypedAsFunction() {
"}",
"f(Foo);"));
}

public void testInstantiateTypeVariablesToUnknownWhenUsingCallApply() {
typeCheck(LINE_JOINER.join(
"/**",
" * @constructor",
" * @template T",
" */",
"function Foo() {}",
"/** @return {!Foo<T>} */",
"Foo.prototype.method = function() { return this; }",
"function f(/** !Foo<number> */ x) {",
" var /** !Foo<number> */ y = Foo.prototype.method.call(x);",
"}"));

typeCheck(LINE_JOINER.join(
"/**",
" * @constructor",
" * @template T",
" */",
"function Foo() {}",
"/** @return {!Foo<T>} */",
"Foo.prototype.method = function() { return this; }",
"function f(/** !Foo<number> */ x) {",
" var /** !Foo<number> */ y = Foo.prototype.method.apply(x);",
"}"));

typeCheck(LINE_JOINER.join(
"/**",
" * @constructor",
" * @template T",
" */",
"function Foo() {}",
"/** @return {!Foo<T>} */",
"Foo.prototype.method = function() { return this; }",
"function f(/** !Foo<number> */ x) {",
" var /** !Foo<string> */ y = Foo.prototype.method.call(x);",
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck(LINE_JOINER.join(
"/**",
" * @constructor",
" * @template T",
" */",
"function Foo() {}",
"/** @return {!Foo<T>} */",
"Foo.prototype.method = function() { return this; }",
"function f(/** !Foo<number> */ x) {",
" var /** !Foo<string> */ y = Foo.prototype.method.apply(x);",
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck(LINE_JOINER.join(
"/**",
" * @constructor",
" * @template T",
" */",
"function Foo() {}",
"/**",
" * @template U",
" * @param {U} x",
" * @return {U}",
" */",
"Foo.prototype.f = function(x) { return x; }",
"var /** number */ n = Foo.prototype.f.call(new Foo, 'asdf');"),
NewTypeInference.MISTYPED_ASSIGN_RHS);
}
}
21 changes: 21 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceWithTranspilationTest.java
View file
Expand Up @@ -299,6 +299,27 @@ public void testSuper() {
" }",
"}"),
NewTypeInference.INVALID_ARGUMENT_TYPE);

typeCheck(LINE_JOINER.join(
"/** @template T */",
"class Collection {",
" constructor() {",
" /** @type {!Array<T>} */",
" this.items = [];",
" }",
" /** @param {T} item */",
" add(item) { this.items.push(item); }",
"}",
"/** @extends {Collection<number>} */",
"class NumberCollection extends Collection {",
" constructor() {",
" super();",
" }",
" /** @override */",
" add(item) {",
" super.add(item);",
" }",
"}"));
}

public void testDefaultValuesForArguments() {
Expand Down

0 comments on commit fd9db66

Please sign in to comment.