From 3c3d1200049059e1618023c087517fb37be34b25 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 14 Apr 2014 18:32:03 +0400 Subject: [PATCH] Prohibit implicit overrides with lesser visibility A non-abstract super-declaration is allowed to stand as 'an implementation' of a fake override only if its visibility is not less than visibilities of all other super-declarations #KT-2491 Fixed --- .../noWarningsOnJavaKotlinInheritance.kt | 2 +- .../diagnostics/tests/override/kt2491.kt | 11 +++++++ .../kotlinInheritsBothJavaAndKotlin.kt | 2 +- .../checkers/JetDiagnosticsTestGenerated.java | 5 +++ .../jet/lang/resolve/OverridingUtil.java | 33 ++++++++++++------- 5 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/override/kt2491.kt diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/noWarningsOnJavaKotlinInheritance/noWarningsOnJavaKotlinInheritance.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/noWarningsOnJavaKotlinInheritance/noWarningsOnJavaKotlinInheritance.kt index e3524aec341..11f0f17de03 100644 --- a/compiler/testData/compileKotlinAgainstCustomBinaries/noWarningsOnJavaKotlinInheritance/noWarningsOnJavaKotlinInheritance.kt +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/noWarningsOnJavaKotlinInheritance/noWarningsOnJavaKotlinInheritance.kt @@ -8,7 +8,7 @@ class SimpleSubclass : JavaInterface { // Class extends both Java and Kotlin traits. Checks that there's no DIFFERENT_NAMES_FOR_THE_SAME_PARAMETER_IN_SUPERTYPES warning trait KotlinTrait { - fun foo(someOtherName: Int) {} + public fun foo(someOtherName: Int) {} } class BothTraitsSubclass : JavaInterface, KotlinTrait diff --git a/compiler/testData/diagnostics/tests/override/kt2491.kt b/compiler/testData/diagnostics/tests/override/kt2491.kt new file mode 100644 index 00000000000..02f0b8b74b9 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/kt2491.kt @@ -0,0 +1,11 @@ +trait T { + public fun foo() +} + +open class C { + protected fun foo() {} +} + +class D : C(), T + +val obj: C = object : C(), T {} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/override/parameterNames/kotlinInheritsBothJavaAndKotlin.kt b/compiler/testData/diagnostics/tests/override/parameterNames/kotlinInheritsBothJavaAndKotlin.kt index 74be8a6cdc0..e1d8929395c 100644 --- a/compiler/testData/diagnostics/tests/override/parameterNames/kotlinInheritsBothJavaAndKotlin.kt +++ b/compiler/testData/diagnostics/tests/override/parameterNames/kotlinInheritsBothJavaAndKotlin.kt @@ -7,7 +7,7 @@ interface JavaInterface { // FILE: kotlin.kt trait KotlinTrait { - fun foo(someOtherName: Int) {} + public fun foo(someOtherName: Int) {} } class BothTraitsSubclass : JavaInterface, KotlinTrait diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index f229b4c71e8..91c77f1c4e5 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -5342,6 +5342,11 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest("compiler/testData/diagnostics/tests/override/kt2052.kt"); } + @TestMetadata("kt2491.kt") + public void testKt2491() throws Exception { + doTest("compiler/testData/diagnostics/tests/override/kt2491.kt"); + } + @TestMetadata("kt4763.kt") public void testKt4763() throws Exception { doTest("compiler/testData/diagnostics/tests/override/kt4763.kt"); diff --git a/core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java b/core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java index cd23f7110ea..9ae925fa88a 100644 --- a/core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java +++ b/core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java @@ -379,8 +379,18 @@ public class OverridingUtil { return; } - Visibility maxVisibility = computeVisibilityToInherit(memberDescriptor, cannotInferVisibility); - Visibility visibilityToInherit = maxVisibility == null ? Visibilities.PUBLIC : maxVisibility; + Visibility maxVisibility = computeVisibilityToInherit(memberDescriptor); + Visibility visibilityToInherit; + if (maxVisibility == null) { + if (cannotInferVisibility != null) { + cannotInferVisibility.invoke(memberDescriptor); + } + visibilityToInherit = Visibilities.PUBLIC; + } + else { + visibilityToInherit = maxVisibility; + } + if (memberDescriptor instanceof PropertyDescriptorImpl) { ((PropertyDescriptorImpl) memberDescriptor).setVisibility(visibilityToInherit); for (PropertyAccessorDescriptor accessor : ((PropertyDescriptor) memberDescriptor).getAccessors()) { @@ -398,18 +408,19 @@ public class OverridingUtil { } @Nullable - private static Visibility computeVisibilityToInherit( - @NotNull CallableMemberDescriptor memberDescriptor, - @Nullable Function1 cannotInferVisibility - ) { - Visibility maxVisibility = findMaxVisibility(memberDescriptor.getOverriddenDescriptors()); + private static Visibility computeVisibilityToInherit(@NotNull CallableMemberDescriptor memberDescriptor) { + Set overriddenDescriptors = memberDescriptor.getOverriddenDescriptors(); + Visibility maxVisibility = findMaxVisibility(overriddenDescriptors); if (maxVisibility == null) { - if (cannotInferVisibility != null) { - cannotInferVisibility.invoke(memberDescriptor); - } return null; } if (memberDescriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE) { + for (CallableMemberDescriptor overridden : overriddenDescriptors) { + // An implementation (a non-abstract overridden member) of a fake override should have the maximum possible visibility + if (overridden.getModality() != Modality.ABSTRACT && !overridden.getVisibility().equals(maxVisibility)) { + return null; + } + } return maxVisibility; } return maxVisibility.normalize(); @@ -423,7 +434,7 @@ public class OverridingUtil { Visibility maxVisibility = null; for (CallableMemberDescriptor descriptor : descriptors) { Visibility visibility = descriptor.getVisibility(); - assert visibility != Visibilities.INHERITED; + assert visibility != Visibilities.INHERITED : "Visibility should have been computed for " + descriptor; if (maxVisibility == null) { maxVisibility = visibility; continue;