From ba4cd7405604cf61d4b6346f3801a9d9f682ccbd Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 26 Oct 2015 12:39:50 +0100 Subject: [PATCH] for java completion, choose method with most specific return type among same-signature candidates (IDEA-136174) --- .../scope/JavaCompletionProcessor.java | 80 ++++++++++--------- .../completion/scope/CompletionElement.java | 44 ++++++++-- .../normal/ShowMostSpecificOverride.java | 18 +++++ .../completion/NormalCompletionTest.groovy | 5 ++ 4 files changed, 105 insertions(+), 42 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/completion/normal/ShowMostSpecificOverride.java diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/scope/JavaCompletionProcessor.java b/java/java-impl/src/com/intellij/codeInsight/completion/scope/JavaCompletionProcessor.java index b9f6532142f8..040bc8bba976 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/scope/JavaCompletionProcessor.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/scope/JavaCompletionProcessor.java @@ -30,13 +30,19 @@ import com.intellij.psi.infos.CandidateInfo; import com.intellij.psi.scope.BaseScopeProcessor; import com.intellij.psi.scope.ElementClassHint; import com.intellij.psi.scope.JavaScopeProcessorEvent; -import com.intellij.psi.util.*; -import gnu.trove.THashSet; -import gnu.trove.TObjectHashingStrategy; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiTypesUtil; +import com.intellij.psi.util.PsiUtil; +import com.intellij.psi.util.PsiUtilCore; +import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.hash.LinkedHashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.*; +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; /** * Created by IntelliJ IDEA. @@ -50,25 +56,9 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme private final boolean myInJavaDoc; private boolean myStatic = false; private PsiElement myDeclarationHolder = null; - private final Set myResultNames = new THashSet(new TObjectHashingStrategy() { - @Override - public int computeHashCode(Object object) { - if (object instanceof MethodSignature) { - return MethodSignatureUtil.METHOD_PARAMETERS_ERASURE_EQUALITY.computeHashCode((MethodSignature)object); - } - return object != null ? object.hashCode() : 0; - } - - @Override - public boolean equals(Object o1, Object o2) { - if (o1 instanceof MethodSignature && o2 instanceof MethodSignature) { - return MethodSignatureUtil.METHOD_PARAMETERS_ERASURE_EQUALITY.equals((MethodSignature)o1, (MethodSignature)o2); - } - return o1 != null ? o1.equals(o2) : o2 == null; - } - }); - private final List myResults = new ArrayList(); - private final List myFilteredResults = new ArrayList(); + private final Map myResults = new LinkedHashMap(); + private final Set mySecondRateResults = ContainerUtil.newIdentityTroveSet(); + private final Set myShadowedNames = ContainerUtil.newHashSet(); private final PsiElement myElement; private final PsiElement myScope; private final ElementFilter myFilter; @@ -241,16 +231,26 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme } } - if (satisfies(element, state) && isAccessible(element)) { - CompletionElement element1 = new CompletionElement(element, state.get(PsiSubstitutor.KEY)); - if (myResultNames.add(element1.getUniqueId())) { - StaticProblem sp = myElement.getParent() instanceof PsiMethodReferenceExpression ? StaticProblem.none : getStaticProblem(element); - if (sp != StaticProblem.instanceAfterStatic) { - (sp == StaticProblem.staticAfterInstance ? myFilteredResults : myResults).add(element1); - } + if (element instanceof PsiVariable) { + String name = ((PsiVariable)element).getName(); + if (myShadowedNames.contains(name)) return true; + if (element instanceof PsiLocalVariable || element instanceof PsiParameter) { + myShadowedNames.add(name); + } + } + + if (!satisfies(element, state) || !isAccessible(element)) return true; + + StaticProblem sp = myElement.getParent() instanceof PsiMethodReferenceExpression ? StaticProblem.none : getStaticProblem(element); + if (sp == StaticProblem.instanceAfterStatic) return true; + + CompletionElement completion = new CompletionElement(element, state.get(PsiSubstitutor.KEY)); + CompletionElement prev = myResults.get(completion); + if (prev == null || completion.isMoreSpecificThan(prev)) { + myResults.put(completion, completion); + if (sp == StaticProblem.staticAfterInstance) { + mySecondRateResults.add(completion); } - } else if (element instanceof PsiLocalVariable || element instanceof PsiParameter) { - myResultNames.add(CompletionElement.getVariableUniqueId((PsiVariable)element)); } return true; @@ -322,20 +322,26 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme public void setCompletionElements(@NotNull Object[] elements) { for (Object element: elements) { - myResults.add(new CompletionElement(element, PsiSubstitutor.EMPTY)); + CompletionElement completion = new CompletionElement(element, PsiSubstitutor.EMPTY); + myResults.put(completion, completion); } } public Iterable getResults() { - if (myResults.isEmpty()) { - return myFilteredResults; + if (mySecondRateResults.size() == myResults.size()) { + return mySecondRateResults; } - return myResults; + return ContainerUtil.filter(myResults.values(), new Condition() { + @Override + public boolean value(CompletionElement element) { + return !mySecondRateResults.contains(element); + } + }); } public void clear() { myResults.clear(); - myFilteredResults.clear(); + mySecondRateResults.clear(); } @Override diff --git a/java/java-psi-impl/src/com/intellij/codeInsight/completion/scope/CompletionElement.java b/java/java-psi-impl/src/com/intellij/codeInsight/completion/scope/CompletionElement.java index 43ab3c12fe69..df758ceb8523 100644 --- a/java/java-psi-impl/src/com/intellij/codeInsight/completion/scope/CompletionElement.java +++ b/java/java-psi-impl/src/com/intellij/codeInsight/completion/scope/CompletionElement.java @@ -15,7 +15,11 @@ */ package com.intellij.codeInsight.completion.scope; +import com.intellij.openapi.util.Comparing; import com.intellij.psi.*; +import com.intellij.psi.util.MethodSignature; +import com.intellij.psi.util.MethodSignatureUtil; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** @@ -28,10 +32,12 @@ import org.jetbrains.annotations.Nullable; public class CompletionElement{ private final Object myElement; private final PsiSubstitutor mySubstitutor; + private final Object myEqualityObject; public CompletionElement(Object element, PsiSubstitutor substitutor) { myElement = element; mySubstitutor = substitutor; + myEqualityObject = getUniqueId(); } public PsiSubstitutor getSubstitutor(){ @@ -43,9 +49,10 @@ public class CompletionElement{ } @Nullable - Object getUniqueId(){ + private Object getUniqueId(){ if(myElement instanceof PsiClass){ - return ((PsiClass)myElement).getQualifiedName(); + String qName = ((PsiClass)myElement).getQualifiedName(); + return qName == null ? ((PsiClass)myElement).getName() : qName; } if(myElement instanceof PsiPackage){ return ((PsiPackage)myElement).getQualifiedName(); @@ -54,13 +61,40 @@ public class CompletionElement{ return ((PsiMethod)myElement).getSignature(mySubstitutor); } if (myElement instanceof PsiVariable) { - return getVariableUniqueId((PsiVariable)myElement); + return "#" + ((PsiVariable)myElement).getName(); } return null; } - public static String getVariableUniqueId(final PsiVariable variable) { - return "#" + variable.getName(); + @Override + public boolean equals(Object obj) { + if (obj == this) return true; + if (!(obj instanceof CompletionElement)) return false; + + Object thatObj = ((CompletionElement)obj).myEqualityObject; + if (myEqualityObject instanceof MethodSignature) { + return thatObj instanceof MethodSignature && + MethodSignatureUtil.METHOD_PARAMETERS_ERASURE_EQUALITY.equals((MethodSignature)myEqualityObject, (MethodSignature)thatObj); + } + return Comparing.equal(myEqualityObject, thatObj); } + + @Override + public int hashCode() { + if (myEqualityObject instanceof MethodSignature) { + return MethodSignatureUtil.METHOD_PARAMETERS_ERASURE_EQUALITY.computeHashCode((MethodSignature)myEqualityObject); + } + return myEqualityObject != null ? myEqualityObject.hashCode() : 0; + } + + public boolean isMoreSpecificThan(@NotNull CompletionElement prev) { + Object prevElement = prev.getElement(); + if (!(prevElement instanceof PsiMethod && myElement instanceof PsiMethod)) return false; + + PsiType prevType = prev.getSubstitutor().substitute(((PsiMethod)prevElement).getReturnType()); + PsiType candidateType = mySubstitutor.substitute(((PsiMethod)myElement).getReturnType()); + return prevType != null && candidateType != null && !prevType.equals(candidateType) && prevType.isAssignableFrom(candidateType); + } + } diff --git a/java/java-tests/testData/codeInsight/completion/normal/ShowMostSpecificOverride.java b/java/java-tests/testData/codeInsight/completion/normal/ShowMostSpecificOverride.java new file mode 100644 index 000000000000..4ea80e1e96c2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/normal/ShowMostSpecificOverride.java @@ -0,0 +1,18 @@ +interface Root1 { + A get(); +} +interface Root2 extends Root1 { + B get(); +} + +interface Child extends Root1, Root2 { +} + +public class Test { + public void test(Child child) { + child.getx + } +} + +interface A {} +interface B extends A {} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/completion/NormalCompletionTest.groovy b/java/java-tests/testSrc/com/intellij/codeInsight/completion/NormalCompletionTest.groovy index 138d8607066c..0a36aee54bc6 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/completion/NormalCompletionTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInsight/completion/NormalCompletionTest.groovy @@ -1542,4 +1542,9 @@ class Bar { assert c1.is(c2) } + + public void testShowMostSpecificOverride() { + configure() + assert 'B' == LookupElementPresentation.renderElement(myFixture.lookup.items[0]).typeText + } }