From eaeb114258d0e493ee6f1aaf741509fca3005c55 Mon Sep 17 00:00:00 2001 From: Skylot Date: Wed, 15 Jun 2022 18:43:17 +0100 Subject: [PATCH] fix: check class name collisions (#1526) --- .../main/java/jadx/core/codegen/ClassGen.java | 27 +++++++-- .../TestCollisionWithJavaLangClasses.java | 59 +++++++++++++++++++ .../tests/integration/names/pkg2/System.java | 6 ++ .../tests/integration/names/pkg2/TestCls.java | 8 +++ 4 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/TestCollisionWithJavaLangClasses.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/pkg2/System.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/pkg2/TestCls.java diff --git a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java index ff96d878..4356d4ac 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java @@ -614,21 +614,23 @@ public class ClassGen { if (useCls.equals(extClsInfo)) { return shortName; } - if (extClsInfo.getPackage().equals("java.lang") && extClsInfo.getParentClass() == null) { - return shortName; - } if (isClassInnerFor(useCls, extClsInfo)) { return shortName; } if (extClsInfo.isInner()) { return expandInnerClassName(useCls, extClsInfo); } - if (searchCollision(cls.root(), useCls, extClsInfo)) { + if (checkInnerCollision(cls.root(), useCls, extClsInfo) + || checkInPackageCollision(cls.root(), useCls, extClsInfo)) { return fullName; } if (isBothClassesInOneTopClass(useCls, extClsInfo)) { return shortName; } + // don't add import for top classes from 'java.lang' package (subpackages excluded) + if (extClsInfo.getPackage().equals("java.lang") && extClsInfo.getParentClass() == null) { + return shortName; + } // don't add import if this class from same package if (extClsInfo.getPackage().equals(useCls.getPackage()) && !extClsInfo.isInner()) { return shortName; @@ -709,7 +711,7 @@ public class ClassGen { return false; } - private static boolean searchCollision(RootNode root, ClassInfo useCls, ClassInfo searchCls) { + private static boolean checkInnerCollision(RootNode root, @Nullable ClassInfo useCls, ClassInfo searchCls) { if (useCls == null) { return false; } @@ -726,7 +728,20 @@ public class ClassGen { } } } - return searchCollision(root, useCls.getParentClass(), searchCls); + return checkInnerCollision(root, useCls.getParentClass(), searchCls); + } + + /** + * Check if class with same name exists in current package + */ + private static boolean checkInPackageCollision(RootNode root, ClassInfo useCls, ClassInfo searchCls) { + String currentPkg = useCls.getAliasPkg(); + if (currentPkg.equals(searchCls.getAliasPkg())) { + // search class already from current package + return false; + } + String shortName = searchCls.getAliasShortName(); + return root.getClsp().isClsKnown(currentPkg + '.' + shortName); } private void insertRenameInfo(ICodeWriter code, ClassNode cls) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/TestCollisionWithJavaLangClasses.java b/jadx-core/src/test/java/jadx/tests/integration/names/TestCollisionWithJavaLangClasses.java new file mode 100644 index 00000000..da7d7831 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/TestCollisionWithJavaLangClasses.java @@ -0,0 +1,59 @@ +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.IntegrationTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestCollisionWithJavaLangClasses extends IntegrationTest { + + public static class TestCls1 { + public static class System { + public static void main(String[] args) { + java.lang.System.out.println("Hello world"); + } + } + } + + @Test + public void test1() { + assertThat(getClassNode(TestCls1.class)) + .code() + .containsOne("java.lang.System.out.println"); + } + + public static class TestCls2 { + public void doSomething() { + System.doSomething(); + java.lang.System.out.println("Hello World"); + } + + public static class System { + public static void doSomething() { + } + } + } + + @Test + public void test2() { + assertThat(getClassNode(TestCls2.class)) + .code() + .containsLine(2, "System.doSomething();") + .containsOne("java.lang.System.out.println"); + } + + @Test + public void test3() { + List classes = getClassNodes( + jadx.tests.integration.names.pkg2.System.class, + jadx.tests.integration.names.pkg2.TestCls.class); + assertThat(searchCls(classes, "TestCls")) + .code() + .containsLine(2, "System.doSomething();") + .containsOne("java.lang.System.out.println"); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/pkg2/System.java b/jadx-core/src/test/java/jadx/tests/integration/names/pkg2/System.java new file mode 100644 index 00000000..84c06765 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/pkg2/System.java @@ -0,0 +1,6 @@ +package jadx.tests.integration.names.pkg2; + +public class System { + public static void doSomething() { + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/pkg2/TestCls.java b/jadx-core/src/test/java/jadx/tests/integration/names/pkg2/TestCls.java new file mode 100644 index 00000000..a47962a6 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/pkg2/TestCls.java @@ -0,0 +1,8 @@ +package jadx.tests.integration.names.pkg2; + +public class TestCls { + public void doSomething() { + System.doSomething(); + java.lang.System.out.println("Hello World"); + } +} -- GitLab