From 9404e50337f94c370097968f13b23cb77d16ab83 Mon Sep 17 00:00:00 2001 From: peter Date: Thu, 25 Oct 2012 19:44:46 +0200 Subject: [PATCH] prefix matching is more important than element kind and statistics --- .../completion/JavaCompletionSorting.java | 6 +- .../completion/RecursionWeigher.java | 59 +++++++++++-------- .../element/ExcludeSillyAssignment.java | 27 ++++++--- .../com/intellij/psi/util/PropertyUtil.java | 3 + .../CommonPrefixMoreImportantThanKind.java | 10 ++++ .../PreferParametersToGetters.java | 14 +---- .../NormalCompletionOrderingTest.groovy | 13 ++-- .../SmartTypeCompletionOrderingTest.groovy | 8 +-- .../src/META-INF/LangExtensions.xml | 8 +-- 9 files changed, 88 insertions(+), 60 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/completion/normalSorting/CommonPrefixMoreImportantThanKind.java diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionSorting.java b/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionSorting.java index ce1d16fd67b4..05103cede58b 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionSorting.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionSorting.java @@ -64,9 +64,6 @@ public class JavaCompletionSorting { else { afterPriority.add(new PreferDefaultTypeWeigher(expectedTypes, parameters)); } - if (!JavaCompletionData.START_FOR.accepts(position)) { - afterPriority.add(new PreferByKindWeigher(type, position)); - } ContainerUtil.addIfNotNull(afterPriority, recursion(parameters, expectedTypes)); if (!smart && !afterNew) { afterPriority.add(new PreferExpected(false, expectedTypes)); @@ -117,7 +114,8 @@ public class JavaCompletionSorting { }); } sorter = sorter.weighAfter("priority", afterPriority.toArray(new LookupElementWeigher[afterPriority.size()])); - sorter = sorter.weighAfter("prefix", new PreferNonGeneric(), new PreferAccessible(position), new PreferSimple(), new PreferEnumConstants(parameters)); + sorter = sorter.weighAfter("prefix", new PreferByKindWeigher(type, position)); + sorter = sorter.weighAfter("stats", new PreferNonGeneric(), new PreferAccessible(position), new PreferSimple(), new PreferEnumConstants(parameters)); sorter = sorter.weighAfter("proximity", afterProximity.toArray(new LookupElementWeigher[afterProximity.size()])); return result.withRelevanceSorter(sorter); } diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/RecursionWeigher.java b/java/java-impl/src/com/intellij/codeInsight/completion/RecursionWeigher.java index e3429c46c0c0..59df2bb447e6 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/RecursionWeigher.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/RecursionWeigher.java @@ -23,8 +23,6 @@ import com.intellij.openapi.util.Comparing; import com.intellij.patterns.PsiJavaPatterns; import com.intellij.patterns.StandardPatterns; import com.intellij.psi.*; -import com.intellij.psi.codeStyle.JavaCodeStyleManager; -import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.filters.AndFilter; import com.intellij.psi.filters.ClassFilter; import com.intellij.psi.filters.ElementFilter; @@ -53,7 +51,7 @@ class RecursionWeigher extends LookupElementWeigher { public RecursionWeigher(PsiElement position, @NotNull PsiReferenceExpression reference, - PsiMethodCallExpression expression, + @Nullable PsiMethodCallExpression expression, ExpectedTypeInfo[] expectedInfos) { super("recursion"); myFilter = recursionFilter(position); @@ -70,7 +68,7 @@ class RecursionWeigher extends LookupElementWeigher { } @Nullable - private static PsiExpression normalizeQualifier(PsiElement qualifier) { + private static PsiExpression normalizeQualifier(@Nullable PsiElement qualifier) { return qualifier instanceof PsiThisExpression || !(qualifier instanceof PsiExpression) ? null : (PsiExpression)qualifier; } @@ -127,21 +125,23 @@ class RecursionWeigher extends LookupElementWeigher { return Result.passingObjectToItself; } - if (myExpression != null) { - if (myExpectedInfos != null) { - final PsiType itemType = JavaCompletionUtil.getLookupElementType(element); - for (final ExpectedTypeInfo expectedInfo : myExpectedInfos) { - PsiMethod calledMethod = expectedInfo.getCalledMethod(); - if (calledMethod != null && itemType != null) { - if (calledMethod.equals(myPositionMethod) && expectedInfo.getType().isAssignableFrom(itemType)) { - return myDelegate ? Result.delegation : Result.recursive; - } - if (isGetterSetterAssignment(object, calledMethod)) { - return myDelegate ? Result.delegation : Result.recursive; - } - } + if (myExpectedInfos != null) { + final PsiType itemType = JavaCompletionUtil.getLookupElementType(element); + for (final ExpectedTypeInfo expectedInfo : myExpectedInfos) { + PsiMethod calledMethod = expectedInfo.getCalledMethod(); + if (itemType != null && + calledMethod != null && + calledMethod.equals(myPositionMethod) && + expectedInfo.getType().isAssignableFrom(itemType)) { + return myDelegate ? Result.delegation : Result.recursive; + } + String propertyName = getSetterPropertyName(calledMethod); + if (propertyName != null && isGetterSetterAssignment(object, propertyName)) { + return myDelegate ? Result.delegation : Result.recursive; } } + } + if (myExpression != null) { return Result.normal; } @@ -159,16 +159,25 @@ class RecursionWeigher extends LookupElementWeigher { return Result.normal; } - private static boolean isGetterSetterAssignment(Object lookupObject, PsiMethod calledMethod) { - if (!PropertyUtil.isSimplePropertySetter(calledMethod)) { - return false; + @Nullable + private String getSetterPropertyName(@Nullable PsiMethod calledMethod) { + if (PropertyUtil.isSimplePropertySetter(calledMethod)) { + assert calledMethod != null; + return PropertyUtil.getPropertyName(calledMethod); } + PsiReferenceExpression reference = ExcludeSillyAssignment.getAssignedReference(myPosition); + if (reference != null) { + PsiElement target = reference.resolve(); + if (target instanceof PsiField) { + return PropertyUtil.suggestPropertyName((PsiField)target); + } + } + return null; + } - String prop = PropertyUtil.getPropertyName(calledMethod); - assert prop != null; + private static boolean isGetterSetterAssignment(Object lookupObject, String prop) { if (lookupObject instanceof PsiField && - prop.equals(JavaCodeStyleManager.getInstance(calledMethod.getProject()) - .variableNameToPropertyName(((PsiField)lookupObject).getName(), VariableKind.FIELD))) { + prop.equals(PropertyUtil.suggestPropertyName((PsiField)lookupObject))) { return true; } if (lookupObject instanceof PsiMethod && @@ -181,7 +190,7 @@ class RecursionWeigher extends LookupElementWeigher { private boolean isPassingObjectToItself(Object object) { if (object instanceof PsiThisExpression) { - return !myDelegate || myCallQualifier instanceof PsiSuperExpression; + return myCallQualifier != null && !myDelegate || myCallQualifier instanceof PsiSuperExpression; } return myCallQualifier instanceof PsiReferenceExpression && object.equals(((PsiReferenceExpression)myCallQualifier).advancedResolve(true).getElement()); diff --git a/java/java-impl/src/com/intellij/psi/filters/element/ExcludeSillyAssignment.java b/java/java-impl/src/com/intellij/psi/filters/element/ExcludeSillyAssignment.java index d807e741d446..d3e7c0c77dc7 100644 --- a/java/java-impl/src/com/intellij/psi/filters/element/ExcludeSillyAssignment.java +++ b/java/java-impl/src/com/intellij/psi/filters/element/ExcludeSillyAssignment.java @@ -17,6 +17,7 @@ package com.intellij.psi.filters.element; import com.intellij.psi.*; import com.intellij.psi.filters.ElementFilter; +import org.jetbrains.annotations.Nullable; /** * Created by IntelliJ IDEA. @@ -26,14 +27,13 @@ import com.intellij.psi.filters.ElementFilter; * To change this template use Options | File Templates. */ public class ExcludeSillyAssignment implements ElementFilter { - @Override - public boolean isAcceptable(Object element, PsiElement context) { - if(!(element instanceof PsiElement)) return true; - PsiElement each = context; + @Nullable + public static PsiReferenceExpression getAssignedReference(PsiElement position) { + PsiElement each = position; while (each != null && !(each instanceof PsiFile)) { if (each instanceof PsiExpressionList || each instanceof PsiPrefixExpression || each instanceof PsiPolyadicExpression) { - return true; + return null; } if (each instanceof PsiAssignmentExpression) { @@ -43,18 +43,29 @@ public class ExcludeSillyAssignment implements ElementFilter { final PsiElement qualifier = referenceExpression.getQualifier(); if (qualifier != null) { if (!(qualifier instanceof PsiThisExpression) || ((PsiThisExpression)qualifier).getQualifier() != null) { - return true; + return null; } } - return !referenceExpression.isReferenceTo((PsiElement)element); + return referenceExpression; } - return true; + return null; } each = each.getContext(); } + + return null; + } + + @Override + public boolean isAcceptable(Object element, PsiElement context) { + if(!(element instanceof PsiElement)) return true; + PsiReferenceExpression referenceExpression = getAssignedReference(context); + if (referenceExpression != null && referenceExpression.isReferenceTo((PsiElement)element)) { + return false; + } return true; } diff --git a/java/java-psi-api/src/com/intellij/psi/util/PropertyUtil.java b/java/java-psi-api/src/com/intellij/psi/util/PropertyUtil.java index c95ddfe13de9..5114fb7f1145 100644 --- a/java/java-psi-api/src/com/intellij/psi/util/PropertyUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/util/PropertyUtil.java @@ -506,6 +506,9 @@ public class PropertyUtil { modifierList.addAfter(factory.createAnnotationFromText("@" + annotationQName, listOwner), null); } + public static String suggestPropertyName(PsiField field) { + return suggestPropertyName(field.getProject(), field); + } public static String suggestPropertyName(Project project, PsiField field) { JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(project); VariableKind kind = codeStyleManager.getVariableKind(field); diff --git a/java/java-tests/testData/codeInsight/completion/normalSorting/CommonPrefixMoreImportantThanKind.java b/java/java-tests/testData/codeInsight/completion/normalSorting/CommonPrefixMoreImportantThanKind.java new file mode 100644 index 000000000000..9d0f81311a1b --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/normalSorting/CommonPrefixMoreImportantThanKind.java @@ -0,0 +1,10 @@ +class PsiElement {} + +public class Foo { + + Object psiElement() {} + + void foo() { + Psi + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/smartTypeSorting/PreferParametersToGetters.java b/java/java-tests/testData/codeInsight/completion/smartTypeSorting/PreferParametersToGetters.java index b4a740993aeb..fa16535e8b78 100644 --- a/java/java-tests/testData/codeInsight/completion/smartTypeSorting/PreferParametersToGetters.java +++ b/java/java-tests/testData/codeInsight/completion/smartTypeSorting/PreferParametersToGetters.java @@ -1,22 +1,14 @@ class T { - I myLastI; - I getLastI() { return myLastI; } + I lastI; + I getLastI() { return lastI; } void setLastI(I a) { - myLastI = + lastI = } } class I { static final I _1 = new I(); - static final I _2a = new I(); - static final I _3a = new I(); - static final I _4a = new I(); - static final I _5a = new I(); - static final I _6a = new I(); - static final I _7a = new I(); - static final I _8a = new I(); - static final I _9a = new I(); static I valueOf(String sth) { return null; diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/completion/NormalCompletionOrderingTest.groovy b/java/java-tests/testSrc/com/intellij/codeInsight/completion/NormalCompletionOrderingTest.groovy index 9b292a5f2c2e..f37deceab9e0 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/completion/NormalCompletionOrderingTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInsight/completion/NormalCompletionOrderingTest.groovy @@ -200,7 +200,7 @@ public class NormalCompletionOrderingTest extends CompletionSortingTestCase { } public void testLocalVarsOverMethods() { - checkPreferredItems(0, "value", "validate", "validateTree", "valueOf"); + checkPreferredItems(1, "value", "valueOf"); } public void testCurrentClassBest() { @@ -281,12 +281,12 @@ public class NormalCompletionOrderingTest extends CompletionSortingTestCase { public void testPreselectMostRelevantInTheMiddleAlpha() { UISettings.getInstance().SORT_LOOKUP_ELEMENTS_LEXICOGRAPHICALLY = true; - myFixture.addClass("package foo; public class Elxaaaaaaaaaaaaaaaaaaaa {}"); + myFixture.addClass("package foo; public class ELXaaaaaaaaaaaaaaaaaaaa {}"); invokeCompletion(getTestName(false) + ".java"); myFixture.completeBasic(); LookupImpl lookup = getLookup(); assertPreferredItems(lookup.getList().getSelectedIndex()); - assertEquals("Elxaaaaaaaaaaaaaaaaaaaa", lookup.getItems().get(0).getLookupString()); + assertEquals("ELXaaaaaaaaaaaaaaaaaaaa", lookup.getItems().get(0).getLookupString()); assertEquals("ELXEMENT_A", lookup.getCurrentItem().getLookupString()); } @@ -395,7 +395,7 @@ import java.lang.annotation.Target; myFixture.addClass('public class fooAClass {}') configureNoCompletion(getTestName(false) + ".java"); myFixture.complete(CompletionType.BASIC, 2); - assertPreferredItems(0, 'fooy', 'foox', 'fooAClass', 'fooBar'); + assertPreferredItems(0, 'fooy', 'fooAClass', 'fooBar', 'foox'); } public void testChangePreselectionOnSecondInvocation() { @@ -499,4 +499,9 @@ import java.lang.annotation.Target; assertPreferredItems 0, 'contains', 'containsAll' } + public void testCommonPrefixMoreImportantThanKind() { + CodeInsightSettings.getInstance().COMPLETION_CASE_SENSITIVE = CodeInsightSettings.NONE; + checkPreferredItems(0, 'PsiElement', 'psiElement') + } + } diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/completion/SmartTypeCompletionOrderingTest.groovy b/java/java-tests/testSrc/com/intellij/codeInsight/completion/SmartTypeCompletionOrderingTest.groovy index 62929e651484..dff7dcb458c9 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/completion/SmartTypeCompletionOrderingTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInsight/completion/SmartTypeCompletionOrderingTest.groovy @@ -21,7 +21,7 @@ public class SmartTypeCompletionOrderingTest extends CompletionSortingTestCase { } public void testJComponentAdd() throws Throwable { - checkPreferredItems(0, "name", "b", "foo", "fooBean239", "this", "getName"); + checkPreferredItems(0, "name", "getName", "b", "foo", "fooBean239", "this"); } public void testJComponentAddNew() throws Throwable { @@ -161,7 +161,7 @@ public class SmartTypeCompletionOrderingTest extends CompletionSortingTestCase { } public void testPreferParametersToGetters() throws Throwable { - checkPreferredItems(0, "a", "getLastI"); + checkPreferredItems(0, "a", "I._1", "valueOf", "getLastI"); } public void testExpectedInterfaceShouldGoFirst() throws Throwable { @@ -178,7 +178,7 @@ public class SmartTypeCompletionOrderingTest extends CompletionSortingTestCase { } public void testPreferNonRecursiveMethodParams() throws Throwable { - checkPreferredItems(0, "b", "s", "a", "hashCode"); + checkPreferredItems(0, "b", "hashCode", "s", "a"); } public void testPreferDelegatingMethodParams() throws Throwable { @@ -222,7 +222,7 @@ public class SmartTypeCompletionOrderingTest extends CompletionSortingTestCase { } public void testFactoryMethodForDefaultType() throws Throwable { - checkPreferredItems(0, "create", "this", "map", "getClass"); + checkPreferredItems(0, "create", "this", "getClass"); } public void testLocalVarsBeforeClassLiterals() throws Throwable { diff --git a/platform/platform-resources/src/META-INF/LangExtensions.xml b/platform/platform-resources/src/META-INF/LangExtensions.xml index 9c0e12d04f60..47217149eac3 100644 --- a/platform/platform-resources/src/META-INF/LangExtensions.xml +++ b/platform/platform-resources/src/META-INF/LangExtensions.xml @@ -464,12 +464,12 @@ order="after sameModule"/> - - + +