[Kotlin] Don't suggest "Copy annotation to actual" fix for f/o and typealias

Don't suggest quick-fix "Copy annotation from expect to actual" for
fake overrides and `actual typealias`, even if the source is available.

This is detected by full fqName of method or class.
If it is the same as the name of `expect` declaration, we are sure
that actual declaration is inside `actual class`.

^KTIJ-26633

GitOrigin-RevId: 21f2b7cb007f539b3275989e86d0ac5a04bb98b4
This commit is contained in:
Roman Efremov
2023-08-23 17:40:25 +00:00
committed by intellij-monorepo-bot
parent e7ee73bafb
commit a3ae7868b0
23 changed files with 202 additions and 1 deletions
@@ -32,11 +32,13 @@ internal object ActualAnnotationsNotMatchExpectFixFactory {
diagnostic: KtFirDiagnostic.ActualAnnotationsNotMatchExpect,
expectAnnotationEntry: KtAnnotationEntry,
): List<QuickFixActionBase<*>> {
val expectDeclaration = diagnostic.expectSymbol.psi as? KtNamedDeclaration ?: return emptyList()
val actualDeclaration = diagnostic.actualSymbol.psi as? KtNamedDeclaration ?: return emptyList()
val mappedIncompatibilityType = diagnostic.incompatibilityType.mapAnnotationType {
it.psi as? KtAnnotationEntry
}
return ActualAnnotationsNotMatchExpectFixFactoryCommon.createCopyAndReplaceAnnotationFixes(
expectDeclaration,
actualDeclaration,
expectAnnotationEntry,
mappedIncompatibilityType,
@@ -124,6 +124,16 @@ public abstract class HighLevelQuickFixMultiModuleTestGenerated extends Abstract
KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
}
@TestMetadata("copyNotSuggestedWhenActualFakeOverride")
public void testCopyNotSuggestedWhenActualFakeOverride() throws Exception {
runTest("../idea/tests/testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/copyNotSuggestedWhenActualFakeOverride/");
}
@TestMetadata("copyNotSuggestedWhenActualTypealias")
public void testCopyNotSuggestedWhenActualTypealias() throws Exception {
runTest("../idea/tests/testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/copyNotSuggestedWhenActualTypealias/");
}
@TestMetadata("copyToActualConstExpression")
public void testCopyToActualConstExpression() throws Exception {
runTest("../idea/tests/testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/copyToActualConstExpression/");
@@ -159,6 +169,21 @@ public abstract class HighLevelQuickFixMultiModuleTestGenerated extends Abstract
runTest("../idea/tests/testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/removeFromExpect/");
}
@TestMetadata("removeFromExpectSuggestedWhenActualFakeOverride")
public void testRemoveFromExpectSuggestedWhenActualFakeOverride() throws Exception {
runTest("../idea/tests/testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/removeFromExpectSuggestedWhenActualFakeOverride/");
}
@TestMetadata("removeFromExpectSuggestedWhenActualHasNoSource")
public void testRemoveFromExpectSuggestedWhenActualHasNoSource() throws Exception {
runTest("../idea/tests/testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/removeFromExpectSuggestedWhenActualHasNoSource/");
}
@TestMetadata("removeFromExpectSuggestedWhenActualTypealias")
public void testRemoveFromExpectSuggestedWhenActualTypealias() throws Exception {
runTest("../idea/tests/testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/removeFromExpectSuggestedWhenActualTypealias/");
}
@TestMetadata("replaceArgsOnActual")
public void testReplaceArgsOnActual() throws Exception {
runTest("../idea/tests/testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/replaceArgsOnActual/");
@@ -1,12 +1,16 @@
// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package org.jetbrains.kotlin.idea.quickfix
import org.jetbrains.kotlin.idea.base.psi.callableIdIfNotLocal
import org.jetbrains.kotlin.idea.base.psi.classIdIfNonLocal
import org.jetbrains.kotlin.idea.base.resources.KotlinBundle
import org.jetbrains.kotlin.idea.codeinsight.api.classic.quickfixes.KotlinQuickFixAction
import org.jetbrains.kotlin.idea.codeinsight.api.classic.quickfixes.QuickFixActionBase
import org.jetbrains.kotlin.idea.inspections.RemoveAnnotationFix
import org.jetbrains.kotlin.name.ClassId
import org.jetbrains.kotlin.psi.KtAnnotationEntry
import org.jetbrains.kotlin.psi.KtCallableDeclaration
import org.jetbrains.kotlin.psi.KtClassOrObject
import org.jetbrains.kotlin.psi.KtNamedDeclaration
import org.jetbrains.kotlin.resolve.multiplatform.ExpectActualAnnotationsIncompatibilityType
@@ -20,11 +24,16 @@ object ActualAnnotationsNotMatchExpectFixFactoryCommon {
}
fun createCopyAndReplaceAnnotationFixes(
expectDeclaration: KtNamedDeclaration,
actualDeclaration: KtNamedDeclaration,
expectAnnotationEntry: KtAnnotationEntry,
incompatibilityType: ExpectActualAnnotationsIncompatibilityType<KtAnnotationEntry?>,
annotationClassIdProvider: () -> ClassId?,
): List<KotlinQuickFixAction<*>> {
if (skipFakeOverrideAndTypealias(expectDeclaration, actualDeclaration)) {
return emptyList()
}
val actualAnnotationEntry = when (incompatibilityType) {
is ExpectActualAnnotationsIncompatibilityType.MissingOnActual -> null
is ExpectActualAnnotationsIncompatibilityType.DifferentOnActual -> incompatibilityType.actualAnnotation
@@ -55,4 +64,30 @@ object ActualAnnotationsNotMatchExpectFixFactoryCommon {
annotationClassId,
)
}
/**
* When actual is typealias, quick fix changes code somewhere in another declaration (nor expect, nor actual), which might be
* unrelated to the KMP world.
* Such a modification is incomprehensible and undesirable for the user.
* Same with actual fake overrides: user wants to fix the incompatibility between expected and actual classes,
* but quick fix changes some third class that probably serves some other purpose than just being a common parent.
*/
private fun skipFakeOverrideAndTypealias(expectDeclaration: KtNamedDeclaration, actualDeclaration: KtNamedDeclaration): Boolean {
fun notEqual(a: Any?, b: Any?): Boolean {
check(a != null && b != null) { "expect and actual cannot be local, so must always have non-null ClassId"}
return a != b
}
return when {
expectDeclaration is KtClassOrObject && actualDeclaration is KtClassOrObject -> {
notEqual(expectDeclaration.classIdIfNonLocal, actualDeclaration.classIdIfNonLocal)
}
expectDeclaration is KtCallableDeclaration && actualDeclaration is KtCallableDeclaration -> {
notEqual(expectDeclaration.callableIdIfNotLocal, actualDeclaration.callableIdIfNotLocal)
}
else -> error("Unexpected types: $expectDeclaration $actualDeclaration")
}
}
}
@@ -32,20 +32,23 @@ internal object ActualAnnotationsNotMatchExpectFixFactory : KotlinIntentionActio
ActualAnnotationsNotMatchExpectFixFactoryCommon.createRemoveAnnotationFromExpectFix(expectAnnotationEntry)
return listOfNotNull(removeAnnotationFix) +
createCopyAndReplaceAnnotationFixes(expectAnnotationEntry, castedDiagnostic.b, incompatibilityType)
createCopyAndReplaceAnnotationFixes(expectAnnotationEntry, castedDiagnostic.a, castedDiagnostic.b, incompatibilityType)
}
private fun createCopyAndReplaceAnnotationFixes(
expectAnnotationEntry: KtAnnotationEntry,
expectDeclarationDescriptor: DeclarationDescriptor,
actualDeclarationDescriptor: DeclarationDescriptor,
incompatibilityType: ExpectActualAnnotationsIncompatibilityType<AnnotationDescriptor>,
): List<QuickFixActionBase<*>> {
val expectDeclaration = expectDeclarationDescriptor.toSourceElement.getPsi() as? KtNamedDeclaration ?: return emptyList()
val actualDeclaration = actualDeclarationDescriptor.toSourceElement.getPsi() as? KtNamedDeclaration ?: return emptyList()
val mappedIncompatibilityType = incompatibilityType.mapAnnotationType {
it.source.getPsi() as? KtAnnotationEntry
}
return ActualAnnotationsNotMatchExpectFixFactoryCommon.createCopyAndReplaceAnnotationFixes(
expectDeclaration,
actualDeclaration,
expectAnnotationEntry,
mappedIncompatibilityType,
@@ -124,6 +124,16 @@ public abstract class QuickFixMultiModuleTestGenerated extends AbstractQuickFixM
KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
}
@TestMetadata("copyNotSuggestedWhenActualFakeOverride")
public void testCopyNotSuggestedWhenActualFakeOverride() throws Exception {
runTest("testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/copyNotSuggestedWhenActualFakeOverride/");
}
@TestMetadata("copyNotSuggestedWhenActualTypealias")
public void testCopyNotSuggestedWhenActualTypealias() throws Exception {
runTest("testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/copyNotSuggestedWhenActualTypealias/");
}
@TestMetadata("copyToActualConstExpression")
public void testCopyToActualConstExpression() throws Exception {
runTest("testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/copyToActualConstExpression/");
@@ -159,6 +169,21 @@ public abstract class QuickFixMultiModuleTestGenerated extends AbstractQuickFixM
runTest("testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/removeFromExpect/");
}
@TestMetadata("removeFromExpectSuggestedWhenActualFakeOverride")
public void testRemoveFromExpectSuggestedWhenActualFakeOverride() throws Exception {
runTest("testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/removeFromExpectSuggestedWhenActualFakeOverride/");
}
@TestMetadata("removeFromExpectSuggestedWhenActualHasNoSource")
public void testRemoveFromExpectSuggestedWhenActualHasNoSource() throws Exception {
runTest("testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/removeFromExpectSuggestedWhenActualHasNoSource/");
}
@TestMetadata("removeFromExpectSuggestedWhenActualTypealias")
public void testRemoveFromExpectSuggestedWhenActualTypealias() throws Exception {
runTest("testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/removeFromExpectSuggestedWhenActualTypealias/");
}
@TestMetadata("replaceArgsOnActual")
public void testReplaceArgsOnActual() throws Exception {
runTest("testData/multiModuleQuickFix/actualAnnotationsNotMatchExpect/replaceArgsOnActual/");
@@ -0,0 +1,11 @@
// DISABLE-ERRORS
annotation class Ann
interface I {
fun foo() {}
}
expect class Foo : I {
@Ann
override fun foo()
}
@@ -0,0 +1,4 @@
MODULE common { platform=[JVM, JS, Native]; root=common }
MODULE jvm { platform=[JVM]; root=jvm }
jvm -> common { kind=DEPENDS_ON }
@@ -0,0 +1,6 @@
// "Copy mismatched annotation 'Ann' from expect to actual declaration (may change semantics)" "false"
// IGNORE_IRRELEVANT_ACTIONS
// DISABLE-ERRORS
// FIR_COMPARISON
actual class Fo<caret>o : I
@@ -0,0 +1,5 @@
// DISABLE-ERRORS
annotation class Ann
@Ann
expect class Foo
@@ -0,0 +1,4 @@
MODULE common { platform=[JVM, JS, Native]; root=common }
MODULE jvm { platform=[JVM]; root=jvm }
jvm -> common { kind=DEPENDS_ON }
@@ -0,0 +1,8 @@
// "Copy mismatched annotation 'Ann' from expect to actual declaration (may change semantics)" "false"
// IGNORE_IRRELEVANT_ACTIONS
// DISABLE-ERRORS
// FIR_COMPARISON
class FooImpl
actual typealias Foo<caret> = FooImpl
@@ -0,0 +1,11 @@
// DISABLE-ERRORS
annotation class Ann
interface I {
fun foo() {}
}
expect class Foo : I {
@Ann
override fun foo()
}
@@ -0,0 +1,10 @@
// DISABLE-ERRORS
annotation class Ann
interface I {
fun foo() {}
}
expect class Foo : I {
override fun foo()
}
@@ -0,0 +1,7 @@
MODULE common { platform=[JVM, JS, Native]; root=common }
MODULE jvm { platform=[JVM]; root=jvm }
common -> STDLIB_COMMON { kind=DEPENDENCY }
jvm -> common { kind=DEPENDS_ON }
jvm -> STDLIB_JVM { kind=DEPENDENCY }
@@ -0,0 +1,5 @@
// "Remove mismatched annotation 'Ann' from expect declaration (may change semantics)" "true"
// DISABLE-ERRORS
// FIR_COMPARISON
actual class Fo<caret>o : I
@@ -0,0 +1,5 @@
// DISABLE-ERRORS
annotation class Ann
@Ann
expect annotation class CommonSynchronized
@@ -0,0 +1,4 @@
// DISABLE-ERRORS
annotation class Ann
expect annotation class CommonSynchronized
@@ -0,0 +1,6 @@
MODULE common { platform=[JVM, JS, Native]; root=common }
MODULE jvm { platform=[JVM]; root=jvm }
common -> STDLIB_COMMON { kind=DEPENDENCY }
jvm -> common { kind=DEPENDS_ON }
jvm -> STDLIB_JVM { kind=DEPENDENCY }
@@ -0,0 +1,5 @@
// "Remove mismatched annotation 'Ann' from expect declaration (may change semantics)" "true"
// DISABLE-ERRORS
// FIR_COMPARISON
actual typealias CommonSynchronized<caret> = kotlin.jvm.Synchronized
@@ -0,0 +1,5 @@
// DISABLE-ERRORS
annotation class Ann
@Ann
expect class Foo
@@ -0,0 +1,4 @@
MODULE common { platform=[JVM, JS, Native]; root=common }
MODULE jvm { platform=[JVM]; root=jvm }
jvm -> common { kind=DEPENDS_ON }
@@ -0,0 +1,7 @@
// "Remove mismatched annotation 'Ann' from expect declaration (may change semantics)" "true"
// DISABLE-ERRORS
// FIR_COMPARISON
class FooImpl
actual typealias Foo<caret> = FooImpl