From b3857e85e06693901761dba3965eff431213363e Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 6 Nov 2018 17:52:48 +0300 Subject: [PATCH] Report exposed type for properties in class primary constructor Before this commit, we compared property visibility with constructor visibility only, which is incorrect. Now we compare property visibility also with class visibility #KT-19613 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 1 + .../rendering/DefaultErrorMessages.java | 1 + .../resolve/ExposedVisibilityChecker.kt | 32 ++++++++++++++----- .../privatePropertyInPrivateConstructor.kt | 3 ++ .../privatePropertyInPrivateConstructor.txt | 30 +++++++++++++++++ .../propertyInConstructorOfPrivateClass.kt | 3 ++ .../propertyInConstructorOfPrivateClass.txt | 30 +++++++++++++++++ .../exposed/propertyInPrivateConstructor.kt | 3 ++ .../exposed/propertyInPrivateConstructor.txt | 30 +++++++++++++++++ .../exposed/propertyInSimpleConstructor.kt | 3 ++ .../exposed/propertyInSimpleConstructor.txt | 30 +++++++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 20 ++++++++++++ .../DiagnosticsUsingJavacTestGenerated.java | 20 ++++++++++++ .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 4 +-- 14 files changed, 200 insertions(+), 10 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.kt create mode 100644 compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.txt create mode 100644 compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.kt create mode 100644 compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.txt create mode 100644 compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.kt create mode 100644 compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.txt create mode 100644 compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.kt create mode 100644 compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 696f7aebae0..3e0b0c286be 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -116,6 +116,7 @@ public interface Errors { // Exposed visibility group DiagnosticFactory3 EXPOSED_PROPERTY_TYPE = DiagnosticFactory3.create(ERROR); + DiagnosticFactory3 EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR = DiagnosticFactory3.create(WARNING); DiagnosticFactory3 EXPOSED_FUNCTION_RETURN_TYPE = DiagnosticFactory3.create(ERROR); DiagnosticFactory3 EXPOSED_PARAMETER_TYPE = DiagnosticFactory3.create(ERROR); DiagnosticFactory3 EXPOSED_RECEIVER_TYPE = DiagnosticFactory3.create(ERROR); 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 4b085c64a8d..9ce2413946a 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -69,6 +69,7 @@ public class DefaultErrorMessages { MAP.put(PROTECTED_CONSTRUCTOR_NOT_IN_SUPER_CALL, "Protected constructor ''{0}'' from other classes can only be used in super-call", Renderers.SHORT_NAMES_IN_TYPES); MAP.put(EXPOSED_PROPERTY_TYPE, "''{0}'' property exposes its ''{2}'' type{1}", TO_STRING, TO_STRING, TO_STRING); + MAP.put(EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR, "''{0}'' property exposes its ''{2}'' type{1}", TO_STRING, TO_STRING, TO_STRING); MAP.put(EXPOSED_FUNCTION_RETURN_TYPE, "''{0}'' function exposes its ''{2}'' return type{1}", TO_STRING, TO_STRING, TO_STRING); MAP.put(EXPOSED_PARAMETER_TYPE, "''{0}'' function exposes its ''{2}'' parameter type{1}", TO_STRING, TO_STRING, TO_STRING); MAP.put(EXPOSED_RECEIVER_TYPE, "''{0}'' member exposes its ''{2}'' receiver type{1}", TO_STRING, TO_STRING, TO_STRING); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt index 8f4dee26247..3c994089712 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt @@ -91,15 +91,31 @@ class ExposedVisibilityChecker(private val trace: DiagnosticSink = DO_NOTHING) { } } functionDescriptor.valueParameters.forEachIndexed { i, parameterDescriptor -> - val restricting = parameterDescriptor.type.leastPermissiveDescriptor(functionVisibility) - if (restricting != null && i < function.valueParameters.size) { - trace.report( - Errors.EXPOSED_PARAMETER_TYPE.on( - function.valueParameters[i], functionVisibility, - restricting, restricting.effectiveVisibility() + if (i < function.valueParameters.size) { + val valueParameter = function.valueParameters[i] + val restricting = parameterDescriptor.type.leastPermissiveDescriptor(functionVisibility) + if (restricting != null) { + trace.report( + Errors.EXPOSED_PARAMETER_TYPE.on( + valueParameter, functionVisibility, + restricting, restricting.effectiveVisibility() + ) ) - ) - result = false + result = false + } else if (functionDescriptor is ClassConstructorDescriptor && valueParameter.hasValOrVar()) { + val propertyDescriptor = (trace as? BindingTrace)?.get(BindingContext.VALUE_PARAMETER_AS_PROPERTY, parameterDescriptor) + val propertyOrClassVisibility = (propertyDescriptor ?: functionDescriptor.constructedClass).effectiveVisibility() + val restrictingByProperty = parameterDescriptor.type.leastPermissiveDescriptor(propertyOrClassVisibility) + if (restrictingByProperty != null) { + trace.report( + Errors.EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR.on( + valueParameter, propertyOrClassVisibility, + restrictingByProperty, restrictingByProperty.effectiveVisibility() + ) + ) + result = false + } + } } } return result and checkMemberReceiver(function.receiverTypeReference, functionDescriptor) diff --git a/compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.kt b/compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.kt new file mode 100644 index 00000000000..db0bb7a35bd --- /dev/null +++ b/compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.kt @@ -0,0 +1,3 @@ +private enum class Foo { A, B } + +class Bar private constructor(private val foo: Foo) \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.txt b/compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.txt new file mode 100644 index 00000000000..5ca5d16dd1b --- /dev/null +++ b/compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.txt @@ -0,0 +1,30 @@ +package + +public final class Bar { + private constructor Bar(/*0*/ foo: Foo) + private final val foo: Foo + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +private final enum class Foo : kotlin.Enum { + enum entry A + + enum entry B + + private constructor Foo() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: Foo): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun finalize(): kotlin.Unit + public final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun getDeclaringClass(): java.lang.Class! + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): Foo + public final /*synthesized*/ fun values(): kotlin.Array +} diff --git a/compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.kt b/compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.kt new file mode 100644 index 00000000000..d2b672f3f0e --- /dev/null +++ b/compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.kt @@ -0,0 +1,3 @@ +private enum class Foo { A, B } + +private class Bar(val foo: Foo) \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.txt b/compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.txt new file mode 100644 index 00000000000..b6a6bc586a5 --- /dev/null +++ b/compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.txt @@ -0,0 +1,30 @@ +package + +private final class Bar { + public constructor Bar(/*0*/ foo: Foo) + public final val foo: Foo + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +private final enum class Foo : kotlin.Enum { + enum entry A + + enum entry B + + private constructor Foo() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: Foo): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun finalize(): kotlin.Unit + public final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun getDeclaringClass(): java.lang.Class! + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): Foo + public final /*synthesized*/ fun values(): kotlin.Array +} diff --git a/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.kt b/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.kt new file mode 100644 index 00000000000..277913e8b57 --- /dev/null +++ b/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.kt @@ -0,0 +1,3 @@ +private enum class Foo { A, B } + +class Bar private constructor(val foo: Foo) \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.txt b/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.txt new file mode 100644 index 00000000000..cbe4134d1e5 --- /dev/null +++ b/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.txt @@ -0,0 +1,30 @@ +package + +public final class Bar { + private constructor Bar(/*0*/ foo: Foo) + public final val foo: Foo + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +private final enum class Foo : kotlin.Enum { + enum entry A + + enum entry B + + private constructor Foo() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: Foo): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun finalize(): kotlin.Unit + public final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun getDeclaringClass(): java.lang.Class! + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): Foo + public final /*synthesized*/ fun values(): kotlin.Array +} diff --git a/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.kt b/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.kt new file mode 100644 index 00000000000..21371c56dd6 --- /dev/null +++ b/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.kt @@ -0,0 +1,3 @@ +private enum class Foo { A, B } + +class Bar(val foo: Foo) \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.txt b/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.txt new file mode 100644 index 00000000000..dafa422b443 --- /dev/null +++ b/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.txt @@ -0,0 +1,30 @@ +package + +public final class Bar { + public constructor Bar(/*0*/ foo: Foo) + public final val foo: Foo + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +private final enum class Foo : kotlin.Enum { + enum entry A + + enum entry B + + private constructor Foo() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: Foo): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun finalize(): kotlin.Unit + public final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun getDeclaringClass(): java.lang.Class! + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): Foo + public final /*synthesized*/ fun values(): kotlin.Array +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 48fe3bd4f51..6307a7dc10a 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -7315,6 +7315,26 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { runTest("compiler/testData/diagnostics/tests/exposed/privateFromLocal.kt"); } + @TestMetadata("privatePropertyInPrivateConstructor.kt") + public void testPrivatePropertyInPrivateConstructor() throws Exception { + runTest("compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.kt"); + } + + @TestMetadata("propertyInConstructorOfPrivateClass.kt") + public void testPropertyInConstructorOfPrivateClass() throws Exception { + runTest("compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.kt"); + } + + @TestMetadata("propertyInPrivateConstructor.kt") + public void testPropertyInPrivateConstructor() throws Exception { + runTest("compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.kt"); + } + + @TestMetadata("propertyInSimpleConstructor.kt") + public void testPropertyInSimpleConstructor() throws Exception { + runTest("compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.kt"); + } + @TestMetadata("protected.kt") public void testProtected() throws Exception { runTest("compiler/testData/diagnostics/tests/exposed/protected.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index 4101eb647d2..56e39edf150 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -7315,6 +7315,26 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/exposed/privateFromLocal.kt"); } + @TestMetadata("privatePropertyInPrivateConstructor.kt") + public void testPrivatePropertyInPrivateConstructor() throws Exception { + runTest("compiler/testData/diagnostics/tests/exposed/privatePropertyInPrivateConstructor.kt"); + } + + @TestMetadata("propertyInConstructorOfPrivateClass.kt") + public void testPropertyInConstructorOfPrivateClass() throws Exception { + runTest("compiler/testData/diagnostics/tests/exposed/propertyInConstructorOfPrivateClass.kt"); + } + + @TestMetadata("propertyInPrivateConstructor.kt") + public void testPropertyInPrivateConstructor() throws Exception { + runTest("compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.kt"); + } + + @TestMetadata("propertyInSimpleConstructor.kt") + public void testPropertyInSimpleConstructor() throws Exception { + runTest("compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.kt"); + } + @TestMetadata("protected.kt") public void testProtected() throws Exception { runTest("compiler/testData/diagnostics/tests/exposed/protected.kt"); diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index abe8680890d..26711fcced4 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -188,8 +188,8 @@ class QuickFixRegistrar : QuickFixContributor { INVISIBLE_SETTER.registerFactory(MakeVisibleFactory) for (exposed in listOf( - EXPOSED_FUNCTION_RETURN_TYPE, EXPOSED_PARAMETER_TYPE, EXPOSED_PROPERTY_TYPE, EXPOSED_RECEIVER_TYPE, - EXPOSED_SUPER_CLASS, EXPOSED_SUPER_INTERFACE, EXPOSED_TYPE_PARAMETER_BOUND + EXPOSED_FUNCTION_RETURN_TYPE, EXPOSED_PARAMETER_TYPE, EXPOSED_PROPERTY_TYPE, EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR, + EXPOSED_RECEIVER_TYPE, EXPOSED_SUPER_CLASS, EXPOSED_SUPER_INTERFACE, EXPOSED_TYPE_PARAMETER_BOUND )) { exposed.registerFactory(ChangeVisibilityOnExposureFactory) }