From e806341cdced6927fabc0ac479272b32c82eaf2c Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 15 Apr 2016 13:23:42 +0300 Subject: [PATCH] Increase visibility: make not available in case of potential exposed visibility problems (see KT-11920) --- .../resolve/ExposedVisibilityChecker.kt | 35 ++++++++++++++++--- .../kotlin/descriptors/EffectiveVisibility.kt | 2 +- .../idea/quickfix/IncreaseVisibilityFix.kt | 10 ++++-- .../privateMemberToInternalWithExposed.kt | 14 ++++++++ .../idea/quickfix/QuickFixTestGenerated.java | 6 ++++ 5 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 idea/testData/quickfix/increaseVisibility/privateMemberToInternalWithExposed.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt index ec098589800..1597903de84 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt @@ -17,13 +17,15 @@ package org.jetbrains.kotlin.resolve import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.diagnostics.DiagnosticSink +import org.jetbrains.kotlin.diagnostics.DiagnosticSink.DO_NOTHING import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.types.TypeUtils // Checker for all seven EXPOSED_* errors // All functions return true if everything is OK, or false in case of any errors -class ExposedVisibilityChecker(private val trace: BindingTrace) { +class ExposedVisibilityChecker(private val trace: DiagnosticSink = DO_NOTHING) { // NB: does not check any members fun checkClassHeader(klass: KtClassOrObject, classDescriptor: ClassDescriptor): Boolean { @@ -35,8 +37,27 @@ class ExposedVisibilityChecker(private val trace: BindingTrace) { return result and checkFunction(constructor, constructorDescriptor) } - fun checkFunction(function: KtFunction, functionDescriptor: FunctionDescriptor): Boolean { - val functionVisibility = functionDescriptor.effectiveVisibility() + fun checkDeclarationWithVisibility(modifierListOwner: KtModifierListOwner, + descriptor: DeclarationDescriptorWithVisibility, + visibility: Visibility + ) : Boolean { + return when { + modifierListOwner is KtFunction && + descriptor is FunctionDescriptor -> checkFunction(modifierListOwner, descriptor, visibility) + + modifierListOwner is KtProperty && + descriptor is PropertyDescriptor -> checkProperty(modifierListOwner, descriptor, visibility) + + else -> true + } + } + + fun checkFunction(function: KtFunction, + functionDescriptor: FunctionDescriptor, + // for checking situation with modified basic visibility + visibility: Visibility = functionDescriptor.visibility + ): Boolean { + val functionVisibility = functionDescriptor.effectiveVisibility(visibility) var result = true if (function !is KtConstructor<*>) { val restricting = functionDescriptor.returnType?.leastPermissiveDescriptor(functionVisibility) @@ -57,8 +78,12 @@ class ExposedVisibilityChecker(private val trace: BindingTrace) { return result and checkMemberReceiver(function.receiverTypeReference, functionDescriptor) } - fun checkProperty(property: KtProperty, propertyDescriptor: PropertyDescriptor): Boolean { - val propertyVisibility = propertyDescriptor.effectiveVisibility() + fun checkProperty(property: KtProperty, + propertyDescriptor: PropertyDescriptor, + // for checking situation with modified basic visibility + visibility: Visibility = propertyDescriptor.visibility + ): Boolean { + val propertyVisibility = propertyDescriptor.effectiveVisibility(visibility) val restricting = propertyDescriptor.type.leastPermissiveDescriptor(propertyVisibility) var result = true if (restricting != null) { diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/EffectiveVisibility.kt b/core/descriptors/src/org/jetbrains/kotlin/descriptors/EffectiveVisibility.kt index 3e2b6c66bfc..a6fd55aa80e 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/EffectiveVisibility.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/EffectiveVisibility.kt @@ -281,7 +281,7 @@ private fun Set.leastPermissive(base: EffectiveVisibilit fun KotlinType.leastPermissiveDescriptor(base: EffectiveVisibility) = dependentDescriptors().leastPermissive(base) -fun DeclarationDescriptorWithVisibility.effectiveVisibility(): EffectiveVisibility = +fun DeclarationDescriptorWithVisibility.effectiveVisibility(visibility: Visibility = this.visibility): EffectiveVisibility = lowerBound(visibility.effectiveVisibility(this.containingDeclaration as? ClassDescriptor), (this.containingDeclaration as? ClassDescriptor)?.effectiveVisibility() ?: Public) diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/IncreaseVisibilityFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/IncreaseVisibilityFix.kt index 023fbec8072..8f31544b371 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/IncreaseVisibilityFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/IncreaseVisibilityFix.kt @@ -33,6 +33,7 @@ import org.jetbrains.kotlin.psi.KtModifierListOwner import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.ExposedVisibilityChecker import org.jetbrains.kotlin.resolve.descriptorUtil.module import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.idea.core.setVisibility @@ -62,12 +63,15 @@ class IncreaseVisibilityFix( val declaration = DescriptorToSourceUtils.getSourceFromDescriptor(descriptor) as? KtModifierListOwner ?: return null val module = DescriptorUtils.getContainingModule(descriptor) - val modifier = if (module != usageModule || descriptor.visibility != Visibilities.PRIVATE) { - KtTokens.PUBLIC_KEYWORD + val (modifier, visibility) = if (module != usageModule || descriptor.visibility != Visibilities.PRIVATE) { + Pair(KtTokens.PUBLIC_KEYWORD, Visibilities.PUBLIC) } else { - KtTokens.INTERNAL_KEYWORD + Pair(KtTokens.INTERNAL_KEYWORD, Visibilities.INTERNAL) } + + if (!ExposedVisibilityChecker().checkDeclarationWithVisibility(declaration, descriptor, visibility)) return null + return IncreaseVisibilityFix(declaration, descriptor.name.asString(), modifier) } } diff --git a/idea/testData/quickfix/increaseVisibility/privateMemberToInternalWithExposed.kt b/idea/testData/quickfix/increaseVisibility/privateMemberToInternalWithExposed.kt new file mode 100644 index 00000000000..4d14464c1fe --- /dev/null +++ b/idea/testData/quickfix/increaseVisibility/privateMemberToInternalWithExposed.kt @@ -0,0 +1,14 @@ +// "Make bar internal" "false" +// ACTION: Convert property initializer to getter +// ERROR: Cannot access 'bar': it is 'private' in 'First' + +private data class Data(val x: Int) + +class First { + // Making it internal exposes 'Data' + private fun bar(x: Int) = Data(x) +} + +class Second(f: First) { + private val y = f.bar(42) +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index b6fe817c1ec..b258c008fdb 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -4731,6 +4731,12 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { doTest(fileName); } + @TestMetadata("privateMemberToInternalWithExposed.kt") + public void testPrivateMemberToInternalWithExposed() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/increaseVisibility/privateMemberToInternalWithExposed.kt"); + doTest(fileName); + } + @TestMetadata("protectedMemberToPublicSingleFile.kt") public void testProtectedMemberToPublicSingleFile() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/increaseVisibility/protectedMemberToPublicSingleFile.kt");