[kotlin] Run import optimizer when no usages are found

Fixes the issue in Kotlin, Java and Groovy. #KTIJ-28288 Fixed

GitOrigin-RevId: 7cd5942539a10f9ccdba386396f15546b0a9ac00
This commit is contained in:
Bart van Helvert
2024-01-11 18:32:37 +01:00
committed by intellij-monorepo-bot
parent f33959ccc5
commit dc11424f70
36 changed files with 154 additions and 33 deletions

View File

@@ -15,8 +15,10 @@ import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.usageView.UsageInfo;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.ObjectUtils;
import com.intellij.util.SequentialModalProgressTask;
import com.intellij.util.SequentialTask;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
import java.util.*;
@@ -38,12 +40,13 @@ public final class OptimizeImportsRefactoringHelper implements RefactoringHelper
@Override
public Set<PsiJavaFile> prepareOperation(UsageInfo @NotNull [] usages, @NotNull PsiElement primaryElement) {
Set<PsiJavaFile> files = prepareOperation(usages);
PsiFile containingFile = primaryElement.getContainingFile();
if (containingFile instanceof PsiJavaFile) {
files.add((PsiJavaFile)containingFile);
}
return files;
return prepareOperation(usages, List.of(primaryElement));
}
@Override
public Set<PsiJavaFile> prepareOperation(UsageInfo @NotNull [] usages, List<@NotNull PsiElement> elements) {
Set<PsiJavaFile> movedFiles = ContainerUtil.map2SetNotNull(elements, e -> ObjectUtils.tryCast(e.getContainingFile(), PsiJavaFile.class));
return ContainerUtil.union(movedFiles, prepareOperation(usages));
}
@Override

View File

@@ -1,6 +1,4 @@
// "Safe delete 'i'" "true-preview"
import java.io.*;
class a {
private void run(<caret>) {
}

View File

@@ -1,6 +1,4 @@
// "Safe delete 'i'" "true-preview"
import java.io.*;
class a {
private void run(int i1, <caret>int ee) {
}

View File

@@ -1,6 +1,4 @@
// "Safe delete 'i'" "true-preview"
import java.io.*;
class a {
private void run(int <caret>i) {
}

View File

@@ -1,6 +1,4 @@
// "Safe delete 'i'" "true-preview"
import java.io.*;
class a {
private void run(int i1, int <caret>i, int ee) {
}

View File

@@ -0,0 +1,7 @@
package pack2;
public class X {
public static void main(String[] args) {
Y().foo();
}
}

View File

@@ -0,0 +1,5 @@
package pack2;
public class Y {
void foo() { }
}

View File

@@ -0,0 +1,9 @@
package pack1;
import pack2.Y;
public class X {
public static void main(String[] args) {
Y().foo();
}
}

View File

@@ -0,0 +1,5 @@
package pack2;
public class Y {
void foo() { }
}

View File

@@ -16,14 +16,14 @@ public class MoveToPackageTest extends LightJavaCodeInsightFixtureTestCase {
PsiClass bClass = myFixture.addClass("package foo;\nimport bar.A;\npublic final class B extends A {}");
myFixture.configureByText("A.java", "package bar;\n import foo.B;\npublic sealed class A permits <caret>B {}");
invokeFix("Move to package 'bar'");
assertEquals("package bar;\nimport bar.A;\npublic final class B extends A {}", bClass.getContainingFile().getText());
assertEquals("package bar;\n\npublic final class B extends A {}", bClass.getContainingFile().getText());
}
public void testNestedClass() {
PsiClass bClass = myFixture.addClass("package foo;\nimport bar.A;\npublic class B { public static final class C extends A {} }");
myFixture.configureByText("A.java", "package bar;\n import foo.B;\npublic sealed class A permits <caret>B.C {}");
invokeFix("Move to package 'bar'");
assertEquals("package bar;\nimport bar.A;\npublic class B { public static final class C extends A {} }",
assertEquals("package bar;\n\npublic class B { public static final class C extends A {} }",
bClass.getContainingFile().getText());
}

View File

@@ -93,6 +93,10 @@ public class MoveClassTest extends LightMultiFileTestCase {
doTest(new String[]{"p2.F2"}, "p1");
}
public void testWithoutReference() {
doTest(new String[]{"pack1.X"}, "pack2");
}
public void testQualifiedRef() {
doTest(new String[]{"p1.Test"}, "p2");
}

View File

@@ -505,10 +505,16 @@ public abstract class BaseRefactoringProcessor implements Runnable {
final Map<RefactoringHelper, Object> preparedData = new LinkedHashMap<>();
final Runnable prepareHelpersRunnable = () -> {
RefactoringEventData data = ReadAction.compute(() -> getBeforeData());
PsiElement primaryElement = data != null ? data.getUserData(RefactoringEventData.PSI_ELEMENT_KEY) : null;
for (final RefactoringHelper helper : RefactoringHelper.EP_NAME.getExtensionList()) {
Object operation = ReadAction.compute(() -> primaryElement != null ? helper.prepareOperation(writableUsageInfos, primaryElement)
: helper.prepareOperation(writableUsageInfos));
Object operation = ReadAction.compute(() -> {
if (data != null) {
PsiElement primaryElement = data.getUserData(RefactoringEventData.PSI_ELEMENT_KEY);
if (primaryElement != null) return helper.prepareOperation(writableUsageInfos, primaryElement);
PsiElement[] elements = data.getUserData(RefactoringEventData.PSI_ELEMENT_ARRAY_KEY);
if (elements != null) return helper.prepareOperation(writableUsageInfos, ContainerUtil.filter(elements, e -> e != null));
}
return helper.prepareOperation(writableUsageInfos);
});
preparedData.put(helper, operation);
}
};

View File

@@ -8,6 +8,8 @@ import com.intellij.psi.PsiElement;
import com.intellij.usageView.UsageInfo;
import org.jetbrains.annotations.NotNull;
import java.util.List;
/**
* Allows performing cleanup operations after refactoring is done e.g., optimize imports
*/
@@ -29,6 +31,14 @@ public interface RefactoringHelper<T> {
return prepareOperation(usages);
}
/**
* Is used when refactoring provides {@link BaseRefactoringProcessor#getBeforeData()} with all elements,
* otherwise {@link #prepareOperation(UsageInfo[])} is used instead
*/
default T prepareOperation(UsageInfo @NotNull [] usages, List<@NotNull PsiElement> elements) {
return prepareOperation(usages);
}
/**
* Is invoked in EDT, without WriteAction after refactoring is performed
*

View File

@@ -10,9 +10,12 @@ import com.intellij.openapi.project.Project;
import com.intellij.openapi.roots.ProjectRootManager;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiFile;
import com.intellij.refactoring.RefactoringHelper;
import com.intellij.usageView.UsageInfo;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.plugins.groovy.GroovyBundle;
import org.jetbrains.plugins.groovy.lang.psi.GroovyFile;
@@ -40,6 +43,17 @@ public class GroovyImportOptimizerRefactoringHelper implements RefactoringHelper
return files;
}
@Override
public Set<GroovyFile> prepareOperation(UsageInfo @NotNull [] usages, @NotNull PsiElement primaryElement) {
return prepareOperation(usages, List.of(primaryElement));
}
@Override
public Set<GroovyFile> prepareOperation(UsageInfo @NotNull [] usages, List<@NotNull PsiElement> elements) {
Set<GroovyFile> movedFiles = ContainerUtil.map2SetNotNull(elements, e -> ObjectUtils.tryCast(e.getContainingFile(), GroovyFile.class));
return ContainerUtil.union(movedFiles, prepareOperation(usages));
}
@Override
public void performOperation(@NotNull final Project project, final Set<GroovyFile> files) {
final ProgressManager progressManager = ProgressManager.getInstance();

View File

@@ -65,6 +65,10 @@ class GroovyMoveClassTest extends GroovyMoveTestBase {
doTest("p2", "p1.C1")
}
void testWithoutReference() {
doTest("p2", "p1.X")
}
boolean perform(VirtualFile root, String newPackageName, String... classNames) {
final PsiClass[] classes = new PsiClass[classNames.length]
for (int i = 0; i < classes.length; i++) {

View File

@@ -0,0 +1,7 @@
package p2
class X {
static void main(String[] args) {
Y().foo()
}
}

View File

@@ -0,0 +1,5 @@
package p2
class Y {
void foo() { }
}

View File

@@ -0,0 +1,9 @@
package p1
import p2.Y
class X {
static void main(String[] args) {
Y().foo()
}
}

View File

@@ -0,0 +1,5 @@
package p2
class Y {
void foo() { }
}

View File

@@ -1,7 +1,6 @@
package b
import a.A;
import b.B;
import a.A
def a = new A().foo();
print a;

View File

@@ -1,7 +1,6 @@
package b
import a.A;
import b.B;
import a.A
def a = new A().foo();
print a;

View File

@@ -1,6 +1,3 @@
import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleIntegerProperty;
class Test {
}

View File

@@ -333,6 +333,11 @@ public class MoveTestGenerated extends AbstractMoveTest {
runTest("testData/refactoring/move/kotlin/moveFile/typeRefWithArguments/typeRefWithArguments.test");
}
@TestMetadata("kotlin/moveFile/withoutUsages/withoutUsages.test")
public void testKotlin_moveFile_withoutUsages_WithoutUsages() throws Exception {
runTest("testData/refactoring/move/kotlin/moveFile/withoutUsages/withoutUsages.test");
}
@TestMetadata("kotlin/moveMethod/moveToClass/companionHasReference/companionHasReference.test")
public void testKotlin_moveMethod_moveToClass_companionHasReference_CompanionHasReference() throws Exception {
runTest("testData/refactoring/move/kotlin/moveMethod/moveToClass/companionHasReference/companionHasReference.test");

View File

@@ -0,0 +1,5 @@
package b
class Y {
fun foo() { }
}

View File

@@ -0,0 +1,7 @@
package b
// this import should be removed by the import optimizer even though there are no available usages
fun main() {
Y().foo()
}

View File

@@ -0,0 +1,8 @@
package a
// this import should be removed by the import optimizer even though there are no available usages
import b.Y
fun main() {
Y().foo()
}

View File

@@ -0,0 +1,5 @@
package b
class Y {
fun foo() { }
}

View File

@@ -0,0 +1,7 @@
{
"mainFile": "a/main.kt",
"type": "MOVE_FILES",
"targetPackage": "b",
"enabledInK1": "true",
"enabledInK2": "true"
}

View File

@@ -92,13 +92,12 @@ class KotlinOptimizeImportsRefactoringHelper : RefactoringHelper<Set<KtFile>> {
if (!it.isNonCodeUsage) it.file as? KtFile else null
}
override fun prepareOperation(usages: Array<out UsageInfo>, primaryElement: PsiElement): Set<KtFile> {
val files = super.prepareOperation(usages, primaryElement)
val file = primaryElement.containingFile
if (file is KtFile) {
(files as LinkedHashSet<KtFile>).add(file)
}
return files
override fun prepareOperation(usages: Array<UsageInfo>, primaryElement: PsiElement): Set<KtFile> {
return prepareOperation(usages, listOf(primaryElement))
}
override fun prepareOperation(usages: Array<UsageInfo>, elements: List<PsiElement>): Set<KtFile> {
return elements.mapNotNull { it.containingFile as? KtFile }.toSet() + prepareOperation(usages)
}
override fun performOperation(project: Project, operationData: Set<KtFile>) {

View File

@@ -333,6 +333,11 @@ public class K2MoveTestGenerated extends AbstractK2MoveTest {
runTest("../../idea/tests/testData/refactoring/move/kotlin/moveFile/typeRefWithArguments/typeRefWithArguments.test");
}
@TestMetadata("kotlin/moveFile/withoutUsages/withoutUsages.test")
public void testKotlin_moveFile_withoutUsages_WithoutUsages() throws Exception {
runTest("../../idea/tests/testData/refactoring/move/kotlin/moveFile/withoutUsages/withoutUsages.test");
}
@TestMetadata("kotlin/moveMethod/moveToClass/companionHasReference/companionHasReference.test")
public void testKotlin_moveMethod_moveToClass_companionHasReference_CompanionHasReference() throws Exception {
runTest("../../idea/tests/testData/refactoring/move/kotlin/moveMethod/moveToClass/companionHasReference/companionHasReference.test");