From 8b2f420ab3f3074555cdcdf0a3ecc084e9830b8d Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Tue, 6 Sep 2016 17:41:25 +0300 Subject: [PATCH] IDEA-151933 Optimize Imports can produce ambiguous code --- .../impl/source/codeStyle/ImportHelper.java | 141 ++++++++++-------- .../bar/C.java | 13 ++ .../bar/D.java | 13 ++ .../bar/D1.java | 10 ++ .../bar/D2.java | 10 ++ .../bar/D3.java | 10 ++ .../bar/D4.java | 10 ++ .../foo/A.java | 22 +++ .../foo/B.java | 19 +++ .../reimportConflictingClasses/x/Usage.java | 0 .../x/Usage_afterOptimize.txt | 0 .../reimportConflictingClasses/x/n/Class.java | 0 .../reimportConflictingClasses/x/n/X1.java | 0 .../reimportConflictingClasses/x/n/X2.java | 0 .../reimportConflictingClasses/x/n/X3.java | 0 .../codeInsight/daemon/ImportHelperTest.java | 43 +++--- .../codeInsight/JavaFxImportsOptimizer.java | 11 +- 17 files changed, 212 insertions(+), 90 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/C.java create mode 100644 java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D.java create mode 100644 java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D1.java create mode 100644 java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D2.java create mode 100644 java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D3.java create mode 100644 java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D4.java create mode 100644 java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/foo/A.java create mode 100644 java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/foo/B.java rename java/java-tests/testData/codeInsight/{daemonCodeAnalyzer/advHighlighting => importHelper}/reimportConflictingClasses/x/Usage.java (100%) rename java/java-tests/testData/codeInsight/{daemonCodeAnalyzer/advHighlighting => importHelper}/reimportConflictingClasses/x/Usage_afterOptimize.txt (100%) rename java/java-tests/testData/codeInsight/{daemonCodeAnalyzer/advHighlighting => importHelper}/reimportConflictingClasses/x/n/Class.java (100%) rename java/java-tests/testData/codeInsight/{daemonCodeAnalyzer/advHighlighting => importHelper}/reimportConflictingClasses/x/n/X1.java (100%) rename java/java-tests/testData/codeInsight/{daemonCodeAnalyzer/advHighlighting => importHelper}/reimportConflictingClasses/x/n/X2.java (100%) rename java/java-tests/testData/codeInsight/{daemonCodeAnalyzer/advHighlighting => importHelper}/reimportConflictingClasses/x/n/X3.java (100%) diff --git a/java/java-impl/src/com/intellij/psi/impl/source/codeStyle/ImportHelper.java b/java/java-impl/src/com/intellij/psi/impl/source/codeStyle/ImportHelper.java index cd93c65c02b3..cb633917d044 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/codeStyle/ImportHelper.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/codeStyle/ImportHelper.java @@ -45,6 +45,7 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.ClassUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; +import com.intellij.util.ObjectUtils; import com.intellij.util.containers.ContainerUtil; import gnu.trove.THashMap; import gnu.trove.THashSet; @@ -52,8 +53,10 @@ import gnu.trove.TObjectIntHashMap; import gnu.trove.TObjectIntProcedure; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.*; +import java.util.stream.Collectors; public class ImportHelper{ private static final Logger LOG = Logger.getInstance("#com.intellij.psi.impl.source.codeStyle.ImportHelper"); @@ -65,7 +68,8 @@ public class ImportHelper{ mySettings = settings; } - public PsiImportList prepareOptimizeImportsResult(@NotNull final PsiJavaFile file) { + @Nullable("null means no need to replace the import list because they are the same") + PsiImportList prepareOptimizeImportsResult(@NotNull final PsiJavaFile file) { PsiImportList oldList = file.getImportList(); if (oldList == null) return null; @@ -81,16 +85,16 @@ public class ImportHelper{ List> resultList = sortItemsAccordingToSettings(names, mySettings); - final Set classesOrPackagesToImportOnDemand = new THashSet<>(); - collectOnDemandImports(resultList, classesOrPackagesToImportOnDemand, this.mySettings); + final Map classesOrPackagesToImportOnDemand = new THashMap<>(); + collectOnDemandImports(resultList, mySettings, classesOrPackagesToImportOnDemand); - Set classesToUseSingle = findSingleImports(file, resultList, classesOrPackagesToImportOnDemand); + Set classesToUseSingle = findSingleImports(file, resultList, classesOrPackagesToImportOnDemand.keySet()); Set toReimport = new THashSet<>(); calcClassesConflictingViaOnDemandImports(file, classesOrPackagesToImportOnDemand, file.getResolveScope(), toReimport); classesToUseSingle.addAll(toReimport); try { - StringBuilder text = buildImportListText(resultList, classesOrPackagesToImportOnDemand, classesToUseSingle); + StringBuilder text = buildImportListText(resultList, classesOrPackagesToImportOnDemand.keySet(), classesToUseSingle); for (PsiElement nonImport : nonImports) { text.append("\n").append(nonImport.getText()); } @@ -124,9 +128,9 @@ public class ImportHelper{ } } - public static void collectOnDemandImports(List> resultList, - final Set classesOrPackagesToImportOnDemand, - final CodeStyleSettings settings) { + public static void collectOnDemandImports(@NotNull List> resultList, + @NotNull CodeStyleSettings settings, + @NotNull Map outClassesOrPackagesToImportOnDemand) { TObjectIntHashMap packageToCountMap = new TObjectIntHashMap<>(); TObjectIntHashMap classToCountMap = new TObjectIntHashMap<>(); for (Pair pair : resultList) { @@ -148,14 +152,15 @@ public class ImportHelper{ class MyVisitorProcedure implements TObjectIntProcedure { private final boolean myIsVisitingPackages; - MyVisitorProcedure(boolean isVisitingPackages) { + private MyVisitorProcedure(boolean isVisitingPackages) { myIsVisitingPackages = isVisitingPackages; } @Override public boolean execute(final String packageOrClassName, final int count) { if (isToUseImportOnDemand(packageOrClassName, count, !myIsVisitingPackages, settings)){ - classesOrPackagesToImportOnDemand.add(packageOrClassName); + boolean isStatic = !myIsVisitingPackages; + outClassesOrPackagesToImportOnDemand.put(packageOrClassName, isStatic); } return true; } @@ -192,20 +197,16 @@ public class ImportHelper{ @NotNull private static Set findSingleImports(@NotNull final PsiJavaFile file, @NotNull List> names, - @NotNull final Set onDemandImports - ) { + @NotNull final Set onDemandImports) { final GlobalSearchScope resolveScope = file.getResolveScope(); - Set namesToUseSingle = new THashSet<>(); final String thisPackageName = file.getPackageName(); final Set implicitlyImportedPackages = new THashSet<>(Arrays.asList(file.getImplicitlyImportedPackages())); final PsiManager manager = file.getManager(); JavaPsiFacade facade = JavaPsiFacade.getInstance(manager.getProject()); - List onDemandElements = new ArrayList<>(onDemandImports.size()); + List onDemandImportsList = new ArrayList<>(onDemandImports); - for (String onDemandName : onDemandImportsList) { - PsiElement aClass = facade.findClass(onDemandName, resolveScope); - onDemandElements.add(aClass); - } + List onDemandElements = onDemandImportsList.stream().map(onDemandName -> facade.findClass(onDemandName, resolveScope)).collect(Collectors.toList()); + Set namesToUseSingle = new THashSet<>(); for (Pair pair : names) { String name = pair.getFirst(); Boolean isStatic = pair.getSecond(); @@ -231,8 +232,7 @@ public class ImportHelper{ String onDemandName = onDemandImportsList.get(i); if (prefix.equals(onDemandName)) continue; if (isStatic) { - PsiElement element = onDemandElements.get(i); - PsiClass aClass = (PsiClass)element; + PsiClass aClass = onDemandElements.get(i); if (aClass != null) { PsiField field = aClass.findFieldByName(shortName, true); if (field != null && field.hasModifierProperty(PsiModifier.STATIC)) { @@ -266,31 +266,38 @@ public class ImportHelper{ return namesToUseSingle; } - private static void calcClassesConflictingViaOnDemandImports(PsiJavaFile file, Collection onDemandImportsList, - GlobalSearchScope resolveScope, final Set namesToUseSingle) { + private static void calcClassesConflictingViaOnDemandImports(@NotNull PsiJavaFile file, + @NotNull Map onDemandImports, + @NotNull GlobalSearchScope resolveScope, + @NotNull Set outNamesToUseSingle) { List onDemands = new ArrayList<>(Arrays.asList(file.getImplicitlyImportedPackages())); - for (String onDemand : onDemandImportsList) { + for (String onDemand : onDemandImports.keySet()) { if (!onDemands.contains(onDemand)) { onDemands.add(onDemand); } } if (onDemands.size() < 2) return; + // if we have classes x.A, x.B and there is an "import x.*" then classNames = {"x" -> ("A", "B")} Map> classNames = new THashMap<>(); JavaPsiFacade facade = JavaPsiFacade.getInstance(file.getProject()); for (int i = onDemands.size()-1; i>=0; i--) { String onDemand = onDemands.get(i); PsiPackage aPackage = facade.findPackage(onDemand); - if (aPackage == null) { + boolean isStatic = ObjectUtils.notNull(onDemandImports.get(onDemand), Boolean.FALSE); + PsiClass aClass; + if (aPackage != null) { // import foo.package1.*; + Set set = Arrays.stream(aPackage.getClasses(resolveScope)).map(PsiClass::getName).collect(Collectors.toSet()); + classNames.put(onDemand, set); + } + else if (isStatic && (aClass = facade.findClass(onDemand, resolveScope)) != null) { // import static foo.package1.Class1.*; + PsiMember[][] membersArray = {aClass.getInnerClasses(), aClass.getMethods(), aClass.getFields()}; + Set set = Arrays.stream(membersArray).flatMap(Arrays::stream).map(PsiMember::getName).collect(Collectors.toSet()); + classNames.put(onDemand, set); + } + else { onDemands.remove(i); - continue; } - PsiClass[] psiClasses = aPackage.getClasses(resolveScope); - Set set = new THashSet<>(psiClasses.length); - for (PsiClass psiClass : psiClasses) { - set.add(psiClass.getName()); - } - classNames.put(onDemand, set); } final Set conflicts = new THashSet<>(); @@ -298,10 +305,10 @@ public class ImportHelper{ String on1 = onDemands.get(i); for (int j = i+1; j < onDemands.size(); j++) { String on2 = onDemands.get(j); - Set inter = new THashSet<>(classNames.get(on1)); - inter.retainAll(classNames.get(on2)); + Set intersection = new THashSet<>(classNames.get(on1)); + intersection.retainAll(classNames.get(on2)); - conflicts.addAll(inter); + conflicts.addAll(intersection); } } if (!conflicts.isEmpty() && !(file instanceof PsiCompiledElement)) { @@ -312,7 +319,7 @@ public class ImportHelper{ PsiElement element = reference.resolve(); if (element instanceof PsiClass && conflicts.contains(((PsiClass)element).getName())) { String fqn = ((PsiClass)element).getQualifiedName(); - namesToUseSingle.add(fqn); + outNamesToUseSingle.add(fqn); } } }); @@ -421,7 +428,7 @@ public class ImportHelper{ String refName = ref instanceof PsiClass ? ((PsiClass)ref).getQualifiedName() : ((PsiPackage)ref).getQualifiedName(); refTexts.add(refName); } - calcClassesToReimport(file, facade, helper, packageName, classesToReimport, refTexts); + calcClassesToReimport(file, facade, helper, packageName, refTexts, classesToReimport); } } @@ -474,31 +481,36 @@ public class ImportHelper{ return false; } - private static void calcClassesToReimport(PsiJavaFile file, JavaPsiFacade facade, PsiResolveHelper helper, String packageName, List classesToReimport, - Collection onDemandRefs) { + private static void calcClassesToReimport(@NotNull PsiJavaFile file, + @NotNull JavaPsiFacade facade, + @NotNull PsiResolveHelper helper, + @NotNull String packageName, + @NotNull Collection onDemandRefs, + @NotNull List outClassesToReimport) { if (onDemandRefs.isEmpty()) { return; } PsiPackage aPackage = facade.findPackage(packageName); - if (aPackage != null) { - PsiDirectory[] dirs = aPackage.getDirectories(); - GlobalSearchScope resolveScope = file.getResolveScope(); - for (PsiDirectory dir : dirs) { - PsiFile[] files = dir.getFiles(); // do not iterate classes - too slow when not loaded - for (PsiFile aFile : files) { - if (!(aFile instanceof PsiJavaFile)) continue; - String name = aFile.getVirtualFile().getNameWithoutExtension(); - for (String refName : onDemandRefs) { - String conflictClassName = refName + "." + name; - PsiClass conflictClass = facade.findClass(conflictClassName, resolveScope); - if (conflictClass == null || !helper.isAccessible(conflictClass, file, null)) continue; - String conflictClassName2 = packageName + "." + name; - PsiClass conflictClass2 = facade.findClass(conflictClassName2, resolveScope); - if (conflictClass2 != null && - helper.isAccessible(conflictClass2, file, null) && - ReferencesSearch.search(conflictClass, new LocalSearchScope(file), false).findFirst() != null) { - classesToReimport.add(conflictClass); - } + if (aPackage == null) { + return; + } + PsiDirectory[] dirs = aPackage.getDirectories(); + GlobalSearchScope resolveScope = file.getResolveScope(); + for (PsiDirectory dir : dirs) { + PsiFile[] files = dir.getFiles(); // do not iterate classes - too slow when not loaded + for (PsiFile aFile : files) { + if (!(aFile instanceof PsiJavaFile)) continue; + String name = aFile.getVirtualFile().getNameWithoutExtension(); + for (String refName : onDemandRefs) { + String conflictClassName = refName + "." + name; + PsiClass conflictClass = facade.findClass(conflictClassName, resolveScope); + if (conflictClass == null || !helper.isAccessible(conflictClass, file, null)) continue; + String conflictClassName2 = packageName + "." + name; + PsiClass conflictClass2 = facade.findClass(conflictClassName2, resolveScope); + if (conflictClass2 != null && + helper.isAccessible(conflictClass2, file, null) && + ReferencesSearch.search(conflictClass, new LocalSearchScope(file), false).findFirst() != null) { + outClassesToReimport.add(conflictClass); } } } @@ -681,14 +693,14 @@ public class ImportHelper{ private static boolean isToUseImportOnDemand(@NotNull String packageName, int classCount, boolean isStaticImportNeeded, - final CodeStyleSettings settings){ + @NotNull CodeStyleSettings settings){ if (!settings.USE_SINGLE_CLASS_IMPORTS) return true; int limitCount = isStaticImportNeeded ? settings.NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND : settings.CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND; if (classCount >= limitCount) return true; if (packageName.isEmpty()) return false; PackageEntryTable table = settings.PACKAGES_TO_USE_IMPORT_ON_DEMAND; - return table != null && table.contains(packageName); + return table.contains(packageName); } private static int findEntryIndex(@NotNull String packageName, boolean isStatic, @NotNull PackageEntry[] entries) { @@ -716,7 +728,7 @@ public class ImportHelper{ return bestEntryIndex; } - public int findEntryIndex(@NotNull PsiImportStatementBase statement){ + int findEntryIndex(@NotNull PsiImportStatementBase statement){ PsiJavaCodeReferenceElement ref = statement.getImportReference(); if (ref == null) return -1; String packageName; @@ -785,8 +797,8 @@ public class ImportHelper{ continue; } IElementType elementType = node.getElementType(); - if (elementType != null &&!ElementType.IMPORT_STATEMENT_BASE_BIT_SET.contains(elementType) - && !JavaJspElementType.WHITE_SPACE_BIT_SET.contains(elementType)) + if (!ElementType.IMPORT_STATEMENT_BASE_BIT_SET.contains(elementType) && + !JavaJspElementType.WHITE_SPACE_BIT_SET.contains(elementType)) { comments.add(element); } @@ -909,7 +921,7 @@ public class ImportHelper{ // otherwise, optimize out all red on demand imports for green file } - public static boolean isImplicitlyImported(@NotNull String className, @NotNull PsiJavaFile file) { + static boolean isImplicitlyImported(@NotNull String className, @NotNull PsiJavaFile file) { String[] packageNames = file.getImplicitlyImportedPackages(); for (String packageName : packageNames) { if (hasPackage(className, packageName)) return true; @@ -917,7 +929,7 @@ public class ImportHelper{ return false; } - public static boolean hasPackage(@NotNull String className, @NotNull String packageName){ + static boolean hasPackage(@NotNull String className, @NotNull String packageName){ if (!className.startsWith(packageName)) return false; if (className.length() == packageName.length()) return false; if (!packageName.isEmpty() && className.charAt(packageName.length()) != '.') return false; @@ -929,5 +941,4 @@ public class ImportHelper{ int dotIndex = className.lastIndexOf('.'); return dotIndex < 0 ? "" : className.substring(0, dotIndex); } - } diff --git a/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/C.java b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/C.java new file mode 100644 index 000000000000..1baa9f0b8930 --- /dev/null +++ b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/C.java @@ -0,0 +1,13 @@ +package bar; + +/** + * ******************************* + * Created by Irina.Petrovskaya on 8/31/2016. + * Project: untitled1 + * ******************************* + */ +public class C { + public C() { + System.out.println("com.example.bar.C init"); + } +} diff --git a/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D.java b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D.java new file mode 100644 index 000000000000..e3017d9c0f3a --- /dev/null +++ b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D.java @@ -0,0 +1,13 @@ +package bar; + +/** + * ******************************* + * Created by Irina.Petrovskaya on 8/31/2016. + * Project: untitled1 + * ******************************* + */ +public class D { + public D() { + System.out.println("com.example.bar.D init"); + } +} diff --git a/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D1.java b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D1.java new file mode 100644 index 000000000000..e328ee8c602b --- /dev/null +++ b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D1.java @@ -0,0 +1,10 @@ +package bar; + +/** + * ******************************* + * Created by Irina.Petrovskaya on 8/31/2016. + * Project: untitled1 + * ******************************* + */ +public class D1 { +} diff --git a/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D2.java b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D2.java new file mode 100644 index 000000000000..24678c50650b --- /dev/null +++ b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D2.java @@ -0,0 +1,10 @@ +package bar; + +/** + * ******************************* + * Created by Irina.Petrovskaya on 8/31/2016. + * Project: untitled1 + * ******************************* + */ +public class D2 { +} diff --git a/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D3.java b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D3.java new file mode 100644 index 000000000000..0edbeafb0257 --- /dev/null +++ b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D3.java @@ -0,0 +1,10 @@ +package bar; + +/** + * ******************************* + * Created by Irina.Petrovskaya on 8/31/2016. + * Project: untitled1 + * ******************************* + */ +public class D3 { +} diff --git a/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D4.java b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D4.java new file mode 100644 index 000000000000..a3beae6975d7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/bar/D4.java @@ -0,0 +1,10 @@ +package bar; + +/** + * ******************************* + * Created by Irina.Petrovskaya on 8/31/2016. + * Project: untitled1 + * ******************************* + */ +public class D4 { +} diff --git a/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/foo/A.java b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/foo/A.java new file mode 100644 index 000000000000..7357e1b169d2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/foo/A.java @@ -0,0 +1,22 @@ +package foo; + +import bar.*; + +import static foo.B.*; +import static foo.B.C; + +public class A +{ + public static void main() + { + new D(); + new D1(); + new D2(); + new D3(); + new D4(); + new C(); + new C1(); + new C2(); + new C3(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/foo/B.java b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/foo/B.java new file mode 100644 index 000000000000..67f416b826ff --- /dev/null +++ b/java/java-tests/testData/codeInsight/importHelper/conflictBetweenRegularAndStaticClassesInImportList/foo/B.java @@ -0,0 +1,19 @@ +package foo; + +public class B +{ + public static class C + { + public C() { + } + } + public static class C1 + { + } + public static class C2 + { + } + public static class C3 + { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/Usage.java b/java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/Usage.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/Usage.java rename to java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/Usage.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/Usage_afterOptimize.txt b/java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/Usage_afterOptimize.txt similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/Usage_afterOptimize.txt rename to java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/Usage_afterOptimize.txt diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/n/Class.java b/java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/n/Class.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/n/Class.java rename to java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/n/Class.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/n/X1.java b/java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/n/X1.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/n/X1.java rename to java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/n/X1.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/n/X2.java b/java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/n/X2.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/n/X2.java rename to java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/n/X2.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/n/X3.java b/java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/n/X3.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses/x/n/X3.java rename to java/java-tests/testData/codeInsight/importHelper/reimportConflictingClasses/x/n/X3.java diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ImportHelperTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ImportHelperTest.java index 15e5deb90b1d..653d92c33574 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ImportHelperTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/ImportHelperTest.java @@ -136,12 +136,7 @@ public class ImportHelperTest extends DaemonAnalyzerTestCase { settings.IMPORT_LAYOUT_TABLE.copyFrom(table); CodeStyleSettingsManager.getInstance(getProject()).setTemporarySettings(settings); - try { - JavaCodeStyleManager.getInstance(getProject()).optimizeImports(file); - } - finally { - CodeStyleSettingsManager.getInstance(getProject()).dropTemporarySettings(); - } + JavaCodeStyleManager.getInstance(getProject()).optimizeImports(file); assertOrder(file, "java.awt.*", CommonClassNames.JAVA_UTIL_MAP, "static java.lang.Math.max", "static java.lang.Math.min", "static javax.swing.SwingConstants.CENTER"); @@ -174,29 +169,20 @@ public class ImportHelperTest extends DaemonAnalyzerTestCase { } } - @NonNls private static final String BASE_PATH = "/codeInsight/daemonCodeAnalyzer/advHighlighting/reimportConflictingClasses"; + @NonNls private static final String BASE_PATH = "/codeInsight/importHelper/"; @WrapInCommand public void testReimportConflictingClasses() throws Exception { - configureByFile(BASE_PATH+"/x/Usage.java", BASE_PATH); + String path = BASE_PATH + getTestName(true); + configureByFile(path + "/x/Usage.java", path); assertEmpty(highlightErrors()); CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject()).clone(); settings.CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND = 2; CodeStyleSettingsManager.getInstance(getProject()).setTemporarySettings(settings); - try { - new WriteCommandAction.Simple(getProject()) { - @Override - protected void run() throws Throwable { - JavaCodeStyleManager.getInstance(getProject()).optimizeImports(getFile()); - } - }.execute().throwException(); - } - finally { - CodeStyleSettingsManager.getInstance(getProject()).dropTemporarySettings(); - } + WriteCommandAction.runWriteCommandAction(getProject(), + () -> JavaCodeStyleManager.getInstance(getProject()).optimizeImports(getFile())); - - @NonNls String fullPath = getTestDataPath() + BASE_PATH + "/x/Usage_afterOptimize.txt"; + @NonNls String fullPath = getTestDataPath() + path + "/x/Usage_afterOptimize.txt"; final VirtualFile vFile = LocalFileSystem.getInstance().findFileByPath(fullPath.replace(File.separatorChar, '/')); String text = LoadTextUtil.loadText(vFile).toString(); assertEquals(text, getFile().getText()); @@ -484,4 +470,19 @@ public class ImportHelperTest extends DaemonAnalyzerTestCase { assertFalse(fix.isAvailable(getProject(), getEditor(), getFile())); } + public void testConflictBetweenRegularAndStaticClassesInImportList() throws Exception { + String path = BASE_PATH + getTestName(true); + configureByFile(path + "/foo/A.java", path); + assertEmpty(highlightErrors()); + + CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject()).clone(); + settings.LAYOUT_STATIC_IMPORTS_SEPARATELY = true; + settings.CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND = 3; + settings.NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND = 3; + + CodeStyleSettingsManager.getInstance(getProject()).setTemporarySettings(settings); + WriteCommandAction.runWriteCommandAction(getProject(), () -> JavaCodeStyleManager.getInstance(getProject()).optimizeImports(getFile())); + + assertEmpty(highlightErrors()); + } } diff --git a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/JavaFxImportsOptimizer.java b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/JavaFxImportsOptimizer.java index f936792038c0..cc7976209d63 100644 --- a/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/JavaFxImportsOptimizer.java +++ b/plugins/javaFX/src/org/jetbrains/plugins/javaFX/fxml/codeInsight/JavaFxImportsOptimizer.java @@ -31,6 +31,7 @@ import com.intellij.psi.codeStyle.CodeStyleSettingsManager; import com.intellij.psi.impl.source.codeStyle.ImportHelper; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.xml.*; +import com.intellij.util.containers.HashMap; import com.intellij.util.containers.HashSet; import com.intellij.xml.XmlAttributeDescriptor; import com.intellij.xml.XmlElementDescriptor; @@ -70,9 +71,11 @@ public class JavaFxImportsOptimizer implements ImportOptimizer { Collections.sort(names, (o1, o2) -> StringUtil.compare(o1.first, o2.first, true)); final CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(project); final List> sortedNames = ImportHelper.sortItemsAccordingToSettings(names, settings); - final HashSet onDemand = new HashSet<>(); - ImportHelper.collectOnDemandImports(sortedNames, onDemand, settings); - onDemand.addAll(demandedForNested); + final Map onDemand = new HashMap<>(); + ImportHelper.collectOnDemandImports(sortedNames, settings, onDemand); + for (String s : demandedForNested) { + onDemand.put(s, false); + } final Set imported = new HashSet<>(); final List imports = new ArrayList<>(); for (Pair pair : sortedNames) { @@ -81,7 +84,7 @@ public class JavaFxImportsOptimizer implements ImportOptimizer { if (imported.contains(packageName) || imported.contains(qName)) { continue; } - if (onDemand.contains(packageName)) { + if (onDemand.containsKey(packageName)) { imported.add(packageName); imports.add(""); } else {