Consider overloads when applying method mappings (#8)

* Consider overloads when applying method mappings * Add a test case for the partial match case. --------- Co-authored-by: Sebastian Hartte <shartte@users.noreply.github.com>
neoforged · Feb 2, 2024 · 4780259 · 4780259
1 parent 789d2a5
commit 4780259
Show file tree

Hide file tree

Showing 9 changed files with 132 additions and 4 deletions.
diff --git a/api/src/main/java/net/neoforged/jst/api/PsiHelper.java b/api/src/main/java/net/neoforged/jst/api/PsiHelper.java
@@ -16,6 +16,9 @@
 import com.intellij.util.containers.ObjectIntHashMap;
 import org.jetbrains.annotations.NotNull;
 
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 
@@ -24,6 +27,58 @@ public static String getBinaryMethodName(PsiMethod psiMethod) {
         return psiMethod.isConstructor() ? "<init>" : psiMethod.getName();
     }
 
+    public static Iterator<String> getOverloadedSignatures(PsiMethod method) {
+        final List<String> parameters = new ArrayList<>();
+        final var returnType = Optional.ofNullable(method.getReturnType()).orElse(PsiTypes.voidType());
+        String returnTypeRepresentation = ClassUtil.getBinaryPresentation(returnType);
+        if (returnTypeRepresentation.isEmpty()) {
+            System.err.println("Failed to create binary representation for type " + returnType.getCanonicalText());
+            returnTypeRepresentation = "ERROR";
+        }
+        final String retRep = returnTypeRepresentation;
+
+        // Add implicit constructor parameters
+        // Private enumeration constructors have two hidden parameters (enun name+ordinal)
+        if (isEnumConstructor(method)) {
+            parameters.add("Ljava/lang/String;I");
+        }
+        // Non-Static inner class constructors have the enclosing class as their first argument
+        else if (isNonStaticInnerClassConstructor(method)) {
+            var parent = Objects.requireNonNull(Objects.requireNonNull(method.getContainingClass()).getContainingClass());
+            final StringBuilder par = new StringBuilder();
+            par.append("L");
+            getBinaryClassName(parent, par);
+            par.append(";");
+            parameters.add(par.toString());
+        }
+
+        for (PsiParameter param : method.getParameterList().getParameters()) {
+            var binaryPresentation = ClassUtil.getBinaryPresentation(param.getType());
+            if (binaryPresentation.isEmpty()) {
+                System.err.println("Failed to create binary representation for type " + param.getType().getCanonicalText());
+                binaryPresentation = "ERROR";
+            }
+            parameters.add(binaryPresentation);
+        }
+
+        return new Iterator<>() {
+            @Override
+            public boolean hasNext() {
+                return !parameters.isEmpty();
+            }
+
+            @Override
+            public String next() {
+                StringBuilder signature = new StringBuilder();
+                signature.append("(");
+                parameters.forEach(signature::append);
+                signature.append(")").append(retRep);
+                parameters.remove(parameters.size() - 1);
+                return signature.toString();
+            }
+        };
+    }
+
     public static String getBinaryMethodSignature(PsiMethod method) {
         StringBuilder signature = new StringBuilder();
         signature.append("(");

diff --git a/cli/src/test/java/net/neoforged/jst/cli/PsiHelperTest.java b/cli/src/test/java/net/neoforged/jst/cli/PsiHelperTest.java
@@ -195,6 +195,20 @@ void testMethodParameterIndices() {
         }
     }
 
+    @Test
+    void testPossibleSignatures() {
+        var m = parseSingleMethod("""
+                class Outer {
+                    static boolean m(int p1, boolean p2, long p3) {
+                        return false;
+                    }
+                }
+                """);
+        assertThat(PsiHelper.getOverloadedSignatures(m))
+                .toIterable()
+                .containsExactly("(IZJ)Z", "(IZ)Z", "(I)Z");
+    }
+
     @Test
     void testLvtIndicesForPrimitiveTypes() {
         var m = parseSingleMethod("""

diff --git a/parchment/src/main/java/net/neoforged/jst/parchment/PsiParchmentHelper.java b/parchment/src/main/java/net/neoforged/jst/parchment/PsiParchmentHelper.java
@@ -52,8 +52,10 @@ public static NamesAndDocsForMethod getMethodData(NamesAndDocsDatabase namesAndD
             var classData = getClassData(namesAndDocs, psiMethod.getContainingClass());
             if (classData != null) {
                 var methodName = PsiHelper.getBinaryMethodName(psiMethod);
-                var methodSignature = PsiHelper.getBinaryMethodSignature(psiMethod);
-                methodData = Optional.ofNullable(classData.getMethod(methodName, methodSignature));
+                var signatures = PsiHelper.getOverloadedSignatures(psiMethod);
+                while (signatures.hasNext() && methodData.isEmpty()) {
+                    methodData = Optional.ofNullable(classData.getMethod(methodName, signatures.next()));
+                }
             }
 
             psiMethod.putUserData(METHOD_DATA_KEY, methodData);

diff --git a/tests/data/external_refs/expected/TestClass.java b/tests/data/external_refs/expected/TestClass.java
@@ -9,6 +9,6 @@ public void m(String a, List<String> b, Test c) {
     // This method tests that unresolvable external references aren't simply dropped
     // in the method signature when looking up Parchment data. The IntelliJ default methods
     // would do this. This overload should not pick up data from the first method.
-    public void m(String a1, List<String> a2, Test a3, AnExternalClassThatDoesNotExist a4) {
+    public void m(String a1, List<String> a2, AnExternalClassThatDoesNotExist a4, Test a3) {
     }
 }
diff --git a/tests/data/external_refs/source/TestClass.java b/tests/data/external_refs/source/TestClass.java
@@ -9,6 +9,6 @@ public void m(String a1, List<String> a2, Test a3) {
     // This method tests that unresolvable external references aren't simply dropped
     // in the method signature when looking up Parchment data. The IntelliJ default methods
     // would do this. This overload should not pick up data from the first method.
-    public void m(String a1, List<String> a2, Test a3, AnExternalClassThatDoesNotExist a4) {
+    public void m(String a1, List<String> a2, AnExternalClassThatDoesNotExist a4, Test a3) {
     }
 }
diff --git a/tests/data/partial_matches/expected/TestClass.java b/tests/data/partial_matches/expected/TestClass.java
@@ -0,0 +1,11 @@
+import org.junit.jupiter.api.Test;
+
+import java.util.*;
+
+public class TestClass {
+    // This method tests that we consider parchment entries that are
+    // for methods with strictly fewer parameter. And we prioritize
+    // the entries with the most overlap.
+    public void m(String a, List<String> a2, int a3) {
+    }
+}
diff --git a/tests/data/partial_matches/parchment.json b/tests/data/partial_matches/parchment.json
@@ -0,0 +1,30 @@
+{
+  "version": "1.1.0",
+  "classes": [
+    {
+      "name": "TestClass",
+      "methods": [
+        {
+          "name": "m",
+          "descriptor": "(Ljava/lang/String;)V",
+          "parameters": [
+            {
+              "index": 1,
+              "name": "wrong"
+            }
+          ]
+        },
+        {
+          "name": "m",
+          "descriptor": "(Ljava/lang/String;Ljava/util/List;)V",
+          "parameters": [
+            {
+              "index": 1,
+              "name": "a"
+            }
+          ]
+        }
+      ]
+    }
+  ]
+}
diff --git a/tests/data/partial_matches/source/TestClass.java b/tests/data/partial_matches/source/TestClass.java
@@ -0,0 +1,11 @@
+import org.junit.jupiter.api.Test;
+
+import java.util.*;
+
+public class TestClass {
+    // This method tests that we consider parchment entries that are
+    // for methods with strictly fewer parameter. And we prioritize
+    // the entries with the most overlap.
+    public void m(String a1, List<String> a2, int a3) {
+    }
+}
diff --git a/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java b/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java
@@ -194,6 +194,11 @@ void testExternalReferences() throws Exception {
         runTest("external_refs", "parchment.json");
     }
 
+    @Test
+    void testPartialMatches() throws Exception {
+        runTest("partial_matches", "parchment.json");
+    }
+
     @Test
     void testParamIndices() throws Exception {
         runTest("param_indices", "parchment.json");