core: rename wrapped synthetic method (#336)

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

@@ -11,8 +11,10 @@ import jadx.core.dex.info.FieldInfo;
 import jadx.core.dex.info.MethodInfo;
 import jadx.core.dex.instructions.IndexInsnNode;
 import jadx.core.dex.instructions.InsnType;
+import jadx.core.dex.instructions.InvokeNode;
 import jadx.core.dex.instructions.args.ArgType;
 import jadx.core.dex.instructions.args.InsnArg;
+import jadx.core.dex.instructions.args.InsnWrapArg;
 import jadx.core.dex.instructions.args.RegisterArg;
 import jadx.core.dex.instructions.args.SSAVar;
 import jadx.core.dex.instructions.mods.ConstructorInsn;
@@ -44,8 +46,8 @@ public class ClassModifier extends AbstractVisitor {
 			return false;
 		}
 		removeSyntheticFields(cls);
-		removeSyntheticMethods(cls);
-		removeEmptyMethods(cls);
+		cls.getMethods().forEach(mth -> removeSyntheticMethods(cls, mth));
+		cls.getMethods().forEach(ClassModifier::removeEmptyMethods);
 
 		markAnonymousClass(cls);
 		return false;
@@ -122,24 +124,23 @@ public class ClassModifier extends AbstractVisitor {
 		return true;
 	}
 
-	private static void removeSyntheticMethods(ClassNode cls) {
-		for (MethodNode mth : cls.getMethods()) {
-			if (mth.isNoCode()) {
-				continue;
-			}
-			AccessInfo af = mth.getAccessFlags();
-			// remove bridge methods
-			if (af.isBridge() && af.isSynthetic() && !isMethodUniq(cls, mth)) {
-				// TODO add more checks before method deletion
-				mth.add(AFlag.DONT_GENERATE);
-			} else {
-				// remove synthetic constructor for inner classes
-				if (af.isSynthetic() && af.isConstructor() && mth.getBasicBlocks().size() == 2) {
-					List<RegisterArg> args = mth.getArguments(false);
-					if (isRemovedClassInArgs(cls, args)) {
-						modifySyntheticMethod(cls, mth, args);
-					}
-				}
+	private static void removeSyntheticMethods(ClassNode cls, MethodNode mth) {
+		if (mth.isNoCode()) {
+			return;
+		}
+		AccessInfo af = mth.getAccessFlags();
+		if (!af.isSynthetic()) {
+			return;
+		}
+		if (removeBridgeMethod(cls, mth)) {
+			mth.add(AFlag.DONT_GENERATE);
+			return;
+		}
+		// remove synthetic constructor for inner classes
+		if (af.isConstructor() && mth.getBasicBlocks().size() == 2) {
+			List<RegisterArg> args = mth.getArguments(false);
+			if (isRemovedClassInArgs(cls, args)) {
+				modifySyntheticMethod(cls, mth, args);
 			}
 		}
 	}
@@ -192,7 +193,40 @@ public class ClassModifier extends AbstractVisitor {
 		}
 	}
 
-	private static boolean isMethodUniq(ClassNode cls, MethodNode mth) {
+	private static boolean removeBridgeMethod(ClassNode cls, MethodNode mth) {
+		List<InsnNode> allInsns = BlockUtils.collectAllInsns(mth.getBasicBlocks());
+		if (allInsns.size() == 1) {
+			InsnNode wrappedInsn = allInsns.get(0);
+			if (wrappedInsn.getType() == InsnType.RETURN) {
+				InsnArg arg = wrappedInsn.getArg(0);
+				if (arg.isInsnWrap()) {
+					wrappedInsn = ((InsnWrapArg) arg).getWrapInsn();
+				}
+			}
+			if (checkSyntheticWrapper(mth, wrappedInsn)) {
+				return true;
+			}
+		}
+		return !isMethodUnique(cls, mth);
+	}
+
+	private static boolean checkSyntheticWrapper(MethodNode mth, InsnNode insn) {
+		InsnType insnType = insn.getType();
+		if (insnType == InsnType.INVOKE) {
+			MethodInfo callMth = ((InvokeNode) insn).getCallMth();
+			MethodNode wrappedMth = mth.root().deepResolveMethod(callMth);
+			if (wrappedMth != null) {
+				String alias = mth.getAlias();
+				if (!wrappedMth.getAlias().equals(alias) && wrappedMth.isVirtual()) {
+					wrappedMth.getMethodInfo().setAlias(alias);
+				}
+				return true;
+			}
+		}
+		return false;
+	}
+
+	private static boolean isMethodUnique(ClassNode cls, MethodNode mth) {
 		MethodInfo mi = mth.getMethodInfo();
 		for (MethodNode otherMth : cls.getMethods()) {
 			if (otherMth != mth) {
@@ -207,19 +241,16 @@ public class ClassModifier extends AbstractVisitor {
 		return true;
 	}
 
-	private static void removeEmptyMethods(ClassNode cls) {
-		for (MethodNode mth : cls.getMethods()) {
-			AccessInfo af = mth.getAccessFlags();
-
-			// remove public empty constructors
-			if (af.isConstructor()
-					&& (af.isPublic() || af.isStatic())
-					&& mth.getArguments(false).isEmpty()
-					&& !mth.contains(AType.JADX_ERROR)) {
-				List<BlockNode> bb = mth.getBasicBlocks();
-				if (bb == null || bb.isEmpty() || BlockUtils.isAllBlocksEmpty(bb)) {
-					mth.add(AFlag.DONT_GENERATE);
-				}
+	private static void removeEmptyMethods(MethodNode mth) {
+		AccessInfo af = mth.getAccessFlags();
+		// remove public empty constructors
+		if (af.isConstructor()
+				&& (af.isPublic() || af.isStatic())
+				&& mth.getArguments(false).isEmpty()
+				&& !mth.contains(AType.JADX_ERROR)) {
+			List<BlockNode> bb = mth.getBasicBlocks();
+			if (bb == null || bb.isEmpty() || BlockUtils.isAllBlocksEmpty(bb)) {
+				mth.add(AFlag.DONT_GENERATE);
 			}
 		}
 	}

jadx-core/src/main/java/jadx/core/utils/BlockUtils.java

@@ -550,4 +550,10 @@ public class BlockUtils {
 		}
 		return true;
 	}
+
+	public static List<InsnNode> collectAllInsns(List<BlockNode> blocks) {
+		List<InsnNode> insns = new ArrayList<>();
+		blocks.forEach(block -> insns.addAll(block.getInstructions()));
+		return insns;
+	}
 }

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

+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/336
+ */
+public class TestInnerClassSyntheticRename extends SmaliTest {
+
+//	private class MyAsync extends AsyncTask<Uri, Uri, List<Uri>> {
+//		@Override
+//		protected List<Uri> doInBackground(Uri... uris) {
+//			Log.i("MyAsync", "doInBackground");
+//			return null;
+//		}
+//
+//		@Override
+//		protected void onPostExecute(List<Uri> uris) {
+//			Log.i("MyAsync", "onPostExecute");
+//		}
+//	}
+
+	@Test
+	public void test() {
+		disableCompilation();
+		ClassNode cls = getClassNodeFromSmali("inner/TestInnerClassSyntheticRename", "com.github.skylot.testasync.MyAsync");
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("protected List<Uri> doInBackground(Uri... uriArr) {"));
+		assertThat(code, containsOne("protected void onPostExecute(List<Uri> list) {"));
+		assertThat(code, not(containsString("synthetic")));
+	}
+}

jadx-core/src/test/smali/inner/TestInnerClassSyntheticRename.smali

+.class Lcom/github/skylot/testasync/MyAsync;
+.super Landroid/os/AsyncTask;
+
+
+# annotations
+.annotation system Ldalvik/annotation/Signature;
+    value = {
+        "Landroid/os/AsyncTask<",
+        "Landroid/net/Uri;",
+        "Landroid/net/Uri;",
+        "Ljava/util/List<",
+        "Landroid/net/Uri;",
+        ">;>;"
+    }
+.end annotation
+
+
+# direct methods
+.method private constructor <init>(Lcom/github/skylot/testasync/MainActivity;)V
+    .locals 0
+
+    invoke-direct {p0}, Landroid/os/AsyncTask;-><init>()V
+
+    return-void
+.end method
+
+
+# virtual methods
+.method protected varargs a([Landroid/net/Uri;)Ljava/util/List;
+    .locals 1
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "([",
+            "Landroid/net/Uri;",
+            ")",
+            "Ljava/util/List<",
+            "Landroid/net/Uri;",
+            ">;"
+        }
+    .end annotation
+
+    const-string p1, "MyAsync"
+
+    const-string v0, "doInBackground"
+
+    invoke-static {p1, v0}, Landroid/util/Log;->i(Ljava/lang/String;Ljava/lang/String;)I
+
+    const/4 p1, 0x0
+
+    return-object p1
+.end method
+
+.method protected a(Ljava/util/List;)V
+    .locals 1
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "(",
+            "Ljava/util/List<",
+            "Landroid/net/Uri;",
+            ">;)V"
+        }
+    .end annotation
+
+    const-string p1, "MyAsync"
+
+    const-string v0, "onPostExecute"
+
+    invoke-static {p1, v0}, Landroid/util/Log;->i(Ljava/lang/String;Ljava/lang/String;)I
+
+    return-void
+.end method
+
+.method protected synthetic doInBackground([Ljava/lang/Object;)Ljava/lang/Object;
+    .locals 0
+
+    check-cast p1, [Landroid/net/Uri;
+
+    invoke-virtual {p0, p1}, Lcom/github/skylot/testasync/MyAsync;->a([Landroid/net/Uri;)Ljava/util/List;
+
+    move-result-object p1
+
+    return-object p1
+.end method
+
+.method protected synthetic onPostExecute(Ljava/lang/Object;)V
+    .locals 0
+
+    check-cast p1, Ljava/util/List;
+
+    invoke-virtual {p0, p1}, Lcom/github/skylot/testasync/MyAsync;->a(Ljava/util/List;)V
+
+    return-void
+.end method