[core] Decompilation of empty varargs / vararg overloads

[yawkat]

Calls to vararg functions with an empty array parameter are not sugared as a varargs call because they are a NEW_ARRAY with a zero literal as the arg instead of a FILLED_NEW_ARRAY. This can be fixed easily:

Index: jadx-core/src/main/java/jadx/core/codegen/InsnGen.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java
--- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java    (revision 901f9f0a9675ddaddd0c5924b56c4f734e3eaa1f)
+++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java    (date 1607887256284)
@@ -821,6 +821,13 @@
            return false;
        }
        InsnNode insn = ((InsnWrapArg) lastArg).getWrapInsn();
+       if (insn.getType() == InsnType.NEW_ARRAY) {
+           InsnArg onlyArg = insn.getArg(0);
+           if (onlyArg.isZeroLiteral()) {
+               // don't need to pass any parameters
+               return true;
+           }
+       }
        if (insn.getType() != InsnType.FILLED_NEW_ARRAY) {
            return false;
        }

However, the issue with this approach is that it leads to ambiguity with overloaded methods. When test() and test(int... a) exist, the new int[0] would have to be retained. This appears to be an issue with all vararg decompilation right now, but I'm still vary of submitting what is technically a regression before the overload issue is fixed. Unfortunately vararg sugaring happens after overload resolution, making this a bit difficult. One idea is adding an AFlag that preemptively marks any invocations with possible non-vararg conflicts and disabling vararg sugaring for those methods altogether, but this is fairly conservative.

Test case:

Index: jadx-core/src/test/java/jadx/tests/integration/invoke/TestVarArg.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/jadx-core/src/test/java/jadx/tests/integration/invoke/TestVarArg.java b/jadx-core/src/test/java/jadx/tests/integration/invoke/TestVarArg.java
--- a/jadx-core/src/test/java/jadx/tests/integration/invoke/TestVarArg.java (revision 901f9f0a9675ddaddd0c5924b56c4f734e3eaa1f)
+++ b/jadx-core/src/test/java/jadx/tests/integration/invoke/TestVarArg.java (date 1607887511792)
@@ -21,10 +21,21 @@
        public void test3(int[] a) {
        }

+       public void test4(int... vararg) {
+
+       }
+
+       // overload
+       public void test4() {
+
+       }
+
        public void call() {
+           test1();
            test1(1, 2);
            test2(3, "1", 7);
            test3(new int[] { 5, 8 });
+           test4(new int[0]);
        }
    }

@@ -37,10 +48,13 @@
        assertThat(code, containsString("void test2(int i, Object... a) {"));

        assertThat(code, containsString("test1(1, 2);"));
+       assertThat(code, containsString("test1();"));
        assertThat(code, containsString("test2(3, \"1\", 7);"));

        // negative case
        assertThat(code, containsString("void test3(int[] a) {"));
        assertThat(code, containsString("test3(new int[]{5, 8});"));
+
+       assertThat(code, containsString("test4(new int[0]);"));
    }
 }

[skylot]

@yawkat I like the idea with a flag like ALLOW_OMIT_VARARG which is added if there is no method with the same name and args count minus one (similar to MethodInfo.isOverloadedBy). Good place to add that flag is here. This will cover most cases. For complete fix, we need to directly modify insn (remove that arg) in MethodInvokeVisitor and run overload method processing for that modified method. In latter case flag is not needed, but implementation is more complex.