fix: rename parameters in annotations (#504)

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

@@ -5,6 +5,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
+import org.jetbrains.annotations.Nullable;
+
 import jadx.core.Consts;
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.IAttributeNode;
@@ -72,27 +74,45 @@ public class AnnotationGen {
 
 	private void formatAnnotation(CodeWriter code, Annotation a) {
 		code.add('@');
-		classGen.useType(code, a.getType());
+		ClassNode annCls = cls.dex().resolveClass(a.getType());
+		if (annCls != null) {
+			classGen.useClass(code, annCls);
+		} else {
+			classGen.useType(code, a.getType());
+		}
+
 		Map<String, Object> vl = a.getValues();
 		if (!vl.isEmpty()) {
 			code.add('(');
-			if (vl.size() == 1 && vl.containsKey("value")) {
-				encodeValue(code, vl.get("value"));
-			} else {
-				for (Iterator<Entry<String, Object>> it = vl.entrySet().iterator(); it.hasNext(); ) {
-					Entry<String, Object> e = it.next();
-					code.add(e.getKey());
+			for (Iterator<Entry<String, Object>> it = vl.entrySet().iterator(); it.hasNext(); ) {
+				Entry<String, Object> e = it.next();
+				String paramName = getParamName(annCls, e.getKey());
+				if (paramName.equals("value") && vl.size() == 1) {
+					// don't add "value = " if no other parameters
+				} else {
+					code.add(paramName);
 					code.add(" = ");
-					encodeValue(code, e.getValue());
-					if (it.hasNext()) {
-						code.add(", ");
-					}
+				}
+				encodeValue(code, e.getValue());
+				if (it.hasNext()) {
+					code.add(", ");
 				}
 			}
 			code.add(')');
 		}
 	}
 
+	private String getParamName(@Nullable ClassNode annCls, String paramName) {
+		if (annCls != null) {
+			// TODO: save value type and search using signature
+			MethodNode mth = annCls.searchMethodByShortName(paramName);
+			if (mth != null) {
+				return mth.getAlias();
+			}
+		}
+		return paramName;
+	}
+
 	@SuppressWarnings("unchecked")
 	public void addThrows(MethodNode mth, CodeWriter code) {
 		Annotation an = mth.getAnnotation(Consts.DALVIK_THROWS);

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

@@ -490,10 +490,20 @@ public class ClassGen {
 	public void useClass(CodeWriter code, ClassInfo classInfo) {
 		ClassNode classNode = cls.dex().resolveClass(classInfo);
 		if (classNode != null) {
-			code.attachAnnotation(classNode);
+			useClass(code, classNode);
+		} else {
+			addClsName(code, classInfo);
 		}
-		String baseClass = useClassInternal(cls.getAlias(), classInfo.getAlias());
-		code.add(baseClass);
+	}
+
+	public void useClass(CodeWriter code, ClassNode classNode) {
+		code.attachAnnotation(classNode);
+		addClsName(code, classNode.getClassInfo());
+	}
+
+	private void addClsName(CodeWriter code, ClassInfo classInfo) {
+		String clsName = useClassInternal(cls.getAlias(), classInfo.getAlias());
+		code.add(clsName);
 	}
 
 	private String useClassInternal(ClassInfo useCls, ClassInfo extClsInfo) {

jadx-core/src/main/java/jadx/core/deobf/PackageNode.java

@@ -127,4 +127,9 @@ public class PackageNode {
 		}
 		return pp;
 	}
+
+	@Override
+	public String toString() {
+		return packageAlias;
+	}
 }

jadx-core/src/main/java/jadx/core/dex/info/ClassInfo.java

@@ -63,12 +63,16 @@ public final class ClassInfo implements Comparable<ClassInfo> {
 	}
 
 	public void rename(RootNode root, String fullName) {
-		ClassInfo newAlias = new ClassInfo(root, ArgType.object(fullName), isInner());
+		ArgType clsType = ArgType.object(fullName);
+		ClassInfo newAlias = root.getInfoStorage().getCls(clsType);
+		if (newAlias == null) {
+			newAlias = new ClassInfo(root, clsType, isInner());
+			root.getInfoStorage().putCls(newAlias);
+		}
 		if (!alias.getFullName().equals(newAlias.getFullName())) {
 			this.alias = newAlias;
 		}
 	}
-
 	public boolean isRenamed() {
 		return alias != this;
 	}
@@ -171,6 +175,10 @@ public final class ClassInfo implements Comparable<ClassInfo> {
 		splitNames(root, false);
 	}
 
+	public void updateNames(RootNode root) {
+		splitNames(root, isInner());
+	}
+
 	public ArgType getType() {
 		return type;
 	}

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

@@ -344,7 +344,7 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode {
 		return mthInfoMap.get(mth);
 	}
 
-	public MethodNode searchMethodByName(String shortId) {
+	public MethodNode searchMethodByShortId(String shortId) {
 		for (MethodNode m : methods) {
 			if (m.getMethodInfo().getShortId().equals(shortId)) {
 				return m;
@@ -353,8 +353,22 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode {
 		return null;
 	}
 
+	/**
+	 * Return first method by original short name
+	 * Note: methods are not unique by name (class can have several methods with same name but different signature)
+	 */
+	@Nullable
+	public MethodNode searchMethodByShortName(String name) {
+		for (MethodNode m : methods) {
+			if (m.getMethodInfo().getName().equals(name)) {
+				return m;
+			}
+		}
+		return null;
+	}
+
 	public MethodNode searchMethodById(int id) {
-		return searchMethodByName(MethodInfo.fromDex(dex, id).getShortId());
+		return searchMethodByShortId(MethodInfo.fromDex(dex, id).getShortId());
 	}
 
 	public ClassNode getParentClass() {
@@ -420,7 +434,7 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode {
 
 	@Nullable
 	public MethodNode getClassInitMth() {
-		return searchMethodByName("<clinit>()V");
+		return searchMethodByShortId("<clinit>()V");
 	}
 
 	@Nullable

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

@@ -2,6 +2,7 @@ package jadx.core.dex.nodes;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -48,6 +49,8 @@ public class DexNode implements IDexNode {
 		for (ClassDef cls : dexBuf.classDefs()) {
 			addClassNode(new ClassNode(this, cls));
 		}
+		// sort classes by name, expect top classes before inner
+		classes.sort(Comparator.comparing(ClassNode::getFullName));
 	}
 
 	public void addClassNode(ClassNode clsNode) {
@@ -63,6 +66,7 @@ public class DexNode implements IDexNode {
 				inner.add(cls);
 			}
 		}
+		List<ClassNode> updated = new ArrayList<>();
 		for (ClassNode cls : inner) {
 			ClassInfo clsInfo = cls.getClassInfo();
 			ClassNode parent = resolveClass(clsInfo.getParentClass());
@@ -70,10 +74,17 @@ public class DexNode implements IDexNode {
 				clsMap.remove(clsInfo);
 				clsInfo.notInner(root);
 				clsMap.put(clsInfo, cls);
+				updated.add(cls);
 			} else {
 				parent.addInnerClass(cls);
 			}
 		}
+		// reload names for inner classes of updated parents
+		for (ClassNode updCls : updated) {
+			for (ClassNode innerCls : updCls.getInnerClasses()) {
+				innerCls.getClassInfo().updateNames(root);
+			}
+		}
 	}
 
 	public List<ClassNode> getClasses() {

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

@@ -195,7 +195,7 @@ public class ModVisitor extends AbstractVisitor {
 		if (co.isSuper() && (co.getArgsCount() == 0 || parentClass.isEnum())) {
 			remove = true;
 		} else if (co.isThis() && co.getArgsCount() == 0) {
-			MethodNode defCo = parentClass.searchMethodByName(callMth.getShortId());
+			MethodNode defCo = parentClass.searchMethodByShortId(callMth.getShortId());
 			if (defCo == null || defCo.isNoCode()) {
 				// default constructor not implemented
 				remove = true;
@@ -347,7 +347,7 @@ public class ModVisitor extends AbstractVisitor {
 		}
 		boolean passThis = co.getArgsCount() >= 1 && co.getArg(0).isThis();
 		String ctrId = "<init>(" + (passThis ? TypeGen.signature(co.getArg(0).getType()) : "") + ")V";
-		MethodNode defCtr = classNode.searchMethodByName(ctrId);
+		MethodNode defCtr = classNode.searchMethodByShortId(ctrId);
 		if (defCtr == null) {
 			return null;
 		}

jadx-core/src/test/java/jadx/tests/integration/annotations/TestAnnotationsRename.java

+package jadx.tests.integration.annotations;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.lang.reflect.Method;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+public class TestAnnotationsRename extends IntegrationTest {
+
+	public static class TestCls {
+
+		@Target({ElementType.METHOD})
+		@Retention(RetentionPolicy.RUNTIME)
+		public @interface A {
+			int x();
+		}
+
+		@A(x = 5)
+		void test() {
+		}
+
+		public void check() throws NoSuchMethodException {
+			Method test = TestCls.class.getDeclaredMethod("test");
+			A annotation = test.getAnnotation(A.class);
+			assertThat(annotation.x(), is(5));
+		}
+	}
+
+	@Test
+	public void test() {
+		enableDeobfuscation();
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsString("public @interface "));
+		assertThat(code, not(containsString("(x = 5)")));
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/annotations/AnnotationsRenaming.java → jadx-core/src/test/java/jadx/tests/integration/annotations/TestAnnotationsRenameDef.java

@@ -7,7 +7,6 @@ import java.lang.annotation.Target;
 
 import org.junit.jupiter.api.Test;
 
-import jadx.NotYetImplemented;
 import jadx.core.dex.nodes.ClassNode;
 import jadx.tests.api.IntegrationTest;
 
@@ -15,30 +14,32 @@ import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 
-public class AnnotationsRenaming extends IntegrationTest {
+public class TestAnnotationsRenameDef extends IntegrationTest {
 
 	public static class TestCls {
 
 		@Target({ElementType.METHOD})
 		@Retention(RetentionPolicy.RUNTIME)
-		public static @interface A {
-			int x();
+		public @interface A {
+			int value();
 		}
 
-		@A(x = 5)
+		@A(5)
 		void test() {
 		}
-
 	}
 
 	@Test
-	@NotYetImplemented
-	public void test504() {
+	public void test() {
 		enableDeobfuscation();
+		// force rename 'value' method
+		args.setDeobfuscationMinLength(20);
+
 		ClassNode cls = getClassNode(TestCls.class);
 		String code = cls.getCode().toString();
 
-		assertThat(code, containsString("public static @interface "));
-		assertThat(code, not(containsString("(x = 5)")));
+		assertThat(code, containsString("public @interface "));
+		assertThat(code, not(containsString("int value();")));
+		assertThat(code, not(containsString("(5)")));
 	}
 }

jadx-core/src/test/java/jadx/tests/integration/debuginfo/TestLineNumbers.java

@@ -46,11 +46,11 @@ public class TestLineNumbers extends IntegrationTest {
 		String code = cls.getCode().toString();
 
 		FieldNode field = cls.searchFieldByName("field");
-		MethodNode func = cls.searchMethodByName("func()V");
+		MethodNode func = cls.searchMethodByShortId("func()V");
 		ClassNode inner = cls.getInnerClasses().get(0);
-		MethodNode innerFunc = inner.searchMethodByName("innerFunc()V");
-		MethodNode innerFunc2 = inner.searchMethodByName("innerFunc2()V");
-		MethodNode innerFunc3 = inner.searchMethodByName("innerFunc3()V");
+		MethodNode innerFunc = inner.searchMethodByShortId("innerFunc()V");
+		MethodNode innerFunc2 = inner.searchMethodByShortId("innerFunc2()V");
+		MethodNode innerFunc3 = inner.searchMethodByShortId("innerFunc3()V");
 		FieldNode innerField = inner.searchFieldByName("innerField");
 
 		// check source lines (available only for instructions and methods)

jadx-core/src/test/java/jadx/tests/integration/debuginfo/TestReturnSourceLine.java

 package jadx.tests.integration.debuginfo;
 
-import static jadx.tests.api.utils.JadxMatchers.containsOne;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-
 import org.junit.jupiter.api.Test;
 
 import jadx.NotYetImplemented;
@@ -14,6 +9,11 @@ import jadx.core.dex.nodes.ClassNode;
 import jadx.core.dex.nodes.MethodNode;
 import jadx.tests.api.IntegrationTest;
 
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
 public class TestReturnSourceLine extends IntegrationTest {
 
 	public static class TestCls {
@@ -55,10 +55,10 @@ public class TestReturnSourceLine extends IntegrationTest {
 		String code = codeWriter.toString();
 		String[] lines = code.split(CodeWriter.NL);
 
-		MethodNode test1 = cls.searchMethodByName("test1(Z)I");
+		MethodNode test1 = cls.searchMethodByShortId("test1(Z)I");
 		checkLine(lines, codeWriter, test1, 3, "return 1;");
 
-		MethodNode test2 = cls.searchMethodByName("test2(I)I");
+		MethodNode test2 = cls.searchMethodByShortId("test2(I)I");
 		checkLine(lines, codeWriter, test2, 3, "return v - 1;");
 	}
 
@@ -70,7 +70,7 @@ public class TestReturnSourceLine extends IntegrationTest {
 		String code = codeWriter.toString();
 		String[] lines = code.split(CodeWriter.NL);
 
-		MethodNode test3 = cls.searchMethodByName("test3(I)I");
+		MethodNode test3 = cls.searchMethodByShortId("test3(I)I");
 		checkLine(lines, codeWriter, test3, 3, "return v;");
 	}
 

jadx-core/src/test/java/jadx/tests/integration/deobf/TestMthRename.java

@@ -30,10 +30,7 @@ public class TestMthRename extends IntegrationTest {
 		ClassNode cls = getClassNode(TestCls.class);
 		String code = cls.getCode().toString();
 
-		assertThat(code, containsString("public abstract void mo1a();"));
 		assertThat(code, not(containsString("public abstract void a();")));
-
-		assertThat(code, containsString(".mo1a();"));
 		assertThat(code, not(containsString(".a();")));
 	}
 }