From 01da127c4eff25c44731fb53c068808676275a02 Mon Sep 17 00:00:00 2001 From: Ahmed Ashour Date: Sun, 14 Apr 2019 10:39:27 +0200 Subject: [PATCH] fix: remove generics cast when object types match (#591) (PR #592) --- .../main/java/jadx/core/codegen/InsnGen.java | 17 ++++++---- .../core/dex/instructions/InsnDecoder.java | 16 ++++++--- .../core/dex/instructions/args/ArgType.java | 2 +- .../visitors/regions/LoopRegionVisitor.java | 2 +- .../typeinference/TypeCompareTest.java | 2 +- .../tests/functional/SignatureParserTest.java | 16 ++++----- .../integration/generics/TestGenerics7.java | 34 +++++++++++++++++++ .../invoke/TestCastInOverloadedInvoke.java | 2 +- 8 files changed, 68 insertions(+), 23 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/generics/TestGenerics7.java diff --git a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java index 7d8672a0a..a6a9e3ec3 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java @@ -740,13 +740,18 @@ public class InsnGen { return false; } } - if (!arg.getType().equals(origType)) { - code.add('('); - useType(code, origType); - code.add(") "); - return true; + ArgType argType = arg.getType(); + if (argType.equals(origType)) { + return false; } - return false; + if (origType.isGeneric() + && origType.getObject().equals(argType.getObject())) { + return false; + } + code.add('('); + useType(code, origType); + code.add(") "); + return true; } /** diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/InsnDecoder.java b/jadx-core/src/main/java/jadx/core/dex/instructions/InsnDecoder.java index 51d0b568e..476ed9a1d 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/InsnDecoder.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/InsnDecoder.java @@ -2,6 +2,10 @@ package jadx.core.dex.instructions; import java.io.EOFException; +import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.android.dex.Code; import com.android.dx.io.OpcodeInfo; import com.android.dx.io.Opcodes; @@ -10,10 +14,8 @@ import com.android.dx.io.instructions.FillArrayDataPayloadDecodedInstruction; import com.android.dx.io.instructions.PackedSwitchPayloadDecodedInstruction; import com.android.dx.io.instructions.ShortArrayCodeInput; import com.android.dx.io.instructions.SparseSwitchPayloadDecodedInstruction; -import org.jetbrains.annotations.NotNull; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import jadx.core.Consts; import jadx.core.dex.info.FieldInfo; import jadx.core.dex.info.MethodInfo; import jadx.core.dex.instructions.args.ArgType; @@ -119,9 +121,13 @@ public class InsnDecoder { return constStrInsn; case Opcodes.CONST_CLASS: - InsnNode constClsInsn = new ConstClassNode(dex.getType(insn.getIndex())); - constClsInsn.setResult(InsnArg.reg(insn, 0, ArgType.CLASS)); + { + ArgType clsType = dex.getType(insn.getIndex()); + InsnNode constClsInsn = new ConstClassNode(clsType); + constClsInsn.setResult( + InsnArg.reg(insn, 0, ArgType.generic(Consts.CLASS_CLASS, clsType))); return constClsInsn; + } case Opcodes.MOVE: case Opcodes.MOVE_16: diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java index 7809c079f..dc11143d9 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java @@ -83,7 +83,7 @@ public abstract class ArgType { return new SignatureParser(sign).consumeType(); } - public static ArgType generic(String obj, ArgType[] generics) { + public static ArgType generic(String obj, ArgType... generics) { return new GenericObject(obj, generics); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/LoopRegionVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/LoopRegionVisitor.java index 53b2eb2da..c40b38fcc 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/LoopRegionVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/LoopRegionVisitor.java @@ -326,7 +326,7 @@ public class LoopRegionVisitor extends AbstractVisitor implements IRegionVisitor return true; } // TODO: add checks - iterableType = ArgType.generic(iterableType.getObject(), new ArgType[]{varType}); + iterableType = ArgType.generic(iterableType.getObject(), varType); iterableArg.setType(iterableType); return true; } diff --git a/jadx-core/src/test/java/jadx/core/dex/visitors/typeinference/TypeCompareTest.java b/jadx-core/src/test/java/jadx/core/dex/visitors/typeinference/TypeCompareTest.java index 53d1a016c..f96c01b69 100644 --- a/jadx-core/src/test/java/jadx/core/dex/visitors/typeinference/TypeCompareTest.java +++ b/jadx-core/src/test/java/jadx/core/dex/visitors/typeinference/TypeCompareTest.java @@ -76,7 +76,7 @@ public class TypeCompareTest { ArgType keyType = ArgType.genericType("K"); ArgType valueType = ArgType.genericType("V"); - ArgType mapGeneric = ArgType.generic(mapCls.getObject(), new ArgType[]{keyType, valueType}); + ArgType mapGeneric = ArgType.generic(mapCls.getObject(), keyType, valueType); check(mapGeneric, mapCls, TypeCompareEnum.NARROW_BY_GENERIC); check(mapCls, mapGeneric, TypeCompareEnum.WIDER_BY_GENERIC); diff --git a/jadx-core/src/test/java/jadx/tests/functional/SignatureParserTest.java b/jadx-core/src/test/java/jadx/tests/functional/SignatureParserTest.java index 34d1c1271..3871741e0 100644 --- a/jadx-core/src/test/java/jadx/tests/functional/SignatureParserTest.java +++ b/jadx-core/src/test/java/jadx/tests/functional/SignatureParserTest.java @@ -43,12 +43,12 @@ class SignatureParserTest { @Test public void testGenerics() { checkType("TD;", genericType("D")); - checkType("La;", generic("La;", new ArgType[]{genericType("V"), object("b")})); - checkType("La;>;", generic("La;", new ArgType[]{generic("Lb;", new ArgType[]{object("Lc;")})})); - checkType("La/b/C;>;", generic("La/b/C;", new ArgType[]{generic("Ld/E;", new ArgType[]{object("Lf/G;")})})); - checkType("La.c;", genericInner(generic("La;", new ArgType[]{genericType("D")}), "c", null)); - checkType("La.c/d;", genericInner(generic("La;", new ArgType[]{genericType("D")}), "c.d", null)); - checkType("La.c;", genericInner(generic("La;", new ArgType[]{object("Lb;")}), "c", new ArgType[]{genericType("V")})); + checkType("La;", generic("La;", genericType("V"), object("b"))); + checkType("La;>;", generic("La;", generic("Lb;", object("Lc;")))); + checkType("La/b/C;>;", generic("La/b/C;", generic("Ld/E;", object("Lf/G;")))); + checkType("La.c;", genericInner(generic("La;", genericType("D")), "c", null)); + checkType("La.c/d;", genericInner(generic("La;", genericType("D")), "c.d", null)); + checkType("La.c;", genericInner(generic("La;", object("Lb;")), "c", new ArgType[]{genericType("V")})); } @Test @@ -107,7 +107,7 @@ class SignatureParserTest { List argTypes = new SignatureParser("(Ljava/util/List<*>;)V").consumeMethodArgs(); assertThat(argTypes, hasSize(1)); - assertThat(argTypes.get(0), is(generic("Ljava/util/List;", new ArgType[]{wildcard()}))); + assertThat(argTypes.get(0), is(generic("Ljava/util/List;", wildcard()))); } @Test @@ -117,7 +117,7 @@ class SignatureParserTest { assertThat(argTypes, hasSize(1)); ArgType argType = argTypes.get(0); assertThat(argType.getObject().indexOf('/'), is(-1)); - assertThat(argType, is(genericInner(generic("La/b/C;", new ArgType[]{genericType("T")}), "d.E", null))); + assertThat(argType, is(genericInner(generic("La/b/C;", genericType("T")), "d.E", (ArgType[]) null))); } @Test diff --git a/jadx-core/src/test/java/jadx/tests/integration/generics/TestGenerics7.java b/jadx-core/src/test/java/jadx/tests/integration/generics/TestGenerics7.java new file mode 100644 index 000000000..38d72baa1 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/generics/TestGenerics7.java @@ -0,0 +1,34 @@ +package jadx.tests.integration.generics; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; + +import org.junit.jupiter.api.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +public class TestGenerics7 extends IntegrationTest { + + public static class TestCls { + + public void test() { + declare(String.class); + } + + public T declare(Class cls) { + return null; + } + + public void declare(Object cls) { + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsString("declare(String.class);")); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/invoke/TestCastInOverloadedInvoke.java b/jadx-core/src/test/java/jadx/tests/integration/invoke/TestCastInOverloadedInvoke.java index 27e740edf..d5b1fbbb8 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/invoke/TestCastInOverloadedInvoke.java +++ b/jadx-core/src/test/java/jadx/tests/integration/invoke/TestCastInOverloadedInvoke.java @@ -55,7 +55,7 @@ public class TestCastInOverloadedInvoke extends IntegrationTest { ClassNode cls = getClassNode(TestCls.class); String code = cls.getCode().toString(); - assertThat(code, containsOne("call((ArrayList) new ArrayList());")); + assertThat(code, containsOne("call(new ArrayList());")); assertThat(code, containsOne("call((List) new ArrayList());")); assertThat(code, containsOne("call((String) obj);"));