From 70c200d2659f47c20af45d4c93ff594c3629a709 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Wed, 9 Dec 2015 13:03:48 +0300 Subject: [PATCH] Fix diagnostic messages for properties. Fix IDE tests. Update quick fixes in IDE (TODO: review new inheritance-related diagnostics with regards to quick fixes) --- .../jetbrains/kotlin/diagnostics/Errors.java | 4 ++ .../rendering/DefaultErrorMessages.java | 6 ++- .../kotlin/resolve/OverrideResolver.java | 44 +++++++++++++------ .../TypeMismatchOnOverrideWithSyntaxErrors.kt | 2 +- .../clashesOnInheritance/varTypeMismatch.kt | 12 ++--- .../diagnostics/tests/subtyping/kt3159.kt | 2 +- .../idea/highlighter/IdeErrorMessages.java | 10 ++++- .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 1 + .../checker/ReturnTypeMismatchOnOverride.kt | 7 ++- ...ChangeMultipleOverriddenPropertiesTypes.kt | 2 +- ...ChangeOverriddenPropertyTypeToErrorType.kt | 2 +- 11 files changed, 61 insertions(+), 31 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index f8746588123..76c73f03667 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -323,11 +323,15 @@ public interface Errors { DiagnosticFactory2.create(ERROR, DECLARATION_RETURN_TYPE); DiagnosticFactory2 PROPERTY_TYPE_MISMATCH_ON_OVERRIDE = DiagnosticFactory2.create(ERROR, DECLARATION_RETURN_TYPE); + DiagnosticFactory2 VAR_TYPE_MISMATCH_ON_OVERRIDE = + DiagnosticFactory2.create(ERROR, DECLARATION_RETURN_TYPE); DiagnosticFactory2 RETURN_TYPE_MISMATCH_ON_INHERITANCE = DiagnosticFactory2.create(ERROR, DECLARATION_NAME); DiagnosticFactory2 PROPERTY_TYPE_MISMATCH_ON_INHERITANCE = DiagnosticFactory2.create(ERROR, DECLARATION_NAME); + DiagnosticFactory2 VAR_TYPE_MISMATCH_ON_INHERITANCE = + DiagnosticFactory2.create(ERROR, DECLARATION_NAME); DiagnosticFactory2 OVERRIDING_FINAL_MEMBER_BY_DELEGATION = DiagnosticFactory2.create(ERROR, DECLARATION_NAME); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 7e52fea50fd..69ae44a6d5c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -558,10 +558,14 @@ public class DefaultErrorMessages { MAP.put(RETURN_TYPE_MISMATCH_ON_INHERITANCE, "''{0}'' clashes with ''{1}'': return types are incompatible", SHORT_NAMES_IN_TYPES, SHORT_NAMES_IN_TYPES); - MAP.put(PROPERTY_TYPE_MISMATCH_ON_OVERRIDE, "Type of ''{0}'' doesn''t match the type of the overridden var-property ''{1}''", + MAP.put(PROPERTY_TYPE_MISMATCH_ON_OVERRIDE, "Type of ''{0}'' is not a subtype of the overridden property ''{1}''", + NAME, FQ_NAMES_IN_TYPES); + MAP.put(VAR_TYPE_MISMATCH_ON_OVERRIDE, "Type of ''{0}'' doesn''t match the type of the overridden var-property ''{1}''", NAME, FQ_NAMES_IN_TYPES); MAP.put(PROPERTY_TYPE_MISMATCH_ON_INHERITANCE, "''{0}'' clashes with ''{1}'': property types are incompatible", SHORT_NAMES_IN_TYPES, SHORT_NAMES_IN_TYPES); + MAP.put(VAR_TYPE_MISMATCH_ON_INHERITANCE, "''{0}'' clashes with ''{1}'': property types do not match", + SHORT_NAMES_IN_TYPES, SHORT_NAMES_IN_TYPES); MAP.put(OVERRIDING_FINAL_MEMBER_BY_DELEGATION, "''{0}'' implicitly overrides a final member ''{1}'' by delegation", SHORT_NAMES_IN_TYPES, SHORT_NAMES_IN_TYPES); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java index 4369aac58c6..a12795889a4 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java @@ -286,7 +286,7 @@ public class OverrideResolver { void multipleImplementationsMemberNotImplemented(CallableMemberDescriptor descriptor); void conflictingInterfaceMemberNotImplemented(CallableMemberDescriptor descriptor); void returnTypeMismatchOnInheritance(CallableMemberDescriptor descriptor1, CallableMemberDescriptor descriptor2); - void propertyTypeMismatchOnInheritance(CallableMemberDescriptor descriptor1, CallableMemberDescriptor descriptor2); + void propertyTypeMismatchOnInheritance(PropertyDescriptor descriptor1, PropertyDescriptor descriptor2); } private static class CollectMissingImplementationsStrategy implements CheckInheritedSignaturesReportStrategy { @@ -318,7 +318,7 @@ public class OverrideResolver { } @Override - public void propertyTypeMismatchOnInheritance(CallableMemberDescriptor descriptor1, CallableMemberDescriptor descriptor2) { + public void propertyTypeMismatchOnInheritance(PropertyDescriptor descriptor1, PropertyDescriptor descriptor2) { // don't care } } @@ -373,11 +373,16 @@ public class OverrideResolver { } @Override - public void propertyTypeMismatchOnInheritance(CallableMemberDescriptor descriptor1, CallableMemberDescriptor descriptor2) { + public void propertyTypeMismatchOnInheritance(PropertyDescriptor descriptor1, PropertyDescriptor descriptor2) { conflictingReturnTypes.add(descriptor1); conflictingReturnTypes.add(descriptor2); - reportInheritanceConflictIfRequired(PROPERTY_TYPE_MISMATCH_ON_INHERITANCE, descriptor1, descriptor2); + if (descriptor1.isVar() || descriptor2.isVar()) { + reportInheritanceConflictIfRequired(VAR_TYPE_MISMATCH_ON_INHERITANCE, descriptor1, descriptor2); + } + else { + reportInheritanceConflictIfRequired(PROPERTY_TYPE_MISMATCH_ON_INHERITANCE, descriptor1, descriptor2); + } } private void reportInheritanceConflictIfRequired( @@ -406,8 +411,8 @@ public class OverrideResolver { @Override public void propertyTypeMismatchOnOverride( - @NotNull CallableMemberDescriptor overriding, - @NotNull CallableMemberDescriptor overridden + @NotNull PropertyDescriptor overriding, + @NotNull PropertyDescriptor overridden ) { // Always reported as PROPERTY_TYPE_MISMATCH_ON_INHERITANCE } @@ -732,7 +737,7 @@ public class OverrideResolver { private interface CheckOverrideReportStrategy { void overridingFinalMember(@NotNull CallableMemberDescriptor overriding, @NotNull CallableMemberDescriptor overridden); void returnTypeMismatchOnOverride(@NotNull CallableMemberDescriptor overriding, @NotNull CallableMemberDescriptor overridden); - void propertyTypeMismatchOnOverride(@NotNull CallableMemberDescriptor overriding, @NotNull CallableMemberDescriptor overridden); + void propertyTypeMismatchOnOverride(@NotNull PropertyDescriptor overriding, @NotNull PropertyDescriptor overridden); void varOverriddenByVal(@NotNull CallableMemberDescriptor overriding, @NotNull CallableMemberDescriptor overridden); void cannotOverrideInvisibleMember(@NotNull CallableMemberDescriptor overriding, @NotNull CallableMemberDescriptor invisibleOverridden); void nothingToOverride(@NotNull CallableMemberDescriptor overriding); @@ -782,10 +787,15 @@ public class OverrideResolver { } @Override - public void propertyTypeMismatchOnOverride(@NotNull CallableMemberDescriptor overriding, @NotNull CallableMemberDescriptor overridden) { + public void propertyTypeMismatchOnOverride(@NotNull PropertyDescriptor overriding, @NotNull PropertyDescriptor overridden) { if (!typeMismatchError) { typeMismatchError = true; - trace.report(PROPERTY_TYPE_MISMATCH_ON_OVERRIDE.on(member, declared, overridden)); + if (overridden.isVar()) { + trace.report(VAR_TYPE_MISMATCH_ON_OVERRIDE.on(member, declared, overridden)); + } + else { + trace.report(PROPERTY_TYPE_MISMATCH_ON_OVERRIDE.on(member, declared, overridden)); + } } } @@ -829,7 +839,7 @@ public class OverrideResolver { PropertyDescriptor inheritedPropertyDescriptor = (PropertyDescriptor) inheritedDescriptor; if (!isPropertyTypeOkForOverride(inheritedPropertyDescriptor, mostSpecificProperty)) { - reportingStrategy.propertyTypeMismatchOnInheritance(mostSpecific, inheritedDescriptor); + reportingStrategy.propertyTypeMismatchOnInheritance(mostSpecificProperty, inheritedPropertyDescriptor); } } else if (!isReturnTypeOkForOverride(inheritedDescriptor, mostSpecific)) { @@ -868,6 +878,9 @@ public class OverrideResolver { @NotNull Collection overriddenDescriptors, @NotNull CheckOverrideReportStrategy reportError ) { + PropertyDescriptor propertyMemberDescriptor = + memberDescriptor instanceof PropertyDescriptor ? (PropertyDescriptor) memberDescriptor : null; + for (CallableMemberDescriptor overridden : overriddenDescriptors) { if (overridden == null) continue; @@ -875,9 +888,12 @@ public class OverrideResolver { reportError.overridingFinalMember(memberDescriptor, overridden); } - if (memberDescriptor instanceof PropertyDescriptor && - !isPropertyTypeOkForOverride((PropertyDescriptor) overridden, (PropertyDescriptor) memberDescriptor)) { - reportError.propertyTypeMismatchOnOverride(memberDescriptor, overridden); + if (propertyMemberDescriptor != null) { + assert overridden instanceof PropertyDescriptor : overridden + " is overridden by property " + propertyMemberDescriptor; + PropertyDescriptor overriddenProperty = (PropertyDescriptor) overridden; + if (!isPropertyTypeOkForOverride(overriddenProperty, propertyMemberDescriptor)) { + reportError.propertyTypeMismatchOnOverride(propertyMemberDescriptor, overriddenProperty); + } } else if (!isReturnTypeOkForOverride(overridden, memberDescriptor)) { reportError.returnTypeMismatchOnOverride(memberDescriptor, overridden); @@ -966,7 +982,7 @@ public class OverrideResolver { } @Override - public void propertyTypeMismatchOnOverride(@NotNull CallableMemberDescriptor overriding, @NotNull CallableMemberDescriptor overridden) { + public void propertyTypeMismatchOnOverride(@NotNull PropertyDescriptor overriding, @NotNull PropertyDescriptor overridden) { throw new IllegalStateException("Component functions are not properties"); } diff --git a/compiler/testData/diagnostics/tests/TypeMismatchOnOverrideWithSyntaxErrors.kt b/compiler/testData/diagnostics/tests/TypeMismatchOnOverrideWithSyntaxErrors.kt index e7f2e42d1a1..2c935f8af4c 100644 --- a/compiler/testData/diagnostics/tests/TypeMismatchOnOverrideWithSyntaxErrors.kt +++ b/compiler/testData/diagnostics/tests/TypeMismatchOnOverrideWithSyntaxErrors.kt @@ -4,6 +4,6 @@ abstract class A { } abstract class C : A() { - override abstract var x: String = ? + override abstract var x: String = ? override abstract fun foo(): String = ? } diff --git a/compiler/testData/diagnostics/tests/override/clashesOnInheritance/varTypeMismatch.kt b/compiler/testData/diagnostics/tests/override/clashesOnInheritance/varTypeMismatch.kt index 68ac74ecb34..6b3b1c5bcaa 100644 --- a/compiler/testData/diagnostics/tests/override/clashesOnInheritance/varTypeMismatch.kt +++ b/compiler/testData/diagnostics/tests/override/clashesOnInheritance/varTypeMismatch.kt @@ -14,16 +14,16 @@ interface IGA { var foo: T } -class B1: A(), IA +class B1: A(), IA -class B2: A(), IA, IAA +class B2: A(), IA, IAA -abstract class B3: IA, IAA +abstract class B3: IA, IAA class BS1: A(), IGA -class BS2: A(), IGA +class BS2: A(), IGA -class BS3: A(), IGA +class BS3: A(), IGA -class BG1: A(), IGA +class BG1: A(), IGA diff --git a/compiler/testData/diagnostics/tests/subtyping/kt3159.kt b/compiler/testData/diagnostics/tests/subtyping/kt3159.kt index 6f85c700263..11c335546d3 100644 --- a/compiler/testData/diagnostics/tests/subtyping/kt3159.kt +++ b/compiler/testData/diagnostics/tests/subtyping/kt3159.kt @@ -4,6 +4,6 @@ interface Super { } class Sub: Super { - override var v: String = "fail" + override var v: String = "fail" override val v2: String = "ok" } \ No newline at end of file diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/IdeErrorMessages.java b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/IdeErrorMessages.java index 9e20fb5afca..a1cfb413582 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/IdeErrorMessages.java +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/IdeErrorMessages.java @@ -99,13 +99,19 @@ public class IdeErrorMessages { MAP.put(RETURN_TYPE_MISMATCH_ON_INHERITANCE, "Return types of inherited members are incompatible:
{0},
{1}", DescriptorRenderer.HTML, DescriptorRenderer.HTML); - MAP.put(PROPERTY_TYPE_MISMATCH_ON_OVERRIDE, "Var-property type is ''{0}'', which is not a type of overridden
" + + MAP.put(PROPERTY_TYPE_MISMATCH_ON_OVERRIDE, "Property type is ''{0}'', which is not a subtype type of overridden
" + "{1}", HTML_RENDER_RETURN_TYPE, DescriptorRenderer.HTML); - MAP.put(PROPERTY_TYPE_MISMATCH_ON_INHERITANCE, "Property types of inherited members are incompatible:
{0},
{1}", + MAP.put(VAR_TYPE_MISMATCH_ON_OVERRIDE, "Var-property type is ''{0}'', which is not a type of overridden
" + + "{1}", HTML_RENDER_RETURN_TYPE, DescriptorRenderer.HTML); + MAP.put(PROPERTY_TYPE_MISMATCH_ON_INHERITANCE, "Types of inherited properties are incompatible:
{0},
{1}", + DescriptorRenderer.HTML, DescriptorRenderer.HTML); + MAP.put(VAR_TYPE_MISMATCH_ON_INHERITANCE, "Types of inherited var-properties do not match:
{0},
{1}", DescriptorRenderer.HTML, DescriptorRenderer.HTML); MAP.put(VAR_OVERRIDDEN_BY_VAL, "Val-property cannot override var-property
" + "{1}", DescriptorRenderer.HTML, DescriptorRenderer.HTML); + MAP.put(VAR_OVERRIDDEN_BY_VAL_BY_DELEGATION, "Val-property cannot override var-property
" + + "{1}", DescriptorRenderer.HTML, DescriptorRenderer.HTML); MAP.put(ABSTRACT_MEMBER_NOT_IMPLEMENTED, "{0} must be declared abstract or implement abstract member
" + "{1}", RENDER_CLASS_OR_OBJECT, diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index e495a1a2544..2752ce8d51a 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -215,6 +215,7 @@ public class QuickFixRegistrar : QuickFixContributor { val changeVariableTypeFix = ChangeVariableTypeFix.createFactoryForPropertyOrReturnTypeMismatchOnOverride() RETURN_TYPE_MISMATCH_ON_OVERRIDE.registerFactory(changeVariableTypeFix) PROPERTY_TYPE_MISMATCH_ON_OVERRIDE.registerFactory(changeVariableTypeFix) + VAR_TYPE_MISMATCH_ON_OVERRIDE.registerFactory(changeVariableTypeFix) COMPONENT_FUNCTION_RETURN_TYPE_MISMATCH.registerFactory(ChangeVariableTypeFix.createFactoryForComponentFunctionReturnTypeMismatch()) val changeFunctionReturnTypeFix = ChangeFunctionReturnTypeFix.createFactoryForChangingReturnTypeToUnit() diff --git a/idea/testData/checker/ReturnTypeMismatchOnOverride.kt b/idea/testData/checker/ReturnTypeMismatchOnOverride.kt index ca2b227c06e..959470ad3f4 100644 --- a/idea/testData/checker/ReturnTypeMismatchOnOverride.kt +++ b/idea/testData/checker/ReturnTypeMismatchOnOverride.kt @@ -1,12 +1,11 @@ interface Base { fun foo(): Int - var bar: Int + val qux: Int } class Derived : Base { - override fun foo(): String = "" - - override var bar: String = "" + override var bar: String = "" + override val qux: String = "" } \ No newline at end of file diff --git a/idea/testData/quickfix/override/typeMismatchOnOverride/cantChangeMultipleOverriddenPropertiesTypes.kt b/idea/testData/quickfix/override/typeMismatchOnOverride/cantChangeMultipleOverriddenPropertiesTypes.kt index c70bd46cb9d..2f98a344047 100644 --- a/idea/testData/quickfix/override/typeMismatchOnOverride/cantChangeMultipleOverriddenPropertiesTypes.kt +++ b/idea/testData/quickfix/override/typeMismatchOnOverride/cantChangeMultipleOverriddenPropertiesTypes.kt @@ -1,6 +1,6 @@ // "Change 'A.x' type to '(Int) -> Int'" "false" // ACTION: Change 'C.x' type to '(String) -> Int' -// ERROR: Return type of 'x' is not a subtype of the return type of the overridden member 'public abstract val x: (kotlin.String) -> kotlin.Int defined in A' +// ERROR: Type of 'x' is not a subtype of the overridden property 'public abstract val x: (kotlin.String) -> kotlin.Int defined in A' interface A { val x: (String) -> Int } diff --git a/idea/testData/quickfix/typeMismatch/dontChangeOverriddenPropertyTypeToErrorType.kt b/idea/testData/quickfix/typeMismatch/dontChangeOverriddenPropertyTypeToErrorType.kt index 06646913c1d..be49fe674bb 100644 --- a/idea/testData/quickfix/typeMismatch/dontChangeOverriddenPropertyTypeToErrorType.kt +++ b/idea/testData/quickfix/typeMismatch/dontChangeOverriddenPropertyTypeToErrorType.kt @@ -1,6 +1,6 @@ // "Change 'B.x' type to '(String) -> [ERROR : Ay]'" "false" // ACTION: Change 'A.x' type to '(Int) -> Int' -// ERROR: Return type of 'x' is not a subtype of the return type of the overridden member 'public abstract val x: (kotlin.String) -> [ERROR : Ay] defined in A' +// ERROR: Type of 'x' is not a subtype of the overridden property 'public abstract val x: (kotlin.String) -> [ERROR : Ay] defined in A' // ERROR: Unresolved reference: Ay interface A { val x: (String) -> Ay