for java completion, choose method with most specific return type among same-signature candidates (IDEA-136174)

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<Object> myResultNames = new THashSet<Object>(new TObjectHashingStrategy<Object>() {
-    @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<CompletionElement> myResults = new ArrayList<CompletionElement>();
-  private final List<CompletionElement> myFilteredResults = new ArrayList<CompletionElement>();
+  private final Map<CompletionElement, CompletionElement> myResults = new LinkedHashMap<CompletionElement, CompletionElement>();
+  private final Set<CompletionElement> mySecondRateResults = ContainerUtil.newIdentityTroveSet();
+  private final Set<String> 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<CompletionElement> getResults() {
-    if (myResults.isEmpty()) {
-      return myFilteredResults;
+    if (mySecondRateResults.size() == myResults.size()) {
+      return mySecondRateResults;
     }
-    return myResults;
+    return ContainerUtil.filter(myResults.values(), new Condition<CompletionElement>() {
+      @Override
+      public boolean value(CompletionElement element) {
+        return !mySecondRateResults.contains(element);
+      }
+    });
   }
 
   public void clear() {
     myResults.clear();
-    myFilteredResults.clear();
+    mySecondRateResults.clear();
   }
 
   @Override

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);
+  }
+
 }

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.get<caret>x
+    }
+}
+
+interface A {}
+interface B extends A {}

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
+  }
 }