Skip to content

Commit

Permalink
[NTI] When a class inherits a method from a grandparent interface ins…
Browse files Browse the repository at this point in the history 
…tead of a parent one, be sure to instantiate generics correctly.

Also, delete some unused code in DeclaredFunctionType.

Fixes diamond-inheritance issue #2242 on github.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=148365005
  • Loading branch information
@dimvar
dimvar authored and Tyler Breisacher committed Feb 27, 2017
1 parent 008d235 commit d6f6b10
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 40 deletions.
30 changes: 22 additions & 8 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
View file
Expand Up @@ -516,15 +516,15 @@ public void process(Node externs, Node root) {
this.compiler.setExternProperties(ImmutableSet.copyOf(this.externPropertyNames));
}

private Collection<PropertyDef> getPropDefsFromInterface(
NominalType nominalType, String pname) {
private Collection<PropertyDef> getPropDefsFromInterface(NominalType nominalType, String pname) {
Preconditions.checkArgument(nominalType.isFinalized());
Preconditions.checkArgument(
nominalType.isInterface() || nominalType.isBuiltinObject());
Preconditions.checkArgument(nominalType.isInterface() || nominalType.isBuiltinObject());
if (nominalType.getPropDeclaredType(pname) == null) {
return ImmutableSet.of();
} else if (propertyDefs.get(nominalType.getId(), pname) != null) {
return ImmutableSet.of(propertyDefs.get(nominalType.getId(), pname));
PropertyDef propDef = propertyDefs.get(nominalType.getId(), pname);
return ImmutableSet.of(
nominalType.isGeneric() ? propDef.substituteNominalGenerics(nominalType) : propDef);
}
ImmutableSet.Builder<PropertyDef> result = ImmutableSet.builder();
for (NominalType interf : nominalType.getInstantiatedInterfaces()) {
Expand All @@ -539,7 +539,8 @@ private PropertyDef getPropDefFromClass(NominalType nominalType, String pname) {
Preconditions.checkArgument(nominalType.isClass());

if (propertyDefs.get(nominalType.getId(), pname) != null) {
return propertyDefs.get(nominalType.getId(), pname);
PropertyDef propDef = propertyDefs.get(nominalType.getId(), pname);
return nominalType.isGeneric() ? propDef.substituteNominalGenerics(nominalType) : propDef;
}
nominalType = nominalType.getInstantiatedSuperclass();
}
Expand Down Expand Up @@ -749,8 +750,7 @@ private void checkSuperProperty(
// If we are looking at a method definition, munging may be needed
for (PropertyDef inheritedPropDef : inheritedPropDefs) {
if (inheritedPropDef.methodType != null) {
propMethodTypesToProcess.put(pname,
inheritedPropDef.methodType.substituteNominalGenerics(superType));
propMethodTypesToProcess.put(pname, inheritedPropDef.methodType);
}
}
}
Expand Down Expand Up @@ -2710,12 +2710,26 @@ private static class PropertyDef {
this.methodScope = methodScope;
}

PropertyDef substituteNominalGenerics(NominalType nt) {
Preconditions.checkArgument(nt.isGeneric(), nt);
if (this.methodType == null) {
return this;
}
return new PropertyDef(
this.defSite, this.methodType.substituteNominalGenerics(nt), this.methodScope);
}

void updateMethodType(DeclaredFunctionType updatedType) {
this.methodType = updatedType;
if (this.methodScope != null) {
this.methodScope.setDeclaredType(updatedType);
}
}

@Override
public String toString() {
return "PropertyDef(" + defSite + ", " + methodType + ")";
}
}

@Override
Expand Down
45 changes: 13 additions & 32 deletions src/com/google/javascript/jscomp/newtypes/DeclaredFunctionType.java
View file
Expand Up @@ -18,7 +18,6 @@

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -287,35 +286,23 @@ public DeclaredFunctionType substituteNominalGenerics(NominalType nt) {
}
Map<String, JSType> typeMap = nt.getTypeMap();
Preconditions.checkState(!typeMap.isEmpty());
Map<String, JSType> reducedMap = typeMap;
boolean foundShadowedTypeParam = false;
// Before switching to unique generated names for type variables, a method's type variables
// could shadow type variables defined on the class. Check that it no longer happens.
for (String typeParam : typeParameters) {
if (typeMap.containsKey(typeParam)) {
foundShadowedTypeParam = true;
break;
}
}
if (foundShadowedTypeParam) {
ImmutableMap.Builder<String, JSType> builder = ImmutableMap.builder();
for (Map.Entry<String, JSType> entry : typeMap.entrySet()) {
if (!typeParameters.contains(entry.getKey())) {
builder.put(entry);
}
}
reducedMap = builder.build();
Preconditions.checkState(!typeMap.containsKey(typeParam));
}
FunctionTypeBuilder builder = new FunctionTypeBuilder(this.commonTypes);
for (JSType reqFormal : requiredFormals) {
builder.addReqFormal(reqFormal == null ? null : reqFormal.substituteGenerics(reducedMap));
builder.addReqFormal(reqFormal == null ? null : reqFormal.substituteGenerics(typeMap));
}
for (JSType optFormal : optionalFormals) {
builder.addOptFormal(optFormal == null ? null : optFormal.substituteGenerics(reducedMap));
builder.addOptFormal(optFormal == null ? null : optFormal.substituteGenerics(typeMap));
}
if (restFormals != null) {
builder.addRestFormals(restFormals.substituteGenerics(reducedMap));
builder.addRestFormals(restFormals.substituteGenerics(typeMap));
}
if (returnType != null) {
builder.addRetType(returnType.substituteGenerics(reducedMap));
builder.addRetType(returnType.substituteGenerics(typeMap));
}
// Explicitly forget nominalType and receiverType. This method is used when
// calculating the declared type of a method using the inherited types.
Expand All @@ -324,8 +311,7 @@ public DeclaredFunctionType substituteNominalGenerics(NominalType nt) {
return builder.buildDeclaration();
}

public static DeclaredFunctionType meet(
Collection<DeclaredFunctionType> toMeet) {
public static DeclaredFunctionType meet(Collection<DeclaredFunctionType> toMeet) {
DeclaredFunctionType result = null;
for (DeclaredFunctionType declType : toMeet) {
if (result == null) {
Expand All @@ -337,29 +323,24 @@ public static DeclaredFunctionType meet(
return result;
}

private static DeclaredFunctionType meet(
DeclaredFunctionType f1, DeclaredFunctionType f2) {
private static DeclaredFunctionType meet(DeclaredFunctionType f1, DeclaredFunctionType f2) {
if (f1.equals(f2)) {
return f1;
}
JSTypes commonTypes = f1.commonTypes;
FunctionTypeBuilder builder = new FunctionTypeBuilder(f1.commonTypes);
int minRequiredArity = Math.min(
f1.requiredFormals.size(), f2.requiredFormals.size());
int minRequiredArity = Math.min(f1.requiredFormals.size(), f2.requiredFormals.size());
for (int i = 0; i < minRequiredArity; i++) {
builder.addReqFormal(nullAcceptingJoin(
f1.getFormalType(i), f2.getFormalType(i)));
builder.addReqFormal(nullAcceptingJoin(f1.getFormalType(i), f2.getFormalType(i)));
}
int maxTotalArity = Math.max(
f1.requiredFormals.size() + f1.optionalFormals.size(),
f2.requiredFormals.size() + f2.optionalFormals.size());
for (int i = minRequiredArity; i < maxTotalArity; i++) {
builder.addOptFormal(nullAcceptingJoin(
f1.getFormalType(i), f2.getFormalType(i)));
builder.addOptFormal(nullAcceptingJoin(f1.getFormalType(i), f2.getFormalType(i)));
}
if (f1.restFormals != null || f2.restFormals != null) {
builder.addRestFormals(
nullAcceptingJoin(f1.restFormals, f2.restFormals));
builder.addRestFormals(nullAcceptingJoin(f1.restFormals, f2.restFormals));
}
JSType retType = nullAcceptingMeet(f1.returnType, f2.returnType);
if (commonTypes.BOTTOM.equals(retType)) {
Expand Down
145 changes: 145 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
View file
Expand Up @@ -19167,4 +19167,149 @@ public void testDontSpecializeTopInInstanceofElseBranch() {
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);
}

public void testMethodInheritedFromGrandparentType() {
typeCheck(LINE_JOINER.join(
"/**",
" * @interface",
" * @template T",
" */",
"function Foo() {}",
"/** @return {T} */",
"Foo.prototype.foo = function() {};",
"/**",
" * @interface",
" * @extends {Foo<T>}",
" * @template T",
" */",
"function Bar() {}",
"/**",
" * @constructor",
" * @implements {Bar<T>}",
" * @param {T} x",
" * @template T",
" */",
"function Baz(x) {",
" this.foo_ = x;",
"};",
"Baz.prototype.foo = function() { return this.foo_; };",
"function f(/** !Baz<number> */ x) {",
" var /** number */ n = x.foo();",
"}"));

typeCheck(LINE_JOINER.join(
"/**",
" * @interface",
" * @template T",
" */",
"function Foo() {}",
"/** @return {T} */",
"Foo.prototype.foo = function() {};",
"/**",
" * @interface",
" * @extends {Foo<T>}",
" * @template T",
" */",
"function Bar() {}",
"/**",
" * @constructor",
" * @implements {Bar<T>}",
" * @param {T} x",
" * @template T",
" */",
"function Baz(x) {",
" this.foo_ = x;",
"};",
"Baz.prototype.foo = function() { return this.foo_; };",
"function f(/** !Baz<number> */ x) {",
" var /** string */ n = x.foo();",
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

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

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

public void testDiamondInheritance() {
typeCheck(LINE_JOINER.join(
"/**",
" * @interface",
" * @template T",
" */",
"function Foo() {}",
"/** @return {T} */",
"Foo.prototype.foo = function() {};",
"/**",
" * @interface",
" * @extends {Foo<T>}",
" * @template T",
" */",
"function Bar() {}",
"/**",
" * @constructor",
" * @abstract",
" * @implements {Foo<T>}",
" * @template T",
" */",
"function AbstractBaz() {}",
"/** @abstract */",
"AbstractBaz.prototype.foo = function() {};",
"/**",
" * @constructor",
" * @implements {Bar<T>}",
" * @extends {AbstractBaz<T>}",
" * @param {T} x",
" * @template T",
" */",
"function Baz(x) {",
" this.foo_ = x;",
"};",
"Baz.prototype.foo = function() { return this.foo_; };"));
}
}

0 comments on commit d6f6b10

Please sign in to comment.