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
This commit is contained in:
Leonid Startsev
2021-04-29 19:59:03 +03:00
committed by Space
parent d34c23914a
commit 19dece01f6
10 changed files with 92 additions and 23 deletions
@@ -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
@@ -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
}
}
}
@@ -0,0 +1 @@
org.jetbrains.kotlin.idea.intentions.ChangeVisibilityModifierIntention$Public
@@ -0,0 +1,3 @@
// INTENTION_TEXT: Make public
// COMPILER_ARGUMENTS: -Xexplicit-api=strict
public class C <caret>private constructor(public val /* check no reformat here */ v: Int)
@@ -0,0 +1,3 @@
// INTENTION_TEXT: Make public
// COMPILER_ARGUMENTS: -Xexplicit-api=strict
public class C(public val /* check no reformat here */ v: Int)
@@ -0,0 +1,6 @@
// INTENTION_TEXT: Remove 'private' modifier
// COMPILER_ARGUMENTS: -Xexplicit-api=strict
public class Test {
public var foo: String = ""
<caret>private set
}
@@ -0,0 +1,6 @@
// INTENTION_TEXT: Remove 'private' modifier
// COMPILER_ARGUMENTS: -Xexplicit-api=strict
public class Test {
public var foo: String = ""
set
}
@@ -0,0 +1,4 @@
// COMPILER_ARGUMENTS: -Xexplicit-api=strict
public class C {
<caret>private fun foo(){}
}
@@ -0,0 +1,4 @@
// COMPILER_ARGUMENTS: -Xexplicit-api=strict
public class C {
public fun foo(){}
}
@@ -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")