From 1daf5d1090cc0a4d5e346b9f9fb0c948469b5809 Mon Sep 17 00:00:00 2001
From: Skylot <skylot@gmail.com>
Date: Fri, 7 Nov 2014 21:39:27 +0300
Subject: [PATCH] core: fix condition processing (resolve #25)

---
 .../dex/visitors/regions/IfMakerHelper.java   |  2 +
 .../dex/visitors/regions/IfRegionVisitor.java | 46 +++++++++------
 .../integration/conditions/TestNestedIf2.java | 57 +++++++++++++++++++
 3 files changed, 87 insertions(+), 18 deletions(-)
 create mode 100644 jadx-core/src/test/java/jadx/tests/integration/conditions/TestNestedIf2.java

diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java
index 50f261ff1..097864a70 100644
--- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java
+++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java
@@ -175,6 +175,8 @@ public class IfMakerHelper {
 				if (isInversionNeeded(currentIf, nextIf)) {
 					nextIf = IfInfo.invert(nextIf);
 				}
+			} else {
+				return currentIf;
 			}
 		}
 
diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java
index 752c8dfa2..d8b8df9c6 100644
--- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java
+++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java
@@ -45,7 +45,7 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor,
 	@Override
 	public boolean visitRegion(MethodNode mth, IRegion region) {
 		if (region instanceof IfRegion) {
-			return removeRedundantElseBlock((IfRegion) region);
+			return removeRedundantElseBlock(mth, (IfRegion) region);
 		}
 		return false;
 	}
@@ -136,28 +136,38 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor,
 		}
 	}
 
-	private static boolean removeRedundantElseBlock(IfRegion ifRegion) {
-		if (ifRegion.getElseRegion() != null
-				&& !ifRegion.contains(AFlag.ELSE_IF_CHAIN)
-				&& !ifRegion.getElseRegion().contains(AFlag.ELSE_IF_CHAIN)
-				&& hasBranchTerminator(ifRegion)
-				&& insnsCount(ifRegion.getThenRegion()) < 2) {
-			IRegion parent = ifRegion.getParent();
-			Region newRegion = new Region(parent);
-			if (parent.replaceSubBlock(ifRegion, newRegion)) {
-				newRegion.add(ifRegion);
-				newRegion.add(ifRegion.getElseRegion());
-				ifRegion.setElseRegion(null);
-				return true;
-			}
+	private static boolean removeRedundantElseBlock(MethodNode mth, IfRegion ifRegion) {
+		if (ifRegion.getElseRegion() == null
+				|| ifRegion.contains(AFlag.ELSE_IF_CHAIN)
+				|| ifRegion.getElseRegion().contains(AFlag.ELSE_IF_CHAIN)) {
+			return false;
+		}
+		if (!hasBranchTerminator(ifRegion.getThenRegion())) {
+			return false;
+		}
+		// code style check:
+		// will remove 'return;' from 'then' and 'else' with one instruction
+		// see #jadx.tests.integration.conditions.TestConditions9
+		if (mth.getReturnType() == ArgType.VOID
+				&& insnsCount(ifRegion.getThenRegion()) == 2
+				&& insnsCount(ifRegion.getElseRegion()) == 2) {
+			return false;
+		}
+		IRegion parent = ifRegion.getParent();
+		Region newRegion = new Region(parent);
+		if (parent.replaceSubBlock(ifRegion, newRegion)) {
+			newRegion.add(ifRegion);
+			newRegion.add(ifRegion.getElseRegion());
+			ifRegion.setElseRegion(null);
+			return true;
 		}
 		return false;
 	}
 
-	private static boolean hasBranchTerminator(IfRegion ifRegion) {
+	private static boolean hasBranchTerminator(IContainer region) {
 		// TODO: check for exception throw
-		return RegionUtils.hasExitBlock(ifRegion.getThenRegion())
-				|| RegionUtils.hasBreakInsn(ifRegion.getThenRegion());
+		return RegionUtils.hasExitBlock(region)
+				|| RegionUtils.hasBreakInsn(region);
 	}
 
 	private static void invertIfRegion(IfRegion ifRegion) {
diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestNestedIf2.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestNestedIf2.java
new file mode 100644
index 000000000..0e9ccec22
--- /dev/null
+++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestNestedIf2.java
@@ -0,0 +1,57 @@
+package jadx.tests.integration.conditions;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import org.junit.Test;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestNestedIf2 extends IntegrationTest {
+
+	public static class TestCls {
+		static int executedCount = 0;
+		static boolean finished = false;
+		static int repeatCount = 2;
+
+		static boolean test(float delta, Object object) {
+			if (executedCount != repeatCount && isRun(delta, object)) {
+				if (finished) {
+					return true;
+				}
+				if (repeatCount == -1) {
+					++executedCount;
+					action();
+					return false;
+				}
+				++executedCount;
+				if (executedCount >= repeatCount) {
+					return true;
+				}
+				action();
+			}
+			return false;
+		}
+
+		public static void action() {
+		}
+
+		public static boolean isRun(float delta, Object object) {
+			return delta == 0;
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("if (executedCount != repeatCount && isRun(delta, object)) {"));
+		assertThat(code, containsOne("if (finished) {"));
+		assertThat(code, not(containsString("else")));
+
+	}
+}