fix: restore fields order if init use other fields (#1235)

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

@@ -2,8 +2,10 @@ package jadx.core.dex.visitors;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -269,9 +271,70 @@ public class ExtractFieldInit extends AbstractVisitor {
 		return Objects.equals(exclude, Boolean.TRUE);
 	}
 
-	private static void fixFieldsOrder(ClassNode cls, List<FieldInitInfo> fieldsInit) {
+	private static void fixFieldsOrder(ClassNode cls, List<FieldInitInfo> inits) {
+		List<FieldNode> orderedFields = processFieldsDependencies(cls, inits);
+		applyFieldsOrder(cls, orderedFields);
+	}
+
+	private static List<FieldNode> processFieldsDependencies(ClassNode cls, List<FieldInitInfo> inits) {
+		List<FieldNode> orderedFields = Utils.collectionMap(inits, v -> v.fieldNode);
+		// collect dependant fields
+		Map<FieldNode, List<FieldNode>> deps = new HashMap<>(inits.size());
+		for (FieldInitInfo initInfo : inits) {
+			IndexInsnNode insn = initInfo.putInsn;
+			boolean staticField = insn.getType() == InsnType.SPUT;
+			InsnType useType = staticField ? InsnType.SGET : InsnType.IGET;
+			insn.visitInsns(subInsn -> {
+				if (subInsn.getType() == useType) {
+					FieldInfo fieldInfo = (FieldInfo) ((IndexInsnNode) subInsn).getIndex();
+					if (fieldInfo.getDeclClass().equals(cls.getClassInfo())) {
+						FieldNode depField = cls.searchField(fieldInfo);
+						if (depField != null) {
+							deps.computeIfAbsent(initInfo.fieldNode, k -> new ArrayList<>())
+									.add(depField);
+						}
+					}
+				}
+			});
+		}
+		if (deps.isEmpty()) {
+			return orderedFields;
+		}
+		// build new list with deps fields before usage field
+		List<FieldNode> result = new ArrayList<>();
+		for (FieldNode field : orderedFields) {
+			int idx = result.indexOf(field);
+			List<FieldNode> fieldDeps = deps.get(field);
+			if (fieldDeps == null) {
+				if (idx == -1) {
+					result.add(field);
+				}
+				continue;
+			}
+			if (idx == -1) {
+				for (FieldNode depField : fieldDeps) {
+					if (!result.contains(depField)) {
+						result.add(depField);
+					}
+				}
+				result.add(field);
+				continue;
+			}
+			for (FieldNode depField : fieldDeps) {
+				int depIdx = result.indexOf(depField);
+				if (depIdx == -1) {
+					result.add(idx, depField);
+				} else if (depIdx > idx) {
+					result.remove(depIdx);
+					result.add(idx, depField);
+				}
+			}
+		}
+		return result;
+	}
+
+	private static void applyFieldsOrder(ClassNode cls, List<FieldNode> orderedFields) {
 		List<FieldNode> clsFields = cls.getFields();
-		List<FieldNode> orderedFields = Utils.collectionMap(fieldsInit, v -> v.fieldNode);
 		// check if already ordered
 		boolean ordered = Collections.indexOfSubList(clsFields, orderedFields) != -1;
 		if (!ordered) {

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

@@ -327,40 +327,12 @@ public abstract class IntegrationTest extends TestUtils {
 		String clsName = cls.getClassInfo().getFullName();
 		try {
 			// run 'check' method from original class
-			Class<?> origCls;
-			try {
-				origCls = Class.forName(clsName);
-			} catch (ClassNotFoundException e) {
-				// ignore
+			if (runSourceAutoCheck(clsName)) {
 				return;
 			}
-			Method checkMth;
-			try {
-				checkMth = origCls.getMethod(CHECK_METHOD_NAME);
-			} catch (NoSuchMethodException e) {
-				// ignore
-				return;
-			}
-			if (!checkMth.getReturnType().equals(void.class)
-					|| !Modifier.isPublic(checkMth.getModifiers())
-					|| Modifier.isStatic(checkMth.getModifiers())) {
-				fail("Wrong 'check' method");
-				return;
-			}
-			try {
-				limitExecTime(() -> checkMth.invoke(origCls.getConstructor().newInstance()));
-				System.out.println("Source check: PASSED");
-			} catch (Throwable e) {
-				throw new JadxRuntimeException("Source check failed", e);
-			}
 			// run 'check' method from decompiled class
 			if (compile) {
-				try {
-					limitExecTime(() -> invoke(cls, "check"));
-					System.out.println("Decompiled check: PASSED");
-				} catch (Throwable e) {
-					throw new JadxRuntimeException("Decompiled check failed", e);
-				}
+				runDecompiledAutoCheck(cls);
 			}
 		} catch (Exception e) {
 			e.printStackTrace();
@@ -368,6 +340,45 @@ public abstract class IntegrationTest extends TestUtils {
 		}
 	}
 
+	private boolean runSourceAutoCheck(String clsName) {
+		Class<?> origCls;
+		try {
+			origCls = Class.forName(clsName);
+		} catch (ClassNotFoundException e) {
+			// ignore
+			return true;
+		}
+		Method checkMth;
+		try {
+			checkMth = origCls.getMethod(CHECK_METHOD_NAME);
+		} catch (NoSuchMethodException e) {
+			// ignore
+			return true;
+		}
+		if (!checkMth.getReturnType().equals(void.class)
+				|| !Modifier.isPublic(checkMth.getModifiers())
+				|| Modifier.isStatic(checkMth.getModifiers())) {
+			fail("Wrong 'check' method");
+			return true;
+		}
+		try {
+			limitExecTime(() -> checkMth.invoke(origCls.getConstructor().newInstance()));
+			System.out.println("Source check: PASSED");
+		} catch (Throwable e) {
+			throw new JadxRuntimeException("Source check failed", e);
+		}
+		return false;
+	}
+
+	public void runDecompiledAutoCheck(ClassNode cls) {
+		try {
+			limitExecTime(() -> invoke(cls, "check"));
+			System.out.println("Decompiled check: PASSED");
+		} catch (Throwable e) {
+			throw new JadxRuntimeException("Decompiled check failed", e);
+		}
+	}
+
 	private <T> T limitExecTime(Callable<T> call) {
 		ExecutorService executor = Executors.newSingleThreadExecutor();
 		Future<T> future = executor.submit(call);
@@ -406,10 +417,6 @@ public abstract class IntegrationTest extends TestUtils {
 		return null;
 	}
 
-	void compile(ClassNode cls) {
-		compile(Collections.singletonList(cls));
-	}
-
 	void compile(List<ClassNode> clsList) {
 		if (!compile) {
 			return;

jadx-core/src/test/java/jadx/tests/api/utils/assertj/JadxClassNodeAssertions.java

@@ -42,4 +42,14 @@ public class JadxClassNodeAssertions extends AbstractObjectAssert<JadxClassNodeA
 		testInstance.runChecks(actual);
 		return codeAssertions;
 	}
+
+	/**
+	 * Force running auto check on decompiled code.
+	 * Useful for smali tests.
+	 */
+	public JadxClassNodeAssertions runDecompiledAutoCheck(IntegrationTest testInstance) {
+		isNotNull();
+		testInstance.runDecompiledAutoCheck(actual);
+		return this;
+	}
 }

jadx-core/src/test/java/jadx/tests/integration/others/TestFieldInitOrder2.java

+package jadx.tests.integration.others;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
+
+public class TestFieldInitOrder2 extends SmaliTest {
+
+	@SuppressWarnings({ "SpellCheckingInspection", "StaticVariableName" })
+	public static class TestCls {
+		static String ZPREFIX = "SOME_";
+		private static final String VALUE = ZPREFIX + "VALUE";
+
+		public void check() {
+			assertThat(VALUE).isEqualTo("SOME_VALUE");
+		}
+	}
+
+	@Test
+	public void test() {
+		assertThat(getClassNode(TestCls.class))
+				.code()
+				.containsOne("private static final String VALUE = ZPREFIX + \"VALUE\";");
+	}
+
+	@Test
+	public void testSmali() {
+		assertThat(getClassNodeFromSmali())
+				.runDecompiledAutoCheck(this)
+				.code()
+				.containsOne("private static final String VALUE = ZPREFIX + \"VALUE\";");
+	}
+}

jadx-core/src/test/smali/others/TestFieldInitOrder2.smali

+.class public Lothers/TestFieldInitOrder2;
+.super Ljava/lang/Object;
+
+.field private static final VALUE:Ljava/lang/String;
+.field static final ZPREFIX:Ljava/lang/String; = "SOME_"
+
+
+# direct methods
+.method static constructor <clinit>()V
+    .registers 2
+
+    new-instance v0, Ljava/lang/StringBuilder;
+    invoke-direct {v0}, Ljava/lang/StringBuilder;-><init>()V
+
+    sget-object v1, Lothers/TestFieldInitOrder2;->ZPREFIX:Ljava/lang/String;
+
+    invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder;
+    move-result-object v0
+
+    const-string v1, "VALUE"
+
+    invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder;
+    move-result-object v0
+
+    invoke-virtual {v0}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String;
+    move-result-object v0
+
+    sput-object v0, Lothers/TestFieldInitOrder2;->VALUE:Ljava/lang/String;
+    return-void
+.end method
+
+.method public constructor <init>()V
+    .registers 1
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+    return-void
+.end method
+
+
+.method public check()V
+    .registers 3
+    sget-object v0, Lothers/TestFieldInitOrder2;->VALUE:Ljava/lang/String;
+    invoke-static {v0}, Ljadx/tests/api/utils/assertj/JadxAssertions;->assertThat(Ljava/lang/String;)Ljadx/tests/api/utils/assertj/JadxCodeAssertions;
+    move-result-object v0
+    const-string v1, "SOME_VALUE"
+    invoke-virtual {v0, v1}, Ljadx/tests/api/utils/assertj/JadxCodeAssertions;->isEqualTo(Ljava/lang/String;)Lorg/assertj/core/api/AbstractStringAssert;
+    return-void
+.end method