From 19dece01f64055a3c10bb518730cab18747fb047 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Thu, 29 Apr 2021 19:59:03 +0300 Subject: [PATCH] Add support of explicit API mode to 'Make public' intention #KT-41902 Fixed Do not check languageVersionSettings for fake elements because this leads to an exception --- .../checkers/ExplicitApiDeclarationChecker.kt | 45 ++++++++++--------- .../kotlin/idea/core/psiModificationUtils.kt | 15 ++++++- .../publicExplicitApi/.intention | 1 + .../publicExplicitApi/primaryConstructor.kt | 3 ++ .../primaryConstructor.kt.after | 3 ++ .../propertyPrivateSetter.kt | 6 +++ .../propertyPrivateSetter.kt.after | 6 +++ .../publicExplicitApi/simple.kt | 4 ++ .../publicExplicitApi/simple.kt.after | 4 ++ .../intentions/IntentionTestGenerated.java | 28 ++++++++++++ 10 files changed, 92 insertions(+), 23 deletions(-) create mode 100644 idea/testData/intentions/changeVisibility/publicExplicitApi/.intention create mode 100644 idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt create mode 100644 idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt.after create mode 100644 idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt create mode 100644 idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt.after create mode 100644 idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt create mode 100644 idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt.after diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExplicitApiDeclarationChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExplicitApiDeclarationChecker.kt index 19d0494d31c..fb78908ec04 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExplicitApiDeclarationChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExplicitApiDeclarationChecker.kt @@ -1,5 +1,5 @@ /* - * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. */ @@ -39,7 +39,7 @@ class ExplicitApiDeclarationChecker : DeclarationChecker { val modifier = declaration.visibilityModifier() if (modifier != null) return - if (excludeForDiagnostic(descriptor)) return + if (explicitVisibilityIsNotRequired(descriptor)) return val diagnostic = if (state == ExplicitApiMode.STRICT) Errors.NO_EXPLICIT_VISIBILITY_IN_API_MODE @@ -73,26 +73,26 @@ class ExplicitApiDeclarationChecker : DeclarationChecker { } } - /** - * Exclusion list: - * 1. Primary constructors of public API classes - * 2. Properties of data classes in public API - * 3. Overrides of public API. Effectively, this means 'no report on overrides at all' - * 4. Getters and setters (because getters can't change visibility and setter-only explicit visibility looks ugly) - * 5. Properties of annotations in public API - * - * Do we need something like @PublicApiFile to disable (or invert) this inspection per-file? - */ - private fun excludeForDiagnostic(descriptor: DeclarationDescriptor): Boolean { - /* 1. */ if ((descriptor as? ClassConstructorDescriptor)?.isPrimary == true) return true - /* 2. */ if (descriptor is PropertyDescriptor && (descriptor.containingDeclaration as? ClassDescriptor)?.isData == true) return true - /* 3. */ if ((descriptor as? CallableDescriptor)?.overriddenDescriptors?.isNotEmpty() == true) return true - /* 4. */ if (descriptor is PropertyAccessorDescriptor) return true - /* 5. */ if (descriptor is PropertyDescriptor && (descriptor.containingDeclaration as? ClassDescriptor)?.kind == ClassKind.ANNOTATION_CLASS) return true - return false - } - companion object { + /** + * Exclusion list: + * 1. Primary constructors of public API classes + * 2. Properties of data classes in public API + * 3. Overrides of public API. Effectively, this means 'no report on overrides at all' + * 4. Getters and setters (because getters can't change visibility and setter-only explicit visibility looks ugly) + * 5. Properties of annotations in public API + * + * Do we need something like @PublicApiFile to disable (or invert) this inspection per-file? + */ + fun explicitVisibilityIsNotRequired(descriptor: DeclarationDescriptor): Boolean { + /* 1. */ if ((descriptor as? ClassConstructorDescriptor)?.isPrimary == true) return true + /* 2. */ if (descriptor is PropertyDescriptor && (descriptor.containingDeclaration as? ClassDescriptor)?.isData == true) return true + /* 3. */ if ((descriptor as? CallableDescriptor)?.overriddenDescriptors?.isNotEmpty() == true) return true + /* 4. */ if (descriptor is PropertyAccessorDescriptor) return true + /* 5. */ if (descriptor is PropertyDescriptor && (descriptor.containingDeclaration as? ClassDescriptor)?.kind == ClassKind.ANNOTATION_CLASS) return true + return false + } + fun returnTypeRequired( element: KtCallableDeclaration, descriptor: DeclarationDescriptor?, @@ -140,3 +140,6 @@ class ExplicitApiDeclarationChecker : DeclarationChecker { } } } + +val LanguageVersionSettings.explicitApiEnabled: Boolean + get() = getFlag(AnalysisFlags.explicitApiMode) != ExplicitApiMode.DISABLED \ No newline at end of file diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt index 12b42d4935d..95a6fced201 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt @@ -20,6 +20,7 @@ import org.jetbrains.kotlin.idea.FrontendInternals import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny +import org.jetbrains.kotlin.idea.project.languageVersionSettings import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.resolve.frontendService import org.jetbrains.kotlin.idea.resolve.getLanguageVersionSettings @@ -38,7 +39,10 @@ import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.OverridingUtil import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.callUtil.getValueArgumentsInParentheses +import org.jetbrains.kotlin.resolve.calls.callUtil.isFakeElement import org.jetbrains.kotlin.resolve.calls.model.ArgumentMatch +import org.jetbrains.kotlin.resolve.checkers.ExplicitApiDeclarationChecker +import org.jetbrains.kotlin.resolve.checkers.explicitApiEnabled import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.resolve.sam.SamConversionOracle import org.jetbrains.kotlin.resolve.sam.SamConversionResolver @@ -307,8 +311,15 @@ fun KtModifierListOwner.setVisibility(visibilityModifier: KtModifierKeywordToken val defaultVisibilityKeyword = implicitVisibility() if (visibilityModifier == defaultVisibilityKeyword) { - this.visibilityModifierType()?.let { removeModifier(it) } - return + // Fake elements do not have ModuleInfo and languageVersionSettings because they can't be analysed + // Effectively, this leads to J2K not respecting explicit api mode, but this case seems to be rare anyway. + val explicitVisibilityRequired = !this.isFakeElement && this.languageVersionSettings.explicitApiEnabled + && this.resolveToDescriptorIfAny()?.let { !ExplicitApiDeclarationChecker.explicitVisibilityIsNotRequired(it) } == true + + if (!explicitVisibilityRequired) { + this.visibilityModifierType()?.let { removeModifier(it) } + return + } } } diff --git a/idea/testData/intentions/changeVisibility/publicExplicitApi/.intention b/idea/testData/intentions/changeVisibility/publicExplicitApi/.intention new file mode 100644 index 00000000000..1ce7b13b041 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/publicExplicitApi/.intention @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.intentions.ChangeVisibilityModifierIntention$Public diff --git a/idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt b/idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt new file mode 100644 index 00000000000..54e055b9ee2 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt @@ -0,0 +1,3 @@ +// INTENTION_TEXT: Make public +// COMPILER_ARGUMENTS: -Xexplicit-api=strict +public class C private constructor(public val /* check no reformat here */ v: Int) \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt.after b/idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt.after new file mode 100644 index 00000000000..dde7bbf6bf5 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt.after @@ -0,0 +1,3 @@ +// INTENTION_TEXT: Make public +// COMPILER_ARGUMENTS: -Xexplicit-api=strict +public class C(public val /* check no reformat here */ v: Int) \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt b/idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt new file mode 100644 index 00000000000..6cb2e39a697 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt @@ -0,0 +1,6 @@ +// INTENTION_TEXT: Remove 'private' modifier +// COMPILER_ARGUMENTS: -Xexplicit-api=strict +public class Test { + public var foo: String = "" + private set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt.after b/idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt.after new file mode 100644 index 00000000000..106ee5a72ee --- /dev/null +++ b/idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt.after @@ -0,0 +1,6 @@ +// INTENTION_TEXT: Remove 'private' modifier +// COMPILER_ARGUMENTS: -Xexplicit-api=strict +public class Test { + public var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt b/idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt new file mode 100644 index 00000000000..0c9ce7b2ade --- /dev/null +++ b/idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt @@ -0,0 +1,4 @@ +// COMPILER_ARGUMENTS: -Xexplicit-api=strict +public class C { + private fun foo(){} +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt.after b/idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt.after new file mode 100644 index 00000000000..9fd79939130 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt.after @@ -0,0 +1,4 @@ +// COMPILER_ARGUMENTS: -Xexplicit-api=strict +public class C { + public fun foo(){} +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 0b4dbd46a58..8c4da8784d9 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -3816,6 +3816,34 @@ public class IntentionTestGenerated extends AbstractIntentionTest { runTest("idea/testData/intentions/changeVisibility/public/typeAlias.kt"); } } + + @TestMetadata("idea/testData/intentions/changeVisibility/publicExplicitApi") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class PublicExplicitApi extends AbstractIntentionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, this, testDataFilePath); + } + + public void testAllFilesPresentInPublicExplicitApi() throws Exception { + KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("idea/testData/intentions/changeVisibility/publicExplicitApi"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), null, true); + } + + @TestMetadata("primaryConstructor.kt") + public void testPrimaryConstructor() throws Exception { + runTest("idea/testData/intentions/changeVisibility/publicExplicitApi/primaryConstructor.kt"); + } + + @TestMetadata("propertyPrivateSetter.kt") + public void testPropertyPrivateSetter() throws Exception { + runTest("idea/testData/intentions/changeVisibility/publicExplicitApi/propertyPrivateSetter.kt"); + } + + @TestMetadata("simple.kt") + public void testSimple() throws Exception { + runTest("idea/testData/intentions/changeVisibility/publicExplicitApi/simple.kt"); + } + } } @TestMetadata("idea/testData/intentions/chop")