fix: force rename fields and methods with reserved names (#364)

.gitignore

@@ -9,6 +9,7 @@ out/
 *.iml
 *.ipr
 *.iws
+.attach_pid*
 
 **/.DS_Store
 

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

@@ -78,7 +78,7 @@ public class MethodGen {
 			ai = ai.remove(AccessFlags.ACC_PUBLIC);
 		}
 
-		if (mth.getMethodInfo().isRenamed()) {
+		if (mth.getMethodInfo().isRenamed() && !ai.isConstructor()) {
 			code.startLine("/* renamed from: ").add(mth.getName()).add(" */");
 		}
 		code.startLineWithNum(mth.getSourceLine());
@@ -87,7 +87,7 @@ public class MethodGen {
 		if (classGen.addGenericMap(code, mth.getGenericMap())) {
 			code.add(' ');
 		}
-		if (mth.getAccessFlags().isConstructor()) {
+		if (ai.isConstructor()) {
 			code.attachDefinition(mth);
 			code.add(classGen.getClassNode().getShortName()); // constructor
 		} else {

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

@@ -129,7 +129,7 @@ public class Deobfuscator {
 			for (MethodInfo mth : o.getMethods()) {
 				if (aliasToUse == null) {
 					if (mth.isRenamed() && !mth.isAliasFromPreset()) {
-						mth.setAlias(String.format("mo%d%s", id, makeName(mth.getName())));
+						mth.setAlias(String.format("mo%d%s", id, prepareNamePart(mth.getName())));
 					}
 					aliasToUse = mth.getAlias();
 				}
@@ -350,7 +350,7 @@ public class Deobfuscator {
 
 		if (alias == null) {
 			String clsName = classInfo.getShortName();
-			alias = String.format("C%04d%s", clsIndex++, makeName(clsName));
+			alias = String.format("C%04d%s", clsIndex++, prepareNamePart(clsName));
 		}
 		PackageNode pkg = getPackageNode(classInfo.getPackage(), true);
 		clsMap.put(classInfo, new DeobfClsInfo(this, cls, pkg, alias));
@@ -426,13 +426,13 @@ public class Deobfuscator {
 	}
 
 	public String makeFieldAlias(FieldNode field) {
-		String alias = String.format("f%d%s", fldIndex++, makeName(field.getName()));
+		String alias = String.format("f%d%s", fldIndex++, prepareNamePart(field.getName()));
 		fldMap.put(field.getFieldInfo(), alias);
 		return alias;
 	}
 
 	public String makeMethodAlias(MethodNode mth) {
-		String alias = String.format("m%d%s", mthIndex++, makeName(mth.getName()));
+		String alias = String.format("m%d%s", mthIndex++, prepareNamePart(mth.getName()));
 		mthMap.put(mth.getMethodInfo(), alias);
 		return alias;
 	}
@@ -454,25 +454,21 @@ public class Deobfuscator {
 
 		String pkgName = pkg.getName();
 		if (!pkg.hasAlias() && shouldRename(pkgName)) {
-			String pkgAlias = String.format("p%03d%s", pkgIndex++, makeName(pkgName));
+			String pkgAlias = String.format("p%03d%s", pkgIndex++, prepareNamePart(pkgName));
 			pkg.setAlias(pkgAlias);
 		}
 	}
 
 	private boolean shouldRename(String s) {
-		return s.length() > maxLength
-				|| s.length() < minLength
-				|| NameMapper.isReserved(s)
-				|| !NameMapper.isAllCharsPrintable(s);
+		int len = s.length();
+		return len < minLength || len > maxLength
+				|| !NameMapper.isValidIdentifier(s);
 	}
 
-	private String makeName(String name) {
+	private String prepareNamePart(String name) {
 		if (name.length() > maxLength) {
 			return "x" + Integer.toHexString(name.hashCode());
 		}
-		if (NameMapper.isReserved(name)) {
-			return name;
-		}
 		if (!NameMapper.isAllCharsPrintable(name)) {
 			return removeInvalidChars(name);
 		}

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

@@ -79,12 +79,14 @@ public class NameMapper {
 
 	public static boolean isValidIdentifier(String str) {
 		return notEmpty(str)
+				&& !isReserved(str)
 				&& VALID_JAVA_IDENTIFIER.matcher(str).matches()
 				&& isAllCharsPrintable(str);
 	}
 
 	public static boolean isValidFullIdentifier(String str) {
 		return notEmpty(str)
+				&& !isReserved(str)
 				&& VALID_JAVA_FULL_IDENTIFIER.matcher(str).matches()
 				&& isAllCharsPrintable(str);
 	}

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

@@ -10,6 +10,7 @@ import org.apache.commons.io.FilenameUtils;
 import jadx.api.JadxArgs;
 import jadx.core.Consts;
 import jadx.core.deobf.Deobfuscator;
+import jadx.core.deobf.NameMapper;
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.info.ClassInfo;
 import jadx.core.dex.info.FieldInfo;
@@ -100,7 +101,8 @@ public class RenameVisitor extends AbstractVisitor {
 		Set<String> names = new HashSet<>();
 		for (FieldNode field : cls.getFields()) {
 			FieldInfo fieldInfo = field.getFieldInfo();
-			if (!names.add(fieldInfo.getAlias())) {
+			String fieldName = fieldInfo.getAlias();
+			if (!names.add(fieldName) || !NameMapper.isValidIdentifier(fieldName)) {
 				deobfuscator.renameField(field);
 			}
 		}
@@ -109,11 +111,11 @@ public class RenameVisitor extends AbstractVisitor {
 	private void checkMethods(ClassNode cls) {
 		Set<String> names = new HashSet<>();
 		for (MethodNode mth : cls.getMethods()) {
-			if (mth.contains(AFlag.DONT_GENERATE)) {
+			if (mth.contains(AFlag.DONT_GENERATE) || mth.getAccessFlags().isConstructor()) {
 				continue;
 			}
 			String signature = mth.getMethodInfo().makeSignature(false);
-			if (!names.add(signature)) {
+			if (!names.add(signature) || !NameMapper.isValidIdentifier(mth.getAlias())) {
 				deobfuscator.renameMethod(mth);
 			}
 		}

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

@@ -27,6 +27,7 @@ import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.nodes.RootNode;
 import jadx.core.dex.visitors.DepthTraversal;
 import jadx.core.dex.visitors.IDexTreeVisitor;
+import jadx.core.utils.exceptions.JadxException;
 import jadx.core.xmlgen.ResourceStorage;
 import jadx.core.xmlgen.entry.ResourceEntry;
 import jadx.tests.api.compiler.DynamicCompiler;
@@ -137,13 +138,13 @@ public abstract class IntegrationTest extends TestUtils {
 	}
 
 	protected void decompile(JadxDecompiler jadx, ClassNode cls) {
-		List<IDexTreeVisitor> passes = Jadx.getPassesList(jadx.getArgs());
+		List<IDexTreeVisitor> passes = getPassesList(jadx);
 		ProcessClass.process(cls, passes, new CodeGen());
 	}
 
-	protected void decompileWithoutUnload(JadxDecompiler d, ClassNode cls) {
+	protected void decompileWithoutUnload(JadxDecompiler jadx, ClassNode cls) {
 		cls.load();
-		List<IDexTreeVisitor> passes = Jadx.getPassesList(d.getArgs());
+		List<IDexTreeVisitor> passes = getPassesList(jadx);
 		for (IDexTreeVisitor visitor : passes) {
 			DepthTraversal.visit(visitor, cls);
 		}
@@ -151,6 +152,20 @@ public abstract class IntegrationTest extends TestUtils {
 		// don't unload class
 	}
 
+	private List<IDexTreeVisitor> getPassesList(JadxDecompiler jadx) {
+		RootNode root = JadxInternalAccess.getRoot(jadx);
+		List<IDexTreeVisitor> passesList = Jadx.getPassesList(jadx.getArgs());
+		passesList.forEach(pass -> {
+			try {
+				pass.init(root);
+			} catch (JadxException e) {
+				e.printStackTrace();
+				fail(e.getMessage());
+			}
+		});
+		return passesList;
+	}
+
 	protected void generateClsCode(ClassNode cls) {
 		try {
 			new CodeGen().visit(cls);

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

@@ -0,0 +1,32 @@
+package jadx.tests.integration.names;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestReservedNames extends SmaliTest {
+	/*
+		public static class TestCls {
+
+			public String do; // reserved name
+			public String 0f; // invalid identifier
+
+			public String try() {
+				return this.do;
+			}
+		}
+	*/
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNodeFromSmaliWithPath("names", "TestReservedNames");
+		String code = cls.getCode().toString();
+
+		assertThat(code, not(containsString("public String do;")));
+	}
+}

jadx-core/src/test/smali/names/TestReservedNames.smali

@@ -0,0 +1,32 @@
+.class public LTestReservedNames;
+.super Ljava/lang/Object;
+.source "TestReservedNames.java"
+
+
+# instance fields
+.field public do:Ljava/lang/String; # reserved name
+.field public 0f:Ljava/lang/String; # invalid identifier
+
+
+# direct methods
+.method public constructor <init>()V
+    .registers 1
+
+    .prologue
+    .line 3
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+
+    return-void
+.end method
+
+
+# virtual methods
+.method public try()Ljava/lang/String;
+    .registers 2
+
+    .prologue
+    .line 8
+    iget-object v0, p0, LTestReservedNames;->do:Ljava/lang/String;
+
+    return-object v0
+.end method