diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java index 1bf5663907e..d5b671b2546 100644 --- a/src/com/google/javascript/jscomp/TypeCheck.java +++ b/src/com/google/javascript/jscomp/TypeCheck.java @@ -1332,15 +1332,17 @@ private void checkPropertyInheritanceOnClassMember( private void checkPropertyInheritance( Node key, String propertyName, FunctionType ctorType, ObjectType type) { - if (ctorType != null && (ctorType.isConstructor() || ctorType.isInterface())) { - checkDeclaredPropertyAgainstNominalInheritance( - key.getFirstChild(), - ctorType, - propertyName, - key.getJSDocInfo(), - type.getPropertyType(propertyName)); - checkAbstractMethodInConcreteClass(key, ctorType, key.getJSDocInfo()); + if (ctorType == null || !ctorType.hasInstanceType()) { + return; } + + checkDeclaredPropertyAgainstNominalInheritance( + key.getFirstChild(), + ctorType, + propertyName, + key.getJSDocInfo(), + type.getPropertyType(propertyName)); + checkAbstractMethodInConcreteClass(key, ctorType, key.getJSDocInfo()); } /** @@ -1547,17 +1549,13 @@ private void checkDeclaredPropertyAgainstNominalInheritance( boolean foundInterfaceProperty = false; if (ctorType.isConstructor()) { - for (JSType implementedInterface : - ctorType.getAllImplementedInterfaces()) { + for (ObjectType implementedInterface : ctorType.getAllImplementedInterfaces()) { if (implementedInterface.isUnknownType() || implementedInterface.isEmptyType()) { continue; } - FunctionType interfaceType = - implementedInterface.toObjectType().getConstructor(); - checkNotNull(interfaceType); + checkState(implementedInterface.isInstanceType(), implementedInterface); - boolean interfaceHasProperty = - interfaceType.getPrototype().hasProperty(propertyName); + boolean interfaceHasProperty = implementedInterface.hasProperty(propertyName); foundInterfaceProperty = foundInterfaceProperty || interfaceHasProperty; if (!declaredOverride && interfaceHasProperty @@ -1569,7 +1567,7 @@ private void checkDeclaredPropertyAgainstNominalInheritance( n, HIDDEN_INTERFACE_PROPERTY, propertyName, - interfaceType.getTopMostDefiningType(propertyName).toString())); + implementedInterface.getTopDefiningInterface(propertyName).toString())); } } } diff --git a/src/com/google/javascript/jscomp/TypeValidator.java b/src/com/google/javascript/jscomp/TypeValidator.java index ba4154a18c2..4a2ccc95e9a 100644 --- a/src/com/google/javascript/jscomp/TypeValidator.java +++ b/src/com/google/javascript/jscomp/TypeValidator.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.base.Strings.nullToEmpty; import static com.google.javascript.rhino.jstype.JSTypeNative.ARRAY_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.ASYNC_GENERATOR_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.BOOLEAN_TYPE; @@ -877,9 +876,13 @@ TypedVar expectUndeclaredVariable(String sourceName, CompilerInput input, void expectAllInterfaceProperties(Node n, FunctionType type) { ObjectType instance = type.getInstanceType(); for (ObjectType implemented : type.getAllImplementedInterfaces()) { + // Case: `/** @interface */ class Foo { constructor() { this.prop; } }` + for (String prop : implemented.getOwnPropertyNames()) { + expectInterfaceProperty(n, instance, implemented, prop); + } if (implemented.getImplicitPrototype() != null) { - for (String prop : - implemented.getImplicitPrototype().getOwnPropertyNames()) { + // Case: `/** @interface */ class Foo { prop() { } }` + for (String prop : implemented.getImplicitPrototype().getOwnPropertyNames()) { expectInterfaceProperty(n, instance, implemented, prop); } } @@ -894,9 +897,6 @@ private void expectInterfaceProperty( Node n, ObjectType instance, ObjectType implementedInterface, String prop) { StaticTypedSlot propSlot = instance.getSlot(prop); if (propSlot == null) { - // Not implemented - String sourceName = n.getSourceFileName(); - sourceName = nullToEmpty(sourceName); registerMismatch( instance, implementedInterface, @@ -918,8 +918,7 @@ private void expectInterfaceProperty( JSType found = propSlot.getType(); found = found.restrictByNotNullOrUndefined(); - JSType required - = implementedInterface.getImplicitPrototype().getPropertyType(prop); + JSType required = implementedInterface.getPropertyType(prop); TemplateTypeMap typeMap = implementedInterface.getTemplateTypeMap(); if (!typeMap.isEmpty()) { TemplateTypeMapReplacer replacer = new TemplateTypeMapReplacer( @@ -930,15 +929,13 @@ private void expectInterfaceProperty( if (!found.isSubtype(required, this.subtypingMode)) { // Implemented, but not correctly typed - FunctionType constructor = - implementedInterface.toObjectType().getConstructor(); JSError err = JSError.make( propNode, HIDDEN_INTERFACE_PROPERTY_MISMATCH, prop, instance.toString(), - constructor.getTopMostDefiningType(prop).toString(), + implementedInterface.getTopDefiningInterface(prop).toString(), required.toString(), found.toString()); registerMismatch(found, required, err); @@ -962,8 +959,7 @@ void expectAbstractMethodsImplemented(Node n, FunctionType ctorType) { while (currSuperCtor != null && currSuperCtor.isAbstract()) { ObjectType superType = currSuperCtor.getInstanceType(); - for (String prop : - currSuperCtor.getInstanceType().getImplicitPrototype().getOwnPropertyNames()) { + for (String prop : currSuperCtor.getPrototype().getOwnPropertyNames()) { FunctionType maybeAbstractMethod = superType.findPropertyType(prop).toMaybeFunctionType(); if (maybeAbstractMethod != null && maybeAbstractMethod.isAbstract() @@ -980,8 +976,6 @@ void expectAbstractMethodsImplemented(Node n, FunctionType ctorType) { ObjectType superType = entry.getValue(); FunctionType abstractMethod = instance.findPropertyType(method).toMaybeFunctionType(); if (abstractMethod == null || abstractMethod.isAbstract()) { - String sourceName = n.getSourceFileName(); - sourceName = nullToEmpty(sourceName); registerMismatch( instance, superType, diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index 2bca74f1918..a06aacddb6f 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -3001,6 +3001,28 @@ public void testClassMissingOverrideAnnotationForInterfaceMethod() { "property foo already defined on interface Foo; use @override to override it"); } + @Test + public void testClassMissingOverrideAnnotationForInterfaceInstanceProperty() { + testTypes( + lines( + "/** @record */", // `@interface` would also trigger this. + "class Foo {", + " constructor() {", + " /** @type {number} */", + " this.bar;", + " }", + "}", + "", + "/** @implements {Foo} */", + "class MyFoo { }", + // No `@override`. + // For some reason we only check this when assigning to prototype properties, not to + // instance properties. + "/** @type {number} */", + "MyFoo.prototype.bar = 0;"), + "property bar already defined on interface Foo; use @override to override it"); + } + @Test public void testClassIncompatibleInterfaceMethodImplementation() { testTypes( @@ -3034,6 +3056,68 @@ public void testClassMissingTransitiveInterfaceMethod() { "property foo on interface Foo is not implemented by type Baz"); } + @Test + public void testClassMissingInterfaceInstanceProperty() { + testTypes( + lines( + "/** @record */", // `@interface` would also trigger this. + "class Foo {", + " constructor() {", + " /** @type {number} */", + " this.bar;", + " }", + "}", + "", + "/** @implements {Foo} */", + "class MyFoo { }"), + "property bar on interface Foo is not implemented by type MyFoo"); + } + + @Test + public void testClassInvalidOverrideOfInterfaceInstanceProperty() { + testTypes( + lines( + "/** @record */", // `@interface` would also trigger this. + "class Foo {", + " constructor() {", + " /** @type {number} */", + " this.bar;", + " }", + "}", + "", + "/** @implements {Foo} */", + "class MyFoo {", + " constructor() {", + " /** @type {string} */", + " this.bar;", + " }", + "}"), + lines( + "mismatch of the bar property on type MyFoo and the type " + + "of the property it overrides from interface Foo", + "original: number", + "override: string")); + } + + @Test + public void testClassPrototypeOverrideOfInterfaceInstanceProperty() { + testTypes( + lines( + "/** @record */", // `@interface` would also trigger this. + "class Foo {", + " constructor() {", + " /** @type {number} */", + " this.bar;", + " }", + "}", + "", + "/** @implements {Foo} */", + "class MyFoo { }", + // It's legal to fulfill the interface using either instance or prototype properties. + "/** @override */", + "MyFoo.prototype.bar;")); + } + @Test public void testClassInheritedInterfaceMethod() { testTypes( @@ -5575,8 +5659,6 @@ public void testGetterOverridesPrototypePropertyFromInterface() { @Test public void testGetterOverridesInstancePropertyFromInterface() { - // We treat the interface fields in the constructor as different from prototype properties, - // so trying to override the `num` field with a getter doesn't work. testTypes( lines( "/** @interface */", @@ -5591,8 +5673,7 @@ public void testGetterOverridesInstancePropertyFromInterface() { " /** @override */", " get num() { return 3; }", "}", - "var /** string */ x = (new Baz).num;"), - "property num not defined on any superclass of Baz"); + "var /** string */ x = (new Baz).num;")); } @Test diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java index 83a04522af1..6a84f67f240 100644 --- a/test/com/google/javascript/jscomp/TypeCheckTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckTest.java @@ -13242,14 +13242,17 @@ public void testInterfacePropertyNotImplemented2() { @Test public void testInterfacePropertyNotImplemented3() { testTypes( - "/** @interface\n @template T */function Int() {};" - + "/** @return {T} */Int.prototype.foo = function() {};" - + "/** @constructor\n @implements {Int} */function Foo() {};" - + "/** @return {number}\n @override */Foo.prototype.foo = function() {};", - "mismatch of the foo property on type Foo and the type of the property it " - + "overrides from interface Int\n" - + "original: function(this:Int): string\n" - + "override: function(this:Foo): number"); + lines( + "/** @interface @template T */ function Int() {};", + "/** @return {T} */ Int.prototype.foo = function() {};", + "", + "/** @constructor @implements {Int} */ function Foo() {};", + "/** @return {number} @override */ Foo.prototype.foo = function() {};"), + lines( + "mismatch of the foo property on type Foo and the type of the property it " + + "overrides from interface Int", + "original: function(this:Int): string", + "override: function(this:Foo): number")); } @Test