fix: resolve methods collisions after type fix (#1263)

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

@@ -18,7 +18,9 @@ import jadx.core.clsp.ClspMethod;
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.nodes.MethodOverrideAttr;
+import jadx.core.dex.attributes.nodes.RenameReasonAttr;
 import jadx.core.dex.info.AccessInfo;
+import jadx.core.dex.info.MethodInfo;
 import jadx.core.dex.instructions.args.ArgType;
 import jadx.core.dex.nodes.ClassNode;
 import jadx.core.dex.nodes.IMethodDetails;
@@ -65,8 +67,12 @@ public class OverrideMethodVisitor extends AbstractVisitor {
 			mth.addAttr(attr);
 			IMethodDetails baseMth = Utils.last(attr.getOverrideList());
 			if (baseMth != null) {
-				fixMethodReturnType(mth, baseMth, superTypes);
-				fixMethodArgTypes(mth, baseMth, superTypes);
+				boolean updated = fixMethodReturnType(mth, baseMth, superTypes);
+				updated |= fixMethodArgTypes(mth, baseMth, superTypes);
+				if (updated && cls.root().getArgs().isRenameValid()) {
+					// check if new signature cause method collisions
+					fixMethodSignatureCollisions(mth);
+				}
 			}
 		}
 	}
@@ -248,14 +254,16 @@ public class OverrideMethodVisitor extends AbstractVisitor {
 		}
 	}
 
-	private void fixMethodReturnType(MethodNode mth, IMethodDetails baseMth, List<ArgType> superTypes) {
+	private boolean fixMethodReturnType(MethodNode mth, IMethodDetails baseMth, List<ArgType> superTypes) {
 		ArgType returnType = mth.getReturnType();
 		if (returnType == ArgType.VOID) {
-			return;
+			return false;
 		}
-		if (updateReturnType(mth, baseMth, superTypes)) {
+		boolean updated = updateReturnType(mth, baseMth, superTypes);
+		if (updated) {
 			mth.addInfoComment("Return type fixed from '" + returnType + "' to match base method");
 		}
+		return updated;
 	}
 
 	private boolean updateReturnType(MethodNode mth, IMethodDetails baseMth, List<ArgType> superTypes) {
@@ -283,15 +291,15 @@ public class OverrideMethodVisitor extends AbstractVisitor {
 		return false;
 	}
 
-	private void fixMethodArgTypes(MethodNode mth, IMethodDetails baseMth, List<ArgType> superTypes) {
+	private boolean fixMethodArgTypes(MethodNode mth, IMethodDetails baseMth, List<ArgType> superTypes) {
 		List<ArgType> mthArgTypes = mth.getArgTypes();
 		List<ArgType> baseArgTypes = baseMth.getArgTypes();
 		if (mthArgTypes.equals(baseArgTypes)) {
-			return;
+			return false;
 		}
 		int argCount = mthArgTypes.size();
 		if (argCount != baseArgTypes.size()) {
-			return;
+			return false;
 		}
 		boolean changed = false;
 		List<ArgType> newArgTypes = new ArrayList<>(argCount);
@@ -307,6 +315,7 @@ public class OverrideMethodVisitor extends AbstractVisitor {
 		if (changed) {
 			mth.updateArgTypes(newArgTypes, "Method arguments types fixed to match base method");
 		}
+		return changed;
 	}
 
 	private ArgType updateArgType(MethodNode mth, IMethodDetails baseMth, List<ArgType> superTypes, int argNum) {
@@ -333,4 +342,38 @@ public class OverrideMethodVisitor extends AbstractVisitor {
 		}
 		return null;
 	}
+
+	private void fixMethodSignatureCollisions(MethodNode mth) {
+		String mthName = mth.getMethodInfo().getAlias();
+		String newSignature = MethodInfo.makeShortId(mthName, mth.getArgTypes(), null);
+		for (MethodNode otherMth : mth.getParentClass().getMethods()) {
+			String otherMthName = otherMth.getAlias();
+			if (otherMthName.equals(mthName) && otherMth != mth) {
+				String otherSignature = otherMth.getMethodInfo().makeSignature(true, false);
+				if (otherSignature.equals(newSignature)) {
+					if (otherMth.contains(AFlag.DONT_RENAME) || otherMth.contains(AType.METHOD_OVERRIDE)) {
+						otherMth.addWarnComment("Can't rename method to resolve collision");
+					} else {
+						otherMth.getMethodInfo().setAlias(makeNewAlias(otherMth));
+						otherMth.addAttr(new RenameReasonAttr("avoid collision after fix types in other method"));
+					}
+				}
+			}
+		}
+	}
+
+	// TODO: at this point deobfuscator is not available and map file already saved
+	private static String makeNewAlias(MethodNode mth) {
+		ClassNode cls = mth.getParentClass();
+		String baseName = mth.getAlias();
+		int k = 2;
+		while (true) {
+			String alias = baseName + k;
+			MethodNode methodNode = cls.searchMethodByShortName(alias);
+			if (methodNode == null) {
+				return alias;
+			}
+			k++;
+		}
+	}
 }

jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java

@@ -123,6 +123,7 @@ public abstract class IntegrationTest extends TestUtils {
 		args.setThreadsCount(1);
 		args.setSkipResources(true);
 		args.setFsCaseSensitive(false); // use same value on all systems
+		args.setCommentsLevel(CommentsLevel.DEBUG);
 	}
 
 	@AfterEach

jadx-core/src/test/java/jadx/tests/integration/generics/TestSyntheticOverride.java

+package jadx.tests.integration.generics;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
+
+public class TestSyntheticOverride extends SmaliTest {
+	// @formatter:off
+	/*
+		final class TestSyntheticOverride extends Lambda implements Function1<String, Unit> {
+
+			// fixing method types to match interface (i.e Unit invoke(String str))
+			// make duplicate methods signatures
+			public bridge synthetic Object invoke(Object str) {
+				invoke(str);
+				return Unit.INSTANCE;
+			}
+
+			public final void invoke(String str) {
+				...
+			}
+		}
+	*/
+	// @formatter:on
+
+	@Test
+	public void test() {
+		allowWarnInCode();
+		disableCompilation();
+		List<ClassNode> classNodes = loadFromSmaliFiles();
+		assertThat(searchCls(classNodes, "TestSyntheticOverride"))
+				.code()
+				.containsOne("invoke(String str)")
+				.containsOne("invoke2(String str)");
+	}
+}

jadx-core/src/test/smali/generics/TestSyntheticOverride/KotlinFunction1.smali

+.class public interface abstract Lkotlin/jvm/functions/Function1;
+.super Ljava/lang/Object;
+.source "SourceFile"
+
+.implements Lkotlin/Function;
+
+.annotation system Ldalvik/annotation/Signature;
+    value = {
+        "<P1:",
+        "Ljava/lang/Object;",
+        "R:",
+        "Ljava/lang/Object;",
+        ">",
+        "Ljava/lang/Object;",
+        "Lkotlin/Function<",
+        "TR;>;"
+    }
+.end annotation
+
+.method public abstract invoke(Ljava/lang/Object;)Ljava/lang/Object;
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "(TP1;)TR;"
+        }
+    .end annotation
+.end method

jadx-core/src/test/smali/generics/TestSyntheticOverride/TestSyntheticOverride.smali

+.class final Lgenerics/TestSyntheticOverride;
+.super Ljava/lang/Object;
+
+.implements Lkotlin/jvm/functions/Function1;
+
+.annotation system Ldalvik/annotation/Signature;
+    value = {
+        "Lkotlin/jvm/internal/Lambda;",
+        "Lkotlin/jvm/functions/Function1<",
+        "Ljava/lang/String;",
+        "Lkotlin/Unit;",
+        ">;"
+    }
+.end annotation
+
+.field final synthetic this$0:Lgenerics/TestSyntheticOverrideParent;
+
+
+.method public bridge synthetic invoke(Ljava/lang/Object;)Ljava/lang/Object;
+    .registers 2
+    check-cast p1, Ljava/lang/String;
+    invoke-virtual {p0, p1}, Lgenerics/TestSyntheticOverride;->invoke(Ljava/lang/String;)V
+    sget-object p1, Lkotlin/Unit;->INSTANCE:Lkotlin/Unit;
+    return-object p1
+.end method
+
+.method public final invoke(Ljava/lang/String;)V
+    .registers 5
+    iget-object v0, p0, Lgenerics/TestSyntheticOverride;->this$0:Lgenerics/TestSyntheticOverrideParent;
+    invoke-static {v0}, Lgenerics/TestSyntheticOverride;->access$getDialog$p(Lgenerics/TestSyntheticOverride;)Landroidx/appcompat/app/AlertDialog;
+    move-result-object v0
+    if-nez v0, :cond_9
+    goto :goto_c
+
+    :cond_9
+    invoke-virtual {v0}, Landroidx/appcompat/app/AppCompatDialog;->dismiss()V
+
+    :goto_c
+    iget-object v0, p0, Lgenerics/TestSyntheticOverride;->this$0:Lgenerics/TestSyntheticOverrideParent;
+    invoke-virtual {v0}, Landroid/app/Activity;->getIntent()Landroid/content/Intent;
+    move-result-object v1
+    const-string v2, "intent"
+    invoke-static {v1, v2}, Lkotlin/jvm/internal/Intrinsics;->checkNotNullExpressionValue(Ljava/lang/Object;Ljava/lang/String;)V
+    invoke-static {v0, v1, p1}, Lgenerics/TestSyntheticOverride;->access$getChooserIntent(Lgenerics/TestSyntheticOverride;Landroid/content/Intent;Ljava/lang/String;)Landroid/content/Intent;
+    move-result-object p1
+    invoke-virtual {v0, p1}, Landroid/app/Activity;->startActivity(Landroid/content/Intent;)V
+    iget-object p1, p0, Lgenerics/TestSyntheticOverride;->this$0:Lgenerics/TestSyntheticOverrideParent;
+    invoke-virtual {p1}, Landroid/app/Activity;->finish()V
+    return-void
+.end method