diff --git a/src/com/google/javascript/jscomp/GlobalTypeInfo.java b/src/com/google/javascript/jscomp/GlobalTypeInfo.java index dd2ec86c4f2..d2eaec23c75 100644 --- a/src/com/google/javascript/jscomp/GlobalTypeInfo.java +++ b/src/com/google/javascript/jscomp/GlobalTypeInfo.java @@ -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#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#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); diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java index 3ed1c4d6af8..5d7eb2c5cdd 100644 --- a/src/com/google/javascript/jscomp/NewTypeInference.java +++ b/src/com/google/javascript/jscomp/NewTypeInference.java @@ -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, @@ -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); @@ -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()) { @@ -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 = diff --git a/src/com/google/javascript/jscomp/newtypes/NominalType.java b/src/com/google/javascript/jscomp/newtypes/NominalType.java index 902588b28eb..a5883ed4041 100644 --- a/src/com/google/javascript/jscomp/newtypes/NominalType.java +++ b/src/com/google/javascript/jscomp/newtypes/NominalType.java @@ -107,6 +107,10 @@ JSType getIndexedType() { return foundIObject ? result : null; } + boolean inheritsFromIObjectReflexive() { + return this.rawType.inheritsFromIObjectReflexive(); + } + boolean isClassy() { return !isFunction() && !isBuiltinObject(); } @@ -241,9 +245,9 @@ public JSType getPrototype() { } public ImmutableSet getInstantiatedInterfaces() { - Preconditions.checkState(this.rawType.isFinalized()); ImmutableSet.Builder result = ImmutableSet.builder(); for (NominalType interf : this.rawType.getInterfaces()) { + Preconditions.checkState(interf.rawType.isFinalized()); result.add(interf.instantiateGenerics(typeMap)); } return result.build(); diff --git a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java index 57c4d640b06..177c5936e86 100644 --- a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java +++ b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java @@ -243,14 +243,13 @@ 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; } } @@ -258,6 +257,10 @@ private boolean inheritsFromIObject() { return false; } + public boolean inheritsFromIObject() { + return !isBuiltinWithName("IObject") && inheritsFromIObjectReflexive(); + } + /** @return Whether the interface can be added without creating a cycle. */ public boolean addInterfaces(ImmutableSet interfaces) { Preconditions.checkState(!this.isFinalized); @@ -274,7 +277,7 @@ public boolean addInterfaces(ImmutableSet 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; } } diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java index 8a475c7654b..071f92cd9cc 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java @@ -16150,6 +16150,47 @@ public void testIObjectInheritance() { "function f(/** !Foo */ x, /** string */ s) {", " x[s];", "}")); + + typeCheck(LINE_JOINER.join( + "/** @interface @extends {IObject} */", + "function Int1() {}", + "/** @interface @extends {IObject} */", + "function Int2() {}", + "/**", + " * @constructor", + " * @implements {Int1}", + " * @implements {Int2}", + " */", + "function Foo() {}"), + GlobalTypeInfo.SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES); + + typeCheck(LINE_JOINER.join( + "/** @interface @extends {IObject} */", + "function Int1() {}", + "/** @interface @extends {IObject} */", + "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 Int1() {}", + "/** @interface @extends {IObject} */", + "function Int2() {}", + "/**", + " * @constructor", + " * @implements {Int1}", + " * @implements {Int2}", + " */", + "function Foo() {}"), + GlobalTypeInfo.SUPER_INTERFACES_HAVE_INCOMPATIBLE_PROPERTIES); } public void testDontWarnForMissingReturnOnInfiniteLoop() {