Java: Added priorities in the completion list for arguments of getField() and getMethod() (IDEA-167250)

java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangClassMemberReference.java

@@ -18,8 +18,8 @@ package com.intellij.psi.impl.source.resolve.reference.impl;
 import com.intellij.codeInsight.completion.InsertHandler;
 import com.intellij.codeInsight.completion.InsertionContext;
 import com.intellij.codeInsight.completion.JavaLookupElementBuilder;
+import com.intellij.codeInsight.completion.PrioritizedLookupElement;
 import com.intellij.codeInsight.lookup.LookupElement;
-import com.intellij.codeInsight.lookup.LookupElementBuilder;
 import com.intellij.psi.*;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtilCore;
@@ -29,6 +29,7 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.Arrays;
+import java.util.Comparator;
 
 import static com.intellij.psi.impl.source.resolve.reference.impl.JavaReflectionReferenceUtil.*;
 
@@ -71,7 +72,7 @@ public class JavaLangClassMemberReference extends PsiReferenceBase<PsiLiteralExp
 
             case DECLARED_FIELD: {
               PsiField field = psiClass.findFieldByName(name, false);
-              return isReachable(field, psiClass) ? field : null;
+              return isPotentiallyAccessible(field, psiClass) ? field : null;
             }
 
             case METHOD: {
@@ -85,7 +86,7 @@ public class JavaLangClassMemberReference extends PsiReferenceBase<PsiLiteralExp
 
             case DECLARED_METHOD: {
               final PsiMethod[] methods = psiClass.findMethodsByName(name, false);
-              return ContainerUtil.find(methods, method -> isRegularMethod(method) && isReachable(method, psiClass));
+              return ContainerUtil.find(methods, method -> isRegularMethod(method) && isPotentiallyAccessible(method, psiClass));
             }
           }
         }
@@ -115,21 +116,30 @@ public class JavaLangClassMemberReference extends PsiReferenceBase<PsiLiteralExp
         switch (type) {
 
           case DECLARED_FIELD:
-            return psiClass.getFields();
+            return Arrays.stream(psiClass.getFields())
+              .sorted(Comparator.comparing(PsiField::getName))
+              .map(field -> lookupField(field))
+              .toArray();
 
           case FIELD:
-            return ContainerUtil.filter(psiClass.getAllFields(), field -> isReachable(field, psiClass)).toArray();
+            return Arrays.stream(psiClass.getAllFields())
+              .filter(field -> isPotentiallyAccessible(field, psiClass))
+              .sorted(Comparator.comparingInt((PsiField field) -> isPublic(field) ? 0 : 1).thenComparing(PsiField::getName))
+              .map(field -> withPriority(lookupField(field), isPublic(field)))
+              .toArray();
 
           case DECLARED_METHOD:
             return Arrays.stream(psiClass.getMethods())
               .filter(method -> isRegularMethod(method))
-              .map(this::lookupMethod)
+              .sorted(Comparator.comparing(PsiMethod::getName))
+              .map(method -> lookupMethod(method))
               .toArray();
 
           case METHOD:
             return Arrays.stream(psiClass.getAllMethods())
-              .filter(method -> isRegularMethod(method) && isReachable(method, psiClass) && !isJavaLangObject(method.getContainingClass()))
-              .map(this::lookupMethod)
+              .filter(method -> isRegularMethod(method) && isPotentiallyAccessible(method, psiClass))
+              .sorted(Comparator.comparingInt((PsiMethod method) -> getMethodSortOrder(method)).thenComparing(PsiMethod::getName))
+              .map(method -> PrioritizedLookupElement.withPriority(lookupMethod(method), -getMethodSortOrder(method)))
               .toArray();
         }
       }
@@ -137,8 +147,17 @@ public class JavaLangClassMemberReference extends PsiReferenceBase<PsiLiteralExp
     return EMPTY_ARRAY;
   }
 
+  private static int getMethodSortOrder(PsiMethod method) {
+    return isJavaLangObject(method.getContainingClass()) ? 1 : isPublic(method) ? -1 : 0;
+  }
+
   @NotNull
-  private LookupElementBuilder lookupMethod(PsiMethod method) {
+  private static LookupElement lookupField(PsiField field) {
+    return JavaLookupElementBuilder.forField(field);
+  }
+
+  @NotNull
+  private LookupElement lookupMethod(PsiMethod method) {
     return JavaLookupElementBuilder.forMethod(method, PsiSubstitutor.EMPTY).withInsertHandler(this);
   }
 

java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionCompletionContributor.java

@@ -19,7 +19,6 @@ import com.intellij.codeInsight.AnnotationUtil;
 import com.intellij.codeInsight.completion.*;
 import com.intellij.codeInsight.lookup.LookupElement;
 import com.intellij.codeInsight.lookup.LookupElementBuilder;
-import com.intellij.codeInsight.lookup.LookupValueWithPriority;
 import com.intellij.openapi.util.TextRange;
 import com.intellij.patterns.ElementPattern;
 import com.intellij.patterns.PatternCondition;

java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java

@@ -153,7 +153,7 @@ class JavaReflectionReferenceUtil {
    * Non-public members of superclass/superinterface can't be obtained via reflection, they need to be filtered out.
    */
   @Contract("null, _ -> false")
-  static boolean isReachable(PsiMember member, PsiClass psiClass) {
+  static boolean isPotentiallyAccessible(PsiMember member, PsiClass psiClass) {
     return member != null && (member.getContainingClass() == psiClass || isPublic(member));
   }
 
@@ -180,6 +180,6 @@ class JavaReflectionReferenceUtil {
 
   @NotNull
   static LookupElement withPriority(LookupElement lookupElement, boolean hasPriority) {
-    return PrioritizedLookupElement.withPriority(lookupElement, hasPriority ? 0 : -1);
+    return PrioritizedLookupElement.withPriority(lookupElement, hasPriority ? 1 : -1);
   }
 }

java/java-tests/testData/codeInsight/completion/reflection/DeclaredField.java

@@ -22,5 +22,5 @@ class DeclaredField {
 class Test {
   public int num;
   public int num2;
-  int num3;
+  int num1;
 }

java/java-tests/testData/codeInsight/completion/reflection/DeclaredField_after.java

@@ -22,5 +22,5 @@ class DeclaredField {
 class Test {
   public int num;
   public int num2;
-  int num3;
+  int num1;
 }

java/java-tests/testData/codeInsight/completion/reflection/DeclaredMethod.java

@@ -22,7 +22,7 @@ class DecalredMethod {
 class Test {
   public void method(){}
   public void method2(A a, B b){}
-  public void method3(){}
+  void method1(){}
 }
 
 class A {}

java/java-tests/testData/codeInsight/completion/reflection/DeclaredMethod2.java

@@ -22,7 +22,7 @@ class DecalredMethod2 {
 class Test {
   void method(){}
   void method2(A a, B b){}
-  void method3(){}
+  void method1(){}
 }
 
 class A {}

java/java-tests/testData/codeInsight/completion/reflection/DeclaredMethod2_after.java

@@ -15,14 +15,14 @@
  */
 class DecalredMethod2 {
   void foo() {
-    Test.class.getDeclaredMethod("method3");
+    Test.class.getDeclaredMethod("method1");
   }
 }
 
 class Test {
   void method(){}
   void method2(A a, B b){}
-  void method3(){}
+  void method1(){}
 }
 
 class A {}

java/java-tests/testData/codeInsight/completion/reflection/DeclaredMethod_after.java

@@ -22,7 +22,7 @@ class DecalredMethod {
 class Test {
   public void method(){}
   public void method2(A a, B b){}
-  public void method3(){}
+  void method1(){}
 }
 
 class A {}

java/java-tests/testData/codeInsight/completion/reflection/ForNameDeclaredField.java

@@ -22,5 +22,5 @@ class ForNameDeclaredField {
 class Test {
   public int num;
   public int num2;
-  int num3;
+  int num1;
 }

java/java-tests/testData/codeInsight/completion/reflection/ForNameDeclaredField_after.java

@@ -15,12 +15,12 @@
  */
 class ForNameDeclaredField {
   void foo() {
-    Class.forName("Test").getDeclaredField("num3");
+    Class.forName("Test").getDeclaredField("num1");
   }
 }
 
 class Test {
   public int num;
   public int num2;
-  int num3;
+  int num1;
 }

java/java-tests/testData/codeInsight/completion/reflection/ForNameDeclaredMethod.java

@@ -22,7 +22,7 @@ class ForNameDeclaredMethod {
 class Test {
   public void method(){}
   public void method2(A a, B b){}
-  void method3(){}
+  void method1(){}
 }
 
 class A {}

java/java-tests/testData/codeInsight/completion/reflection/ForNameDeclaredMethod_after.java

@@ -22,7 +22,7 @@ class ForNameDeclaredMethod {
 class Test {
   public void method(){}
   public void method2(A a, B b){}
-  void method3(){}
+  void method1(){}
 }
 
 class A {}

java/java-tests/testData/codeInsight/completion/reflection/HasConstructor.java

@@ -0,0 +1,17 @@
+class HasConstructor {
+  void foo() {
+    Test.class.getMethod("<caret>");
+  }
+}
+
+class Test {
+  public Test() {}
+  Test(int n) {}
+  public void method(){}
+  public void method2(A a, B b){}
+  void method1(){}
+}
+
+class A {}
+class B {}
+class C {}

java/java-tests/testData/codeInsight/completion/reflection/HasConstructor_after.java

@@ -0,0 +1,17 @@
+class HasConstructor {
+  void foo() {
+    Test.class.getMethod("method1");
+  }
+}
+
+class Test {
+  public Test() {}
+  Test(int n) {}
+  public void method(){}
+  public void method2(A a, B b){}
+  void method1(){}
+}
+
+class A {}
+class B {}
+class C {}

java/java-tests/testData/codeInsight/completion/reflectionOverload/JavaLangObjectMethods.java

@@ -0,0 +1,9 @@
+class Main {
+  void foo() {
+    Test.class.getMethod("<caret>");
+  }
+}
+
+class Test {
+  public void method(){}
+}

java/java-tests/testData/codeInsight/completion/reflectionOverload/JavaLangObjectMethods_after.java

@@ -0,0 +1,9 @@
+class Main {
+  void foo() {
+    Test.class.getMethod("notifyAll");
+  }
+}
+
+class Test {
+  public void method(){}
+}

java/java-tests/testData/codeInsight/completion/reflectionOverload/JavaLangObjectOwnMethods.java

@@ -0,0 +1,5 @@
+class Main {
+  void foo() {
+    Object.class.getMethod("<caret>");
+  }
+}

java/java-tests/testData/codeInsight/completion/reflectionOverload/JavaLangObjectOwnMethods_after.java

@@ -0,0 +1,5 @@
+class Main {
+  void foo() {
+    Object.class.getMethod("wait", long.class);
+  }
+}

java/java-tests/testData/codeInsight/completion/reflectionOverload/OverloadMethods.java

@@ -0,0 +1,17 @@
+class Main {
+  void foo() {
+    Test.class.getMethod("<caret>");
+  }
+}
+
+class Test {
+  public Test() {}
+  Test(int n) {}
+  public void method(){}
+  void method(A a, B b){}
+  public void method(C c){}
+}
+
+class A {}
+class B {}
+class C {}

java/java-tests/testData/codeInsight/completion/reflectionOverload/OverloadMethods_after.java

@@ -0,0 +1,17 @@
+class Main {
+  void foo() {
+    Test.class.getMethod("method", A.class, B.class);
+  }
+}
+
+class Test {
+  public Test() {}
+  Test(int n) {}
+  public void method(){}
+  void method(A a, B b){}
+  public void method(C c){}
+}
+
+class A {}
+class B {}
+class C {}

java/java-tests/testSrc/com/intellij/codeInsight/completion/JavaReflectionCompletionOverloadTest.kt

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.intellij.codeInsight.completion
+
+import com.intellij.JavaTestUtil
+
+/**
+ * @author Pavel.Dolgov
+ */
+class JavaReflectionCompletionOverloadTest : LightFixtureCompletionTestCase() {
+
+  override fun getBasePath() = JavaTestUtil.getRelativeJavaTestDataPath() + "/codeInsight/completion/reflectionOverload/"
+
+  fun testOverloadMethods() = doTest(2, "method()", "method(C c)", "method(A a,B b)")
+
+  fun testJavaLangObjectMethods() = doTest(6,
+                                           "method()",
+                                           "equals(java.lang.Object obj)", "hashCode()", "toString()",
+                                           "getClass()", "notify()", "notifyAll()",
+                                           "wait()", "wait(long timeout)", "wait(long timeout,int nanos)"
+  )
+
+  fun testJavaLangObjectOwnMethods() = doTest(9,
+                                              "clone()", "equals(java.lang.Object obj)", "hashCode()",
+                                              "toString()", "finalize()", "getClass()",
+                                              "notify()", "notifyAll()",
+                                              "wait()", "wait(long timeout)", "wait(long timeout,int nanos)",
+                                              "registerNatives()")
+
+
+  private fun doTest(index: Int, vararg expected: String) {
+    configureByFile(getTestName(false) + ".java")
+
+    val lookupItems = lookup.items
+    val texts = lookupItemTexts(lookupItems, expected.size)
+    assertOrderedEquals(texts, *expected)
+    if (index >= 0) selectItem(lookupItems[index])
+    myFixture.checkResultByFile(getTestName(false) + "_after.java")
+  }
+}

java/java-tests/testSrc/com/intellij/codeInsight/completion/JavaReflectionCompletionTest.java

@@ -34,15 +34,15 @@ public class JavaReflectionCompletionTest extends LightFixtureCompletionTestCase
   }
 
   public void testDeclaredField() throws Exception {
-    doTest(1, "num", "num2", "num3");
+    doTest(2, "num", "num1", "num2");
   }
 
   public void testDeclaredMethod() throws Exception {
-    doTest(1, "method", "method2", "method3");
+    doTest(2, "method", "method1", "method2");
   }
 
   public void testDeclaredMethod2() throws Exception {
-    doTest(2, "method", "method2", "method3");
+    doTest(1, "method", "method1", "method2");
   }
 
   public void testMethod() throws Exception {
@@ -50,7 +50,7 @@ public class JavaReflectionCompletionTest extends LightFixtureCompletionTestCase
   }
 
   public void testForNameDeclaredMethod() throws Exception {
-    doTest(1, "method", "method2", "method3");
+    doTest(2, "method", "method1", "method2");
   }
 
   public void testForNameMethod() throws Exception {
@@ -62,7 +62,7 @@ public class JavaReflectionCompletionTest extends LightFixtureCompletionTestCase
   }
 
   public void testForNameDeclaredField() throws Exception {
-    doTest(2, "num", "num2", "num3");
+    doTest(1, "num", "num1", "num2");
   }
 
   public void testVarargMethod() throws Exception {
@@ -135,7 +135,7 @@ public class JavaReflectionCompletionTest extends LightFixtureCompletionTestCase
   }
 
   public void testConstantGetClassField() throws Exception {
-    doTest(1, "num", "num2", "num3");
+    doTest(2, "num", "num3", "num2");
   }
 
   public void testExpressionGetClassField() throws Exception {
@@ -165,9 +165,14 @@ public class JavaReflectionCompletionTest extends LightFixtureCompletionTestCase
     doTest(0, "PublicClass");
   }
 
+  public void testHasConstructor() {
+    doTest(2, "method", "method2", "method1");
+  }
+
+
   private void doTest(int index, String... expected) {
     configureByFile(getTestName(false) + ".java");
-    assertStringItems(expected);
+    assertFirstStringItems(expected);
     if (index >= 0) selectItem(getLookup().getItems().get(index));
     myFixture.checkResultByFile(getTestName(false) + "_after.java");
   }