fix: improve bridge methods renaming (#397)

jadx-core/src/main/java/jadx/core/codegen/MethodGen.java

@@ -4,15 +4,15 @@ import java.util.Iterator;
 import java.util.List;
 
 import com.android.dx.rop.code.AccessFlags;
-import jadx.core.dex.info.ClassInfo;
-import jadx.core.utils.Utils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import jadx.core.Consts;
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.annotations.MethodParameters;
 import jadx.core.dex.info.AccessInfo;
+import jadx.core.dex.info.ClassInfo;
 import jadx.core.dex.instructions.InsnType;
 import jadx.core.dex.instructions.args.ArgType;
 import jadx.core.dex.instructions.args.RegisterArg;
@@ -24,6 +24,7 @@ import jadx.core.dex.visitors.DepthTraversal;
 import jadx.core.dex.visitors.FallbackModeVisitor;
 import jadx.core.utils.ErrorsCounter;
 import jadx.core.utils.InsnUtils;
+import jadx.core.utils.Utils;
 import jadx.core.utils.exceptions.CodegenException;
 import jadx.core.utils.exceptions.DecodeException;
 
@@ -85,6 +86,9 @@ public class MethodGen {
 		}
 		code.startLineWithNum(mth.getSourceLine());
 		code.add(ai.makeString());
+		if (Consts.DEBUG) {
+			code.add(mth.isVirtual() ? "/* virtual */ " : "/* direct */ ");
+		}
 
 		if (classGen.addGenericMap(code, mth.getGenericMap())) {
 			code.add(' ');

jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java

@@ -3,6 +3,8 @@ package jadx.core.dex.visitors;
 import java.util.List;
 import java.util.Objects;
 
+import com.android.dx.rop.code.AccessFlags;
+
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.nodes.FieldReplaceAttr;
@@ -31,7 +33,10 @@ import jadx.core.utils.exceptions.JadxException;
 @JadxVisitor(
 		name = "ClassModifier",
 		desc = "Remove synthetic classes, methods and fields",
-		runAfter = ModVisitor.class
+		runAfter = {
+				ModVisitor.class,
+				FixAccessModifiers.class
+		}
 )
 public class ClassModifier extends AbstractVisitor {
 
@@ -218,6 +223,10 @@ public class ClassModifier extends AbstractVisitor {
 			MethodInfo callMth = ((InvokeNode) insn).getCallMth();
 			MethodNode wrappedMth = mth.root().deepResolveMethod(callMth);
 			if (wrappedMth != null) {
+				AccessInfo wrappedAccFlags = wrappedMth.getAccessFlags();
+				if (wrappedAccFlags.isStatic()) {
+					return false;
+				}
 				if (callMth.getArgsCount() != mth.getMethodInfo().getArgsCount()) {
 					return false;
 				}
@@ -235,8 +244,9 @@ public class ClassModifier extends AbstractVisitor {
 				if (Objects.equals(wrappedMth.getAlias(), alias)) {
 					return true;
 				}
-				if (!wrappedMth.isVirtual()) {
-					return false;
+				if (!wrappedAccFlags.isPublic()) {
+					// must be public
+					FixAccessModifiers.changeVisibility(wrappedMth, AccessFlags.ACC_PUBLIC);
 				}
 				wrappedMth.getMethodInfo().setAlias(alias);
 				return true;

jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java

@@ -26,22 +26,27 @@ public class FixAccessModifiers extends AbstractVisitor {
 		if (respectAccessModifiers) {
 			return;
 		}
-		AccessInfo accessFlags = mth.getAccessFlags();
-		int newVisFlag = fixVisibility(mth, accessFlags);
+		int newVisFlag = fixVisibility(mth);
 		if (newVisFlag != 0) {
-			AccessInfo newAccFlags = accessFlags.changeVisibility(newVisFlag);
-			if (newAccFlags != accessFlags) {
-				mth.setAccFlags(newAccFlags);
-				mth.addAttr(AType.COMMENTS, "Access modifiers changed, original: " + accessFlags.rawString());
-			}
+			changeVisibility(mth, newVisFlag);
 		}
 	}
 
-	private int fixVisibility(MethodNode mth, AccessInfo accessFlags) {
+	public static void changeVisibility(MethodNode mth, int newVisFlag) {
+		AccessInfo accessFlags = mth.getAccessFlags();
+		AccessInfo newAccFlags = accessFlags.changeVisibility(newVisFlag);
+		if (newAccFlags != accessFlags) {
+			mth.setAccFlags(newAccFlags);
+			mth.addAttr(AType.COMMENTS, "access modifiers changed from: " + accessFlags.rawString());
+		}
+	}
+
+	private static int fixVisibility(MethodNode mth) {
 		if (mth.isVirtual()) {
 			// make virtual methods public
 			return AccessFlags.ACC_PUBLIC;
 		} else {
+			AccessInfo accessFlags = mth.getAccessFlags();
 			if (accessFlags.isAbstract()) {
 				// make abstract methods public
 				return AccessFlags.ACC_PUBLIC;

jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java

@@ -0,0 +1,77 @@
+package jadx.tests.integration.inner;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestInner2Samples extends IntegrationTest {
+
+	public static class TestInner2 {
+
+		private String a;
+
+		public class A {
+			public A() {
+				a = "a";
+			}
+
+			public String a() {
+				return a;
+			}
+		}
+
+		private static String b;
+
+		public static class B {
+			public B() {
+				b = "b";
+			}
+
+			public String b() {
+				return b;
+			}
+		}
+
+		private String c;
+
+		private void setC(String c) {
+			this.c = c;
+		}
+
+		public class C {
+			public String c() {
+				setC("c");
+				return c;
+			}
+		}
+
+		private static String d;
+
+		private static void setD(String s) {
+			d = s;
+		}
+
+		public static class D {
+			public String d() {
+				setD("d");
+				return d;
+			}
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestInner2.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("setD(\"d\");"));
+		assertThat(code, not(containsString("synthetic")));
+		assertThat(code, not(containsString("access$")));
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java

@@ -0,0 +1,44 @@
+package jadx.tests.integration.inner;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertThat;
+
+/**
+ * Issue: https://github.com/skylot/jadx/issues/397
+ */
+public class TestSyntheticMthRename extends SmaliTest {
+
+//	public class TestCls {
+//		public interface I<R, P> {
+//			R call(P... p);
+//		}
+//
+//		public static final class A implements I<String, Runnable> {
+//			public /* synthetic */ /* virtual */ Object call(Object[] objArr) {
+//				return renamedCall((Runnable[]) objArr);
+//			}
+//
+//			private /* varargs */ /* direct */ String renamedCall(Runnable... p) {
+//				return "str";
+//			}
+//		}
+//	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNodeFromSmaliFiles("inner", "TestSyntheticMthRename", "TestCls",
+				"TestCls", "TestCls$I", "TestCls$A"
+		);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("public String call(Runnable... p) {"));
+		assertThat(code, not(containsString("synthetic")));
+	}
+}

jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali

@@ -0,0 +1,66 @@
+.class public final Linner/TestCls$A;
+.super Ljava/lang/Object;
+.source "TestCls.java"
+
+# interfaces
+.implements Linner/TestCls$I;
+
+
+# annotations
+.annotation system Ldalvik/annotation/EnclosingClass;
+    value = Linner/TestCls;
+.end annotation
+
+.annotation system Ldalvik/annotation/InnerClass;
+    accessFlags = 0x19
+    name = "A"
+.end annotation
+
+.annotation system Ldalvik/annotation/Signature;
+    value = {
+        "Ljava/lang/Object;",
+        "Linner/TestCls$I",
+        "<",
+        "Ljava/lang/String;",
+        "Ljava/lang/Runnable;",
+        ">;"
+    }
+.end annotation
+
+
+# direct methods
+.method public constructor <init>()V
+    .registers 1
+
+    .prologue
+    .line 9
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+
+    return-void
+.end method
+
+.method private varargs renamedCall([Ljava/lang/Runnable;)Ljava/lang/String;
+    .registers 3
+    .param p1, "p"    # [Ljava/lang/Runnable;
+
+    .prologue
+    .line 12
+    const-string v0, "str"
+
+    return-object v0
+.end method
+
+# virtual methods
+.method public synthetic call([Ljava/lang/Object;)Ljava/lang/Object;
+    .registers 3
+
+    .prologue
+    .line 9
+    check-cast p1, [Ljava/lang/Runnable;
+
+    invoke-virtual {p0, p1}, Linner/TestCls$A;->renamedCall([Ljava/lang/Runnable;)Ljava/lang/String;
+
+    move-result-object v0
+
+    return-object v0
+.end method

jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali

@@ -0,0 +1,35 @@
+.class public interface abstract Linner/TestCls$I;
+.super Ljava/lang/Object;
+.source "TestCls.java"
+
+
+# annotations
+.annotation system Ldalvik/annotation/EnclosingClass;
+    value = Linner/TestCls;
+.end annotation
+
+.annotation system Ldalvik/annotation/InnerClass;
+    accessFlags = 0x609
+    name = "I"
+.end annotation
+
+.annotation system Ldalvik/annotation/Signature;
+    value = {
+        "<R:",
+        "Ljava/lang/Object;",
+        "P:",
+        "Ljava/lang/Object;",
+        ">",
+        "Ljava/lang/Object;"
+    }
+.end annotation
+
+
+# virtual methods
+.method public varargs abstract call([Ljava/lang/Object;)Ljava/lang/Object;
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "([TP;)TR;"
+        }
+    .end annotation
+.end method

jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali

@@ -0,0 +1,24 @@
+.class public Linner/TestCls;
+.super Ljava/lang/Object;
+.source "TestCls.java"
+
+
+# annotations
+.annotation system Ldalvik/annotation/MemberClasses;
+    value = {
+        Linner/TestCls$A;,
+        Linner/TestCls$I;
+    }
+.end annotation
+
+
+# direct methods
+.method public constructor <init>()V
+    .registers 1
+
+    .prologue
+    .line 3
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+
+    return-void
+.end method