fix: avoid local variables collision with full class names (#647)

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

@@ -1,6 +1,6 @@
 package jadx.core.codegen;
 
-import java.util.LinkedHashSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -30,7 +30,7 @@ public class NameGen {
 
 	private static final Map<String, String> OBJ_ALIAS;
 
-	private final Set<String> varNames = new LinkedHashSet<>();
+	private final Set<String> varNames = new HashSet<>();
 	private final MethodNode mth;
 	private final boolean fallback;
 
@@ -67,6 +67,8 @@ private void addNamesUsedInClass() {
 		for (ClassNode innerClass : parentClass.getInnerClasses()) {
 			varNames.add(innerClass.getAlias().getShortName());
 		}
+		// add all root package names to avoid collisions with full class names
+		varNames.addAll(mth.root().getCacheStorage().getRootPkgs());
 	}
 
 	public String assignArg(CodeVar var) {

jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java

@@ -19,6 +19,7 @@
 import jadx.core.dex.info.InfoStorage;
 import jadx.core.dex.info.MethodInfo;
 import jadx.core.dex.visitors.typeinference.TypeUpdate;
+import jadx.core.utils.CacheStorage;
 import jadx.core.utils.ErrorsCounter;
 import jadx.core.utils.StringUtils;
 import jadx.core.utils.android.AndroidResourcesUtils;
@@ -36,6 +37,7 @@ public class RootNode {
 	private final StringUtils stringUtils;
 	private final ConstStorage constValues;
 	private final InfoStorage infoStorage = new InfoStorage();
+	private final CacheStorage cacheStorage = new CacheStorage();
 	private final TypeUpdate typeUpdate;
 
 	private ClspGraph clsp;
@@ -222,6 +224,10 @@ public InfoStorage getInfoStorage() {
 		return infoStorage;
 	}
 
+	public CacheStorage getCacheStorage() {
+		return cacheStorage;
+	}
+
 	public JadxArgs getArgs() {
 		return args;
 	}

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

@@ -67,9 +67,7 @@ private void checkClasses(RootNode root, JadxArgs args) {
 				}
 			}
 		}
-		if (args.isRenameValid()) {
-			checkFieldsCollisionWithRootPackage(classes);
-		}
+		processRootPackages(root, classes);
 	}
 
 	private void checkClassName(ClassNode cls, JadxArgs args) {
@@ -140,12 +138,17 @@ private void checkMethods(ClassNode cls, JadxArgs args) {
 		}
 	}
 
-	private void checkFieldsCollisionWithRootPackage(List<ClassNode> classes) {
+	private void processRootPackages(RootNode root, List<ClassNode> classes) {
 		Set<String> rootPkgs = collectRootPkgs(classes);
-		for (ClassNode cls : classes) {
-			for (FieldNode field : cls.getFields()) {
-				if (rootPkgs.contains(field.getAlias())) {
-					deobfuscator.forceRenameField(field);
+		root.getCacheStorage().setRootPkgs(rootPkgs);
+
+		if (root.getArgs().isRenameValid()) {
+			// rename field if collide with any root package
+			for (ClassNode cls : classes) {
+				for (FieldNode field : cls.getFields()) {
+					if (rootPkgs.contains(field.getAlias())) {
+						deobfuscator.forceRenameField(field);
+					}
 				}
 			}
 		}

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

@@ -0,0 +1,17 @@
+package jadx.core.utils;
+
+import java.util.Collections;
+import java.util.Set;
+
+public class CacheStorage {
+
+	private Set<String> rootPkgs = Collections.emptySet();
+
+	public Set<String> getRootPkgs() {
+		return rootPkgs;
+	}
+
+	public void setRootPkgs(Set<String> rootPkgs) {
+		this.rootPkgs = rootPkgs;
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/names/TestLocalVarCollideWithPackage.java

@@ -0,0 +1,71 @@
+package jadx.tests.integration.names;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+
+public class TestLocalVarCollideWithPackage extends SmaliTest {
+	//@formatter:off
+	/*
+		-----------------------------------------------------------
+		package first;
+
+		import pkg.Second;
+
+		public class A {
+			public String test() {
+				Second second = new Second();
+				second.A.call(); // collision
+				return second.str;
+			}
+		}
+		-----------------------------------------------------------
+		package pkg;
+
+		public class Second {
+			public String str;
+		}
+		-----------------------------------------------------------
+		package second;
+
+		public class A {
+		}
+		-----------------------------------------------------------
+	*/
+	//@formatter:on
+
+	@Test
+	public void test() {
+		List<ClassNode> clsList = loadFromSmaliFiles();
+		ClassNode firstA = searchCls(clsList, "first.A");
+		String code = firstA.getCode().toString();
+
+		assertThat(code, containsString("second.A.call();"));
+		assertThat(code, not(containsString("Second second = new Second();")));
+	}
+
+	@Test
+	public void testNoDebug() {
+		noDebugInfo();
+		loadFromSmaliFiles();
+	}
+
+	@Test
+	public void testWithoutImports() {
+		getArgs().setUseImports(false);
+		loadFromSmaliFiles();
+	}
+
+	@Test
+	public void testWithDeobfuscation() {
+		enableDeobfuscation();
+		loadFromSmaliFiles();
+	}
+}

jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/1.smali

@@ -0,0 +1,18 @@
+.class public Lfirst/A;
+.super Ljava/lang/Object;
+
+.method public test()Ljava/lang/String;
+    .registers 2
+
+    new-instance v1, Lpkg/Second;
+
+    invoke-direct {v1}, Lpkg/Second;-><init>()V
+
+    .local v1, "second":Lpkg/Second;
+
+    invoke-static {}, Lsecond/A;->call()Ljava/lang/String;
+
+    iget-object v0, v1, Lpkg/Second;->str:Ljava/lang/String;
+
+    return-object v0
+.end method

jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/2.smali

@@ -0,0 +1,10 @@
+.class public Lsecond/A;
+.super Ljava/lang/Object;
+
+.method static public call()Ljava/lang/String;
+    .registers 1
+
+    const v0, 0
+
+    return-object v0
+.end method

jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/3.smali

@@ -0,0 +1,4 @@
+.class public Lpkg/Second;
+.super Ljava/lang/Object;
+
+.field public str:Ljava/lang/String;