From 26340dea200d976ff884d6b2e85d5bb540fead99 Mon Sep 17 00:00:00 2001 From: Frederik Haselmeier Date: Mon, 25 Nov 2024 12:31:22 +0100 Subject: [PATCH] [kotlin] Add conflict if nested Kotlin properties/methods are moved that are referred to by Java code KTIJ-28862 GitOrigin-RevId: 6f7de83360a718c2ee5957e85716bb3ad3e8c181 --- .../messages/KotlinBundle.properties | 2 ++ .../refactoring/move/MoveTestGenerated.java | 5 +++ .../tests/testData/quickfix/optIn/override.kt | 2 +- .../conflicts.txt | 1 + .../moveToTopLevel/nameClash/nameClash.test | 2 +- .../conflicts.txt | 1 + .../conflicts.txt | 1 + .../after/bar/JavaClass.java | 7 +++++ .../after/bar/a.kt | 3 ++ .../after/bar/test.kt | 4 +++ .../before/bar/JavaClass.java | 7 +++++ .../before/bar/test.kt | 5 +++ .../conflicts.txt | 1 + .../externalPropertyUsageFromJava.test | 8 +++++ .../moveWithInstanceReference/conflicts.txt | 2 +- ...eNestedDeclarationsRefactoringProcessor.kt | 31 +++++++++++++++++-- .../k2/refactoring/move/ui/K2MoveModel.kt | 2 +- .../move/K2MoveNestedTestGenerated.java | 5 +++ 18 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/externalFunctionUsageFromJava/conflicts.txt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/outerInstanceDontAddParameter/conflicts.txt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveNestedClass/innerToTopLevelSimpleThisNoInstanceParameter/conflicts.txt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/JavaClass.java create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/a.kt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/test.kt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/before/bar/JavaClass.java create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/before/bar/test.kt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/conflicts.txt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/externalPropertyUsageFromJava.test diff --git a/plugins/kotlin/base/resources/resources-en/messages/KotlinBundle.properties b/plugins/kotlin/base/resources/resources-en/messages/KotlinBundle.properties index 6ff34391f0cb..c24974253e2f 100644 --- a/plugins/kotlin/base/resources/resources-en/messages/KotlinBundle.properties +++ b/plugins/kotlin/base/resources/resources-en/messages/KotlinBundle.properties @@ -1850,6 +1850,8 @@ searching.for.0=Searching for {0} move.out.of.companion.object=Move out of companion object calls.with.explicit.extension.receiver.won.t.be.processed.0=Calls with explicit extension receiver won''t be processed: {0} usages.of.outer.class.instance.inside.of.property.0.won.t.be.processed=Usages of outer class instance inside of property ''{0}'' won''t be processed +usages.of.outer.class.instance.inside.declaration.0.won.t.be.processed=Usages of outer class instance inside declaration ''{0}'' won''t be processed +usages.of.nested.declarations.from.non.kotlin.code.won.t.be.processed=Usages of nested declarations from non-Kotlin code won't be processed companion.object.already.contains.0=Companion object already contains {0} 0.references.type.parameters.of.the.containing.class={0} references type parameters of the containing class 0.is.overridden.by.declaration.s.in.a.subclass={0} is overridden by declaration(s) in a subclass diff --git a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java index 6960ecc27717..d6bb1283b1eb 100644 --- a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java +++ b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java @@ -1266,6 +1266,11 @@ public abstract class MoveTestGenerated extends AbstractMoveTest { runTest("testData/refactoring/moveNested/kotlin/moveNestedClass/protectedClass/protectedClass.test"); } + @TestMetadata("kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/externalPropertyUsageFromJava.test") + public void testKotlin_moveProperty_moveToTopLevel_externalPropertyUsageFromJava_ExternalPropertyUsageFromJava() throws Exception { + runTest("testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/externalPropertyUsageFromJava.test"); + } + @TestMetadata("kotlin/moveProperty/moveToTopLevel/moveWithInstanceReference/moveWithInstanceReference.test") public void testKotlin_moveProperty_moveToTopLevel_moveWithInstanceReference_MoveWithInstanceReference() throws Exception { runTest("testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/moveWithInstanceReference/moveWithInstanceReference.test"); diff --git a/plugins/kotlin/idea/tests/testData/quickfix/optIn/override.kt b/plugins/kotlin/idea/tests/testData/quickfix/optIn/override.kt index 476395016a0d..f14bd81f98b6 100644 --- a/plugins/kotlin/idea/tests/testData/quickfix/optIn/override.kt +++ b/plugins/kotlin/idea/tests/testData/quickfix/optIn/override.kt @@ -1,5 +1,5 @@ -// IGNORE_K1 // "Propagate 'MyExperimentalAPI' opt-in requirement to containing class 'Derived'" "false" +// IGNORE_K1 // COMPILER_ARGUMENTS: -opt-in=kotlin.RequiresOptIn // WITH_STDLIB // ACTION: Enable a trailing comma by default in the formatter diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/externalFunctionUsageFromJava/conflicts.txt b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/externalFunctionUsageFromJava/conflicts.txt new file mode 100644 index 000000000000..fbc026183b02 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/externalFunctionUsageFromJava/conflicts.txt @@ -0,0 +1 @@ +Usages of nested declarations from non-Kotlin code won't be processed diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/nameClash/nameClash.test b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/nameClash/nameClash.test index 1a1ce5660e55..129b07dfb6c9 100644 --- a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/nameClash/nameClash.test +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/nameClash/nameClash.test @@ -4,5 +4,5 @@ "outerInstanceParameter": "test", "withRuntime": "true", "enabledInK1": "false", - "enabledInK2": "true" + "enabledInK2": "false" } \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/outerInstanceDontAddParameter/conflicts.txt b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/outerInstanceDontAddParameter/conflicts.txt new file mode 100644 index 000000000000..d01d817f216d --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveMethod/moveToTopLevel/outerInstanceDontAddParameter/conflicts.txt @@ -0,0 +1 @@ +Usages of outer class instance inside declaration 'foo' won't be processed diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveNestedClass/innerToTopLevelSimpleThisNoInstanceParameter/conflicts.txt b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveNestedClass/innerToTopLevelSimpleThisNoInstanceParameter/conflicts.txt new file mode 100644 index 000000000000..af01cf86deb8 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveNestedClass/innerToTopLevelSimpleThisNoInstanceParameter/conflicts.txt @@ -0,0 +1 @@ +Usages of outer class instance inside declaration 'Inner' won't be processed diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/JavaClass.java b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/JavaClass.java new file mode 100644 index 000000000000..80c65eb55710 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/JavaClass.java @@ -0,0 +1,7 @@ +package bar; + +class JavaClass { + public void test() { + System.out.println(new Test().getA()); + } +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/a.kt b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/a.kt new file mode 100644 index 000000000000..ef08c32e2381 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/a.kt @@ -0,0 +1,3 @@ +package bar + +val a: Int = 5 \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/test.kt b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/test.kt new file mode 100644 index 000000000000..d9f58b5c68e6 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/after/bar/test.kt @@ -0,0 +1,4 @@ +package bar + +class Test { +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/before/bar/JavaClass.java b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/before/bar/JavaClass.java new file mode 100644 index 000000000000..80c65eb55710 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/before/bar/JavaClass.java @@ -0,0 +1,7 @@ +package bar; + +class JavaClass { + public void test() { + System.out.println(new Test().getA()); + } +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/before/bar/test.kt b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/before/bar/test.kt new file mode 100644 index 000000000000..b99dfc9b25c0 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/before/bar/test.kt @@ -0,0 +1,5 @@ +package bar + +class Test { + val a: Int = 5 +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/conflicts.txt b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/conflicts.txt new file mode 100644 index 000000000000..fbc026183b02 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/conflicts.txt @@ -0,0 +1 @@ +Usages of nested declarations from non-Kotlin code won't be processed diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/externalPropertyUsageFromJava.test b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/externalPropertyUsageFromJava.test new file mode 100644 index 000000000000..b43acee773be --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/externalPropertyUsageFromJava.test @@ -0,0 +1,8 @@ +{ + "mainFile": "bar/test.kt", + "type": "MOVE_KOTLIN_NESTED_DECLARATION", + "outerInstanceParameter": "test", + "withRuntime": "true", + "enabledInK1": "false", + "enabledInK2": "true" +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/moveWithInstanceReference/conflicts.txt b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/moveWithInstanceReference/conflicts.txt index 19f3ccabd563..d01d817f216d 100644 --- a/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/moveWithInstanceReference/conflicts.txt +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/moveWithInstanceReference/conflicts.txt @@ -1 +1 @@ -Usages of outer class instance inside of property 'foo' won't be processed +Usages of outer class instance inside declaration 'foo' won't be processed diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveNestedDeclarationsRefactoringProcessor.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveNestedDeclarationsRefactoringProcessor.kt index ebc16d9d4e61..5269613deb40 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveNestedDeclarationsRefactoringProcessor.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveNestedDeclarationsRefactoringProcessor.kt @@ -19,6 +19,7 @@ import org.jetbrains.kotlin.idea.k2.refactoring.move.descriptor.K2MoveOperationD import org.jetbrains.kotlin.idea.k2.refactoring.move.descriptor.K2MoveSourceDescriptor import org.jetbrains.kotlin.idea.k2.refactoring.move.descriptor.K2MoveTargetDescriptor import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.usages.ImplicitCompanionAsDispatchReceiverUsageInfo +import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.usages.K2MoveRenameUsageInfo import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.usages.OuterInstanceReferenceUsageInfo import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* @@ -49,6 +50,10 @@ class K2MoveNestedDeclarationsRefactoringProcessor( else -> MoveType.UNKNOWN } } + + init { + require(operationDescriptor.sourceElements.size == 1) { "We can only move a single nested declaration at a time" } + } private val elementToMove = operationDescriptor.sourceElements.single() private fun willLoseOuterInstanceReference(): Boolean { @@ -68,6 +73,19 @@ class K2MoveNestedDeclarationsRefactoringProcessor( val element = usage.element ?: continue val isConflict = when (usage) { + is K2MoveRenameUsageInfo.Light -> { + if (moveType != MoveType.CLASS && operationDescriptor.outerInstanceParameterName != null) { + // We only have the facility to correct outer class usages if the moved declaration is a nested class. + conflicts.putValue( + element, + KotlinBundle.message("usages.of.nested.declarations.from.non.kotlin.code.won.t.be.processed", element.text) + ) + true + } else { + false + } + } + is ImplicitCompanionAsDispatchReceiverUsageInfo -> { val isValidTarget = isValidTargetForImplicitCompanionAsDispatchReceiver(moveDescriptor.target, usage.companionObject) if (!isValidTarget) { @@ -80,11 +98,13 @@ class K2MoveNestedDeclarationsRefactoringProcessor( } is OuterInstanceReferenceUsageInfo -> { - if (moveType == MoveType.PROPERTY) { - // For properties, any outer instance reference is a conflict because we do not process them. + if (willLoseOuterInstanceReference()) { conflicts.putValue( element, - KotlinBundle.message("usages.of.outer.class.instance.inside.of.property.0.won.t.be.processed", elementToMove.nameAsSafeName.asString()) + KotlinBundle.message( + "usages.of.outer.class.instance.inside.declaration.0.won.t.be.processed", + elementToMove.nameAsSafeName.asString() + ) ) true } else { @@ -117,6 +137,8 @@ class K2MoveNestedDeclarationsRefactoringProcessor( val outerClass = referencedNestedDeclaration.containingClassOrObject val lightOuterClass = outerClass?.toLightClass() if (lightOuterClass != null) { + // While this is called the `MoveInnerClassUsagesHandler`, it will also correctly modify usages + // of inner methods from Kotlin code (but not from Java code!). MoveInnerClassUsagesHandler.EP_NAME .forLanguage(usage.element?.language ?: continue) ?.correctInnerClassUsage(usage, lightOuterClass, outerInstanceParameterName) @@ -166,6 +188,7 @@ class K2MoveNestedDeclarationsRefactoringProcessor( } } } + is KtNamedFunction -> { if (outerInstanceParameterName != null) { val outerInstanceType = analyze(originalDeclaration) { @@ -181,6 +204,7 @@ class K2MoveNestedDeclarationsRefactoringProcessor( ) } } + is KtProperty -> { } @@ -200,6 +224,7 @@ class K2MoveNestedDeclarationsRefactoringProcessor( val addedParameter = primaryConstructor.valueParameters.firstOrNull { it.name == outerInstanceParameterName } ?: return shortenReferences(addedParameter) } + is KtNamedFunction -> { val addedParameter = newDeclaration.valueParameterList?.parameters?.firstOrNull() ?: return shortenReferences(addedParameter) diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/ui/K2MoveModel.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/ui/K2MoveModel.kt index 8d362df7c415..c6d54b6ec1e6 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/ui/K2MoveModel.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/ui/K2MoveModel.kt @@ -270,7 +270,7 @@ sealed class K2MoveModel { searchReferences = searchReferences.state, dirStructureMatchesPkg = true, newClassName = null, - outerInstanceParameterName = outerClassInstanceParameterName.takeIf { needsInstanceReference }, + outerInstanceParameterName = outerClassInstanceParameterName.takeIf { passOuterClass }, moveCallBack = moveCallBack ) } diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/K2MoveNestedTestGenerated.java b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/K2MoveNestedTestGenerated.java index 7dd7fa80941a..efb6e5e0e749 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/K2MoveNestedTestGenerated.java +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/K2MoveNestedTestGenerated.java @@ -390,6 +390,11 @@ public class K2MoveNestedTestGenerated extends AbstractK2MoveNestedTest { runTest("../../idea/tests/testData/refactoring/moveNested/kotlin/moveNestedClass/protectedClass/protectedClass.test"); } + @TestMetadata("kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/externalPropertyUsageFromJava.test") + public void testKotlin_moveProperty_moveToTopLevel_externalPropertyUsageFromJava_ExternalPropertyUsageFromJava() throws Exception { + runTest("../../idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/externalPropertyUsageFromJava/externalPropertyUsageFromJava.test"); + } + @TestMetadata("kotlin/moveProperty/moveToTopLevel/moveWithInstanceReference/moveWithInstanceReference.test") public void testKotlin_moveProperty_moveToTopLevel_moveWithInstanceReference_MoveWithInstanceReference() throws Exception { runTest("../../idea/tests/testData/refactoring/moveNested/kotlin/moveProperty/moveToTopLevel/moveWithInstanceReference/moveWithInstanceReference.test");