Skip to content

Commit

Permalink
[NTI] Warn when a type is inheriting from incompatible IObject types.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=123344040
  • Loading branch information
dimvar authored and blickly committed May 26, 2016
1 parent 3637562 commit 1274667
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 8 deletions.
20 changes: 20 additions & 0 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
Expand Up @@ -565,6 +565,26 @@ private void checkAndFinalizeNominalType(RawNominalType rawType) {
rawType.addProtoProperty(pname, null, resultType, false);
}

// Warn when inheriting from incompatible IObject types
if (rawType.inheritsFromIObject()) {
JSType wrapped = rawType.getInstanceAsJSType();
if (wrapped.getIndexType() == null) {
warnings.add(JSError.make(
rawType.getDefSite(),
SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES,
rawType.getName(),
"IObject<K,V>#index",
"the keys K have types that can't be joined."));
} else if (wrapped.getIndexedType() == null) {
warnings.add(JSError.make(
rawType.getDefSite(),
SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES,
rawType.getName(),
"IObject<K,V>#index",
"the values V should have a common subtype."));
}
}

// Warn for a prop declared with @override that isn't overriding anything.
for (String pname : nonInheritedPropNames) {
PropertyDef propDef = propertyDefs.get(rawType, pname);
Expand Down
13 changes: 10 additions & 3 deletions src/com/google/javascript/jscomp/NewTypeInference.java
Expand Up @@ -2024,7 +2024,7 @@ private EnvTypePair analyzeGetElemFwd(
pair = analyzeExprFwd(
index, pair.env, indexType.isBottom() ? JSType.UNKNOWN : indexType);
mayWarnAboutBadIObjectIndex(index, recvType, pair.type, indexType);
pair.type = recvType.getIndexedType();
pair.type = getIndexedTypeOrUnknown(recvType);
return pair;
} else if (index.isString()) {
return analyzePropAccessFwd(receiver, index.getString(), inEnv,
Expand Down Expand Up @@ -2831,6 +2831,13 @@ private boolean mayWarnAboutBadIObjectIndex(Node n, JSType iobjectType,
return false;
}

// Don't always wrap getIndexedType calls with this function. Only do it when
// you want to pass around the result type and it has to be non-null.
private JSType getIndexedTypeOrUnknown(JSType t) {
JSType tmp = t.getIndexedType();
return tmp == null ? JSType.UNKNOWN : tmp;
}

private EnvTypePair analyzePropAccessFwd(Node receiver, String pname,
TypeEnv inEnv, JSType requiredType, JSType specializedType) {
QualifiedName propQname = new QualifiedName(pname);
Expand Down Expand Up @@ -3421,7 +3428,7 @@ private EnvTypePair analyzeGetElemBwd(
JSType indexType = recvType.getIndexType();
if (indexType != null) {
pair = analyzeExprBwd(index, pair.env, indexType);
pair.type = recvType.getIndexedType();
pair.type = getIndexedTypeOrUnknown(recvType);
return pair;
}
if (index.isString()) {
Expand Down Expand Up @@ -3822,7 +3829,7 @@ private LValueResultFwd analyzeIObjectElmLvalFwd(
if (mayWarnAboutBadIObjectIndex(prop, recvLvalue.type, pair.type, indexType)) {
return new LValueResultFwd(pair.env, JSType.UNKNOWN, null, null);
}
JSType inferred = recvLvalue.type.getIndexedType();
JSType inferred = getIndexedTypeOrUnknown(recvLvalue.type);
JSType declared = null;
if (recvLvalue.declType != null) {
JSType receiverAdjustedDeclType =
Expand Down
6 changes: 5 additions & 1 deletion src/com/google/javascript/jscomp/newtypes/NominalType.java
Expand Up @@ -107,6 +107,10 @@ JSType getIndexedType() {
return foundIObject ? result : null;
}

boolean inheritsFromIObjectReflexive() {
return this.rawType.inheritsFromIObjectReflexive();
}

boolean isClassy() {
return !isFunction() && !isBuiltinObject();
}
Expand Down Expand Up @@ -241,9 +245,9 @@ public JSType getPrototype() {
}

public ImmutableSet<NominalType> getInstantiatedInterfaces() {
Preconditions.checkState(this.rawType.isFinalized());
ImmutableSet.Builder<NominalType> result = ImmutableSet.builder();
for (NominalType interf : this.rawType.getInterfaces()) {
Preconditions.checkState(interf.rawType.isFinalized());
result.add(interf.instantiateGenerics(typeMap));
}
return result.build();
Expand Down
11 changes: 7 additions & 4 deletions src/com/google/javascript/jscomp/newtypes/RawNominalType.java
Expand Up @@ -243,21 +243,24 @@ boolean hasAncestorInterface(RawNominalType ancestor) {
}
}

private boolean inheritsFromIObject() {
Preconditions.checkState(!this.isFinalized);
boolean inheritsFromIObjectReflexive() {
if (isBuiltinWithName("IObject")) {
return true;
}
if (this.interfaces != null) {
for (NominalType interf : this.interfaces) {
if (interf.getRawNominalType().inheritsFromIObject()) {
if (interf.inheritsFromIObjectReflexive()) {
return true;
}
}
}
return false;
}

public boolean inheritsFromIObject() {
return !isBuiltinWithName("IObject") && inheritsFromIObjectReflexive();
}

/** @return Whether the interface can be added without creating a cycle. */
public boolean addInterfaces(ImmutableSet<NominalType> interfaces) {
Preconditions.checkState(!this.isFinalized);
Expand All @@ -274,7 +277,7 @@ public boolean addInterfaces(ImmutableSet<NominalType> interfaces) {
// TODO(dimvar): When a class extends a class that inherits from IObject,
// it should be unrestricted.
for (NominalType interf : interfaces) {
if (interf.getRawNominalType().inheritsFromIObject()) {
if (interf.getRawNominalType().inheritsFromIObjectReflexive()) {
this.objectKind = ObjectKind.UNRESTRICTED;
}
}
Expand Down
Expand Up @@ -16150,6 +16150,47 @@ public void testIObjectInheritance() {
"function f(/** !Foo */ x, /** string */ s) {",
" x[s];",
"}"));

typeCheck(LINE_JOINER.join(
"/** @interface @extends {IObject<string, string>} */",
"function Int1() {}",
"/** @interface @extends {IObject<string, number>} */",
"function Int2() {}",
"/**",
" * @constructor",
" * @implements {Int1}",
" * @implements {Int2}",
" */",
"function Foo() {}"),
GlobalTypeInfo.SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES);

typeCheck(LINE_JOINER.join(
"/** @interface @extends {IObject<string, string>} */",
"function Int1() {}",
"/** @interface @extends {IObject<string, number>} */",
"function Int2() {}",
"/**",
" * @constructor",
" * @implements {Int1}",
" * @implements {Int2}",
" */",
"function Foo() {}",
// Tests that we don't crash on property accesses of bad IObjects
"var /** null */ n = (new Foo)['asdf'+'asdf'];"),
GlobalTypeInfo.SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES);

typeCheck(LINE_JOINER.join(
"/** @interface @extends {IObject<function(number), number>} */",
"function Int1() {}",
"/** @interface @extends {IObject<function(string), number>} */",
"function Int2() {}",
"/**",
" * @constructor",
" * @implements {Int1}",
" * @implements {Int2}",
" */",
"function Foo() {}"),
GlobalTypeInfo.SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES);
}

public void testDontWarnForMissingReturnOnInfiniteLoop() {
Expand Down

0 comments on commit 1274667

Please sign in to comment.