From 2c0725390ea8f81710d04c2bce1bd005c7e5e561 Mon Sep 17 00:00:00 2001
From: Skylot <skylot@gmail.com>
Date: Thu, 21 Mar 2019 21:23:18 +0300
Subject: [PATCH] fix: check variable usage before convert indexed loop to
 for-each variant (#483)

---
 .../visitors/regions/LoopRegionVisitor.java   |  7 +-
 .../integration/loops/TestIndexedLoop.java    | 71 +++++++++++++++++++
 2 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 jadx-core/src/test/java/jadx/tests/integration/loops/TestIndexedLoop.java

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 0f258e4c..660133bb 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
@@ -116,7 +116,7 @@ public class LoopRegionVisitor extends AbstractVisitor implements IRegionVisitor
 		// all checks passed
 		initInsn.add(AFlag.SKIP);
 		incrInsn.add(AFlag.SKIP);
-		LoopType arrForEach = checkArrayForEach(mth, initInsn, incrInsn, condition);
+		LoopType arrForEach = checkArrayForEach(mth, loopRegion, initInsn, incrInsn, condition);
 		if (arrForEach != null) {
 			loopRegion.setType(arrForEach);
 			return true;
@@ -125,7 +125,7 @@ public class LoopRegionVisitor extends AbstractVisitor implements IRegionVisitor
 		return true;
 	}
 
-	private static LoopType checkArrayForEach(MethodNode mth, InsnNode initInsn, InsnNode incrInsn,
+	private static LoopType checkArrayForEach(MethodNode mth, LoopRegion loopRegion, InsnNode initInsn, InsnNode incrInsn,
 	                                          IfCondition condition) {
 		if (!(incrInsn instanceof ArithNode)) {
 			return null;
@@ -186,6 +186,9 @@ public class LoopRegionVisitor extends AbstractVisitor implements IRegionVisitor
 		if (iterVar == null) {
 			return null;
 		}
+		if (!usedOnlyInLoop(mth, loopRegion, iterVar)) {
+			return null;
+		}
 
 		// array for each loop confirmed
 		len.add(AFlag.SKIP);
diff --git a/jadx-core/src/test/java/jadx/tests/integration/loops/TestIndexedLoop.java b/jadx-core/src/test/java/jadx/tests/integration/loops/TestIndexedLoop.java
new file mode 100644
index 00000000..58454b40
--- /dev/null
+++ b/jadx-core/src/test/java/jadx/tests/integration/loops/TestIndexedLoop.java
@@ -0,0 +1,71 @@
+package jadx.tests.integration.loops;
+
+import java.io.File;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThat;
+
+public class TestIndexedLoop extends IntegrationTest {
+
+	public static class TestCls {
+
+		public File test(File[] files) {
+			File file = null;
+			if (files != null) {
+				int length = files.length;
+				if (length == 0) {
+					file = null;
+				} else {
+					for (int i = 0; i < length; i++) {
+						file = files[i];
+						if (file.getName().equals("f")) {
+							break;
+						}
+					}
+				}
+			} else {
+				file = null;
+			}
+			if (file != null) {
+				file.deleteOnExit();
+			}
+			return file;
+		}
+
+		public void check() {
+			assertThat(test(null), nullValue());
+			assertThat(test(new File[]{}), nullValue());
+
+			File file = new File("f");
+			assertThat(test(new File[]{new File("a"), file}), is(file));
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, not(containsString("for (File file :")));
+		assertThat(code, containsOne("for (int i = 0; i < length; i++) {"));
+	}
+
+	@Test
+	public void testNoDebug() {
+		noDebugInfo();
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, not(containsString("for (File file :")));
+		assertThat(code, containsOne("for (int i = 0; i < length; i++) {"));
+	}
+}
-- 
GitLab