From 52619fbe7f9a390e876fcbdd526aa44567be6d64 Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 26 Sep 2016 13:38:34 +0200 Subject: [PATCH] simplify too smart class proximity heuristic by package name parts (IDEA-161684) --- .../proximity/ExplicitlyImportedWeigher.java | 51 ++++++------------- .../intention/AddImportActionTest.groovy | 28 ++-------- 2 files changed, 19 insertions(+), 60 deletions(-) diff --git a/java/java-impl/src/com/intellij/psi/util/proximity/ExplicitlyImportedWeigher.java b/java/java-impl/src/com/intellij/psi/util/proximity/ExplicitlyImportedWeigher.java index 21670b54fc53..39dffa89f44f 100644 --- a/java/java-impl/src/com/intellij/psi/util/proximity/ExplicitlyImportedWeigher.java +++ b/java/java-impl/src/com/intellij/psi/util/proximity/ExplicitlyImportedWeigher.java @@ -17,7 +17,6 @@ package com.intellij.psi.util.proximity; import com.intellij.openapi.module.Module; import com.intellij.openapi.module.ModuleUtilCore; -import com.intellij.openapi.util.Condition; import com.intellij.openapi.util.NotNullLazyKey; import com.intellij.openapi.util.NullableLazyKey; import com.intellij.openapi.util.text.StringUtil; @@ -26,8 +25,6 @@ import com.intellij.psi.util.ProximityLocation; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.PsiUtilCore; -import com.intellij.util.NotNullFunction; -import com.intellij.util.NullableFunction; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -39,33 +36,23 @@ import java.util.List; * @author peter */ public class ExplicitlyImportedWeigher extends ProximityWeigher { - private static final NullableLazyKey - PLACE_PACKAGE = NullableLazyKey.create("placePackage", new NullableFunction() { - @Override - public PsiPackage fun(ProximityLocation location) { - PsiElement position = location.getPosition(); - if (position == null) return null; - - return getContextPackage(position); - } + private static final NullableLazyKey PLACE_PACKAGE = NullableLazyKey.create("placePackage", location -> { + PsiElement position = location.getPosition(); + return position == null ? null : getContextPackage(position); }); private static final NotNullLazyKey, ProximityLocation> PLACE_IMPORTED_NAMES = - NotNullLazyKey.create("importedNames", new NotNullFunction>() { - @NotNull - @Override - public List fun(ProximityLocation location) { - final PsiJavaFile psiJavaFile = PsiTreeUtil.getContextOfType(location.getPosition(), PsiJavaFile.class, false); - final PsiImportList importList = psiJavaFile == null ? null : psiJavaFile.getImportList(); - if (importList == null) return Collections.emptyList(); + NotNullLazyKey.create("importedNames", location -> { + final PsiJavaFile psiJavaFile = PsiTreeUtil.getContextOfType(location.getPosition(), PsiJavaFile.class, false); + final PsiImportList importList = psiJavaFile == null ? null : psiJavaFile.getImportList(); + if (importList == null) return Collections.emptyList(); - List importedNames = ContainerUtil.newArrayList(); - for (PsiImportStatementBase statement : importList.getAllImportStatements()) { - PsiJavaCodeReferenceElement reference = statement.getImportReference(); - ContainerUtil.addIfNotNull(importedNames, reference == null ? null : reference.getQualifiedName()); - } - - return importedNames; + List importedNames = ContainerUtil.newArrayList(); + for (PsiImportStatementBase statement : importList.getAllImportStatements()) { + PsiJavaCodeReferenceElement reference = statement.getImportReference(); + ContainerUtil.addIfNotNull(importedNames, reference == null ? null : reference.getQualifiedName()); } + + return importedNames; }); @Nullable @@ -114,16 +101,10 @@ public class ExplicitlyImportedWeigher extends ProximityWeigher { return 100; } - String pkg = StringUtil.getPackageName(qname); - - // check if anything from the parent packages is already imported in the file: + // check if anything from the same package is already imported in the file: // people are likely to refer to the same subsystem as they're already working - while (!pkg.isEmpty()) { - if (containsImport(importedNames, pkg)) { - // more specific already imported packages get more weight - return StringUtil.countChars(pkg, '.') + 1; - } - pkg = StringUtil.getPackageName(pkg); + if (containsImport(importedNames, StringUtil.getPackageName(qname))) { + return 50; } } diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/intention/AddImportActionTest.groovy b/java/java-tests/testSrc/com/intellij/codeInsight/intention/AddImportActionTest.groovy index f7c28071890d..83139afcdcb9 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/intention/AddImportActionTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInsight/intention/AddImportActionTest.groovy @@ -524,7 +524,7 @@ public class Foo { } - void "prefer from imported package"() { + void "test prefer from imported package"() { myFixture.addClass 'package foo; public class Log {}' myFixture.addClass 'package foo; public class Imported {}' myFixture.addClass 'package bar; public class Log {}' @@ -535,8 +535,9 @@ public class Foo { } ''' importClass() - myFixture.checkResult '''import foo.Log; + myFixture.checkResult '''\ import foo.Imported; +import foo.Log; public class Foo { Log l; @@ -545,29 +546,6 @@ public class Foo { ''' } - void "test prefer from imported package sibling"() { - myFixture.addClass 'package com.foo.doo; public class Log {}' - myFixture.addClass 'package com.foo.imported; public class Imported {}' - myFixture.addClass 'package com.bar; public class Log {}' - myFixture.configureByText 'a.java', '''import com.foo.imported.Imported; - -public class Foo { - Log l; - Imported i; -} -''' - importClass() - myFixture.checkResult '''import com.foo.doo.Log; -import com.foo.imported.Imported; - -public class Foo { - Log l; - Imported i; -} -''' - - } - void "test remember chosen variants"() { ((StatisticsManagerImpl)StatisticsManager.getInstance()).enableStatistics(getTestRootDisposable()) myFixture.addClass 'package foo; public class Log {}'