From fbeaaf5fbb175bcc16698279722ec7eaf5d6db5b Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 31 Aug 2012 20:47:17 +0400 Subject: [PATCH] Check componentN functions for override conflicts Check if newly generated component functions happen to override something which they're not supposed to. Create new slice to store mapping from annotation descriptors to corresponding PSI elements, which is used by override checker to report errors in data classes on. --- .../jet/lang/diagnostics/Errors.java | 2 + .../rendering/DefaultErrorMessages.java | 2 + .../jet/lang/resolve/BindingContext.java | 4 +- .../jet/lang/resolve/OverrideResolver.java | 72 ++++++++++++++++++- .../dataClasses/dataClassOverrideConflict.kt | 3 + .../implementTraitWhichHasComponent1.kt | 5 ++ .../implementTraitWhichHasFinalComponent1.kt | 5 ++ .../dataClasses/overrideInvisibleMember.kt | 3 + .../checkers/JetDiagnosticsTestGenerated.java | 20 ++++++ 9 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/dataClasses/dataClassOverrideConflict.kt create mode 100644 compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasComponent1.kt create mode 100644 compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasFinalComponent1.kt create mode 100644 compiler/testData/diagnostics/tests/dataClasses/overrideInvisibleMember.kt diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java index 2912f90fb61..4fcfeb6346b 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java @@ -145,6 +145,8 @@ public interface Errors { DiagnosticFactory3.create(ERROR, NAMED_ELEMENT); DiagnosticFactory3 CANNOT_OVERRIDE_INVISIBLE_MEMBER = DiagnosticFactory3.create(ERROR, OVERRIDE_MODIFIER); + DiagnosticFactory2 DATA_CLASS_OVERRIDE_CONFLICT = + DiagnosticFactory2.create(ERROR); SimpleDiagnosticFactory CANNOT_INFER_VISIBILITY = SimpleDiagnosticFactory.create(ERROR, DECLARATION); DiagnosticFactory1 ENUM_ENTRY_SHOULD_BE_INITIALIZED = DiagnosticFactory1.create(ERROR, NAME_IDENTIFIER); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java index bd9f7355599..4db4e45cbd6 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java @@ -156,6 +156,8 @@ public class DefaultErrorMessages { MAP.put(NOTHING_TO_OVERRIDE, "''{0}'' overrides nothing", NAME); MAP.put(VIRTUAL_MEMBER_HIDDEN, "''{0}'' hides member of supertype ''{2}'' and needs ''override'' modifier", NAME, NAME, NAME); + MAP.put(DATA_CLASS_OVERRIDE_CONFLICT, "Function ''{0}'' generated for the data class conflicts with member of supertype ''{1}''", NAME, NAME); + MAP.put(CANNOT_OVERRIDE_INVISIBLE_MEMBER, "''{0}'' cannot has no access to ''{1}'' in class {2}, so it cannot override it", DescriptorRenderer.TEXT, DescriptorRenderer.TEXT, DescriptorRenderer.TEXT); MAP.put(CANNOT_INFER_VISIBILITY, "Cannot infer visibility. Please specify it explicitly"); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java index 22c390d4794..a82dcc47615 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java @@ -61,7 +61,9 @@ public interface BindingContext { } }; - WritableSlice ANNOTATION = Slices.createSimpleSlice(); + WritableSlice ANNOTATION_DESCRIPTOR_TO_PSI_ELEMENT = Slices.createSimpleSlice(); + WritableSlice ANNOTATION = + Slices.sliceBuilder().setOpposite(ANNOTATION_DESCRIPTOR_TO_PSI_ELEMENT).build(); WritableSlice> COMPILE_TIME_VALUE = Slices.createSimpleSlice(); WritableSlice TYPE = Slices.createSimpleSlice(); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java index 64221ae6a64..1ae075bac8e 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java @@ -26,12 +26,14 @@ import com.intellij.util.containers.MultiMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.*; +import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor; import org.jetbrains.jet.lang.diagnostics.Errors; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.name.Name; import org.jetbrains.jet.lang.resolve.scopes.JetScope; import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.checker.JetTypeChecker; +import org.jetbrains.jet.lang.types.lang.JetStandardLibrary; import org.jetbrains.jet.lexer.JetTokens; import org.jetbrains.jet.util.CommonSuppliers; @@ -564,6 +566,14 @@ public class OverrideResolver { } private void checkOverrideForMember(@NotNull final CallableMemberDescriptor declared) { + if (declared.getKind() == CallableMemberDescriptor.Kind.SYNTHESIZED) { + // TODO: this should be replaced soon by a framework of synthesized member generation tools + if (declared.getName().getName().startsWith(DescriptorResolver.COMPONENT_FUNCTION_NAME_PREFIX)) { + checkOverrideForComponentFunction(declared); + } + return; + } + if (declared.getKind() != CallableMemberDescriptor.Kind.DECLARATION) { return; } @@ -578,7 +588,7 @@ public class OverrideResolver { Set overriddenDescriptors = declared.getOverriddenDescriptors(); if (overrideNode != null) { - checkOverridesForMemberMarkedOverride(declared, new CheckOverrideReportStrategy() { + checkOverridesForMemberMarkedOverride(declared, true, new CheckOverrideReportStrategy() { private boolean finalOverriddenError = false; private boolean typeMismatchError = false; private boolean kindMismatchError = false; @@ -624,7 +634,11 @@ public class OverrideResolver { } } - private void checkOverridesForMemberMarkedOverride(@NotNull CallableMemberDescriptor declared, @NotNull CheckOverrideReportStrategy reportError) { + private void checkOverridesForMemberMarkedOverride( + @NotNull CallableMemberDescriptor declared, + boolean checkIfOverridesNothing, + @NotNull CheckOverrideReportStrategy reportError + ) { Set overriddenDescriptors = declared.getOverriddenDescriptors(); for (CallableMemberDescriptor overridden : overriddenDescriptors) { @@ -643,7 +657,7 @@ public class OverrideResolver { } } - if (overriddenDescriptors.isEmpty()) { + if (checkIfOverridesNothing && overriddenDescriptors.isEmpty()) { DeclarationDescriptor containingDeclaration = declared.getContainingDeclaration(); assert containingDeclaration instanceof ClassDescriptor : "Overrides may only be resolved in a class, but " + declared + " comes from " + containingDeclaration; ClassDescriptor declaringClass = (ClassDescriptor) containingDeclaration; @@ -658,6 +672,58 @@ public class OverrideResolver { } } + private void checkOverrideForComponentFunction(@NotNull final CallableMemberDescriptor componentFunction) { + final JetAnnotationEntry dataAnnotation = findDataAnnotationForDataClass(componentFunction.getContainingDeclaration()); + + checkOverridesForMemberMarkedOverride(componentFunction, false, new CheckOverrideReportStrategy() { + private boolean overrideConflict = false; + + @Override + public void overridingFinalMember(@NotNull CallableMemberDescriptor overridden) { + if (!overrideConflict) { + overrideConflict = true; + trace.report(DATA_CLASS_OVERRIDE_CONFLICT.on(dataAnnotation, componentFunction, overridden.getContainingDeclaration())); + } + } + + @Override + public void returnTypeMismatchOnOverride(@NotNull CallableMemberDescriptor overridden) { + if (!overrideConflict) { + overrideConflict = true; + trace.report(DATA_CLASS_OVERRIDE_CONFLICT.on(dataAnnotation, componentFunction, overridden.getContainingDeclaration())); + } + } + + @Override + public void varOverriddenByVal(@NotNull CallableMemberDescriptor overridden) { + throw new IllegalStateException("Component functions are not properties"); + } + + @Override + public void cannotOverrideInvisibleMember(@NotNull CallableMemberDescriptor invisibleOverridden) { + throw new IllegalStateException("CANNOT_OVERRIDE_INVISIBLE_MEMBER should be reported on the corresponding property"); + } + + @Override + public void nothingToOverride() { + throw new IllegalStateException("Component functions are OK to override nothing"); + } + }); + } + + @NotNull + private JetAnnotationEntry findDataAnnotationForDataClass(@NotNull DeclarationDescriptor dataClass) { + ClassDescriptor stdDataClassAnnotation = JetStandardLibrary.getInstance().getDataClassAnnotation(); + for (AnnotationDescriptor annotation : dataClass.getAnnotations()) { + if (stdDataClassAnnotation.equals(annotation.getType().getConstructor().getDeclarationDescriptor())) { + return BindingContextUtils.getNotNull(trace.getBindingContext(), + BindingContext.ANNOTATION_DESCRIPTOR_TO_PSI_ELEMENT, + annotation); + } + } + throw new IllegalStateException("No data annotation is found for data class"); + } + private CallableMemberDescriptor findInvisibleOverriddenDescriptor(CallableMemberDescriptor declared, ClassDescriptor declaringClass) { CallableMemberDescriptor invisibleOverride = null; outer: diff --git a/compiler/testData/diagnostics/tests/dataClasses/dataClassOverrideConflict.kt b/compiler/testData/diagnostics/tests/dataClasses/dataClassOverrideConflict.kt new file mode 100644 index 00000000000..faca497c974 --- /dev/null +++ b/compiler/testData/diagnostics/tests/dataClasses/dataClassOverrideConflict.kt @@ -0,0 +1,3 @@ +open data class A(val x: Int, val y: String) + +data class B(val z: String) : A(42, "") diff --git a/compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasComponent1.kt b/compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasComponent1.kt new file mode 100644 index 00000000000..144286c58ed --- /dev/null +++ b/compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasComponent1.kt @@ -0,0 +1,5 @@ +trait T { + fun component1(): Int +} + +data class A(val x: Int) : T diff --git a/compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasFinalComponent1.kt b/compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasFinalComponent1.kt new file mode 100644 index 00000000000..4e46de1163f --- /dev/null +++ b/compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasFinalComponent1.kt @@ -0,0 +1,5 @@ +trait T { + final fun component1(): Int = 42 +} + +data class A(val x: Int) : T diff --git a/compiler/testData/diagnostics/tests/dataClasses/overrideInvisibleMember.kt b/compiler/testData/diagnostics/tests/dataClasses/overrideInvisibleMember.kt new file mode 100644 index 00000000000..20f66f57503 --- /dev/null +++ b/compiler/testData/diagnostics/tests/dataClasses/overrideInvisibleMember.kt @@ -0,0 +1,3 @@ +open data class D(private final val x: Int) + +data class E(internal override val x: Int) : D(42) diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index af968729d0f..0e687dd563a 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -977,6 +977,11 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage doTest("compiler/testData/diagnostics/tests/dataClasses/componentNamedComponent1.kt"); } + @TestMetadata("dataClassOverrideConflict.kt") + public void testDataClassOverrideConflict() throws Exception { + doTest("compiler/testData/diagnostics/tests/dataClasses/dataClassOverrideConflict.kt"); + } + @TestMetadata("emptyConstructor.kt") public void testEmptyConstructor() throws Exception { doTest("compiler/testData/diagnostics/tests/dataClasses/emptyConstructor.kt"); @@ -992,6 +997,16 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage doTest("compiler/testData/diagnostics/tests/dataClasses/firstParamIsVar.kt"); } + @TestMetadata("implementTraitWhichHasComponent1.kt") + public void testImplementTraitWhichHasComponent1() throws Exception { + doTest("compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasComponent1.kt"); + } + + @TestMetadata("implementTraitWhichHasFinalComponent1.kt") + public void testImplementTraitWhichHasFinalComponent1() throws Exception { + doTest("compiler/testData/diagnostics/tests/dataClasses/implementTraitWhichHasFinalComponent1.kt"); + } + @TestMetadata("multiDeclaration.kt") public void testMultiDeclaration() throws Exception { doTest("compiler/testData/diagnostics/tests/dataClasses/multiDeclaration.kt"); @@ -1012,6 +1027,11 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage doTest("compiler/testData/diagnostics/tests/dataClasses/oneValParam.kt"); } + @TestMetadata("overrideInvisibleMember.kt") + public void testOverrideInvisibleMember() throws Exception { + doTest("compiler/testData/diagnostics/tests/dataClasses/overrideInvisibleMember.kt"); + } + @TestMetadata("secondParamIsVal.kt") public void testSecondParamIsVal() throws Exception { doTest("compiler/testData/diagnostics/tests/dataClasses/secondParamIsVal.kt");