Skip to content

Commit

Permalink
fix: handle empty block at end of else-if chain (#1674)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Sep 23, 2022
1 parent 79477a2 commit 151c171
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import java.util.List;

import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.nodes.IContainer;
import jadx.core.dex.nodes.IRegion;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.regions.Region;
import jadx.core.dex.regions.conditions.IfCondition;
Expand Down Expand Up @@ -45,7 +47,7 @@ public boolean enterRegion(MethodNode mth, IRegion region) {
}
}

@SuppressWarnings("UnnecessaryReturnStatement")
@SuppressWarnings({ "UnnecessaryReturnStatement", "StatementWithEmptyBody" })
private static void orderBranches(MethodNode mth, IfRegion ifRegion) {
if (RegionUtils.isEmpty(ifRegion.getElseRegion())) {
return;
Expand Down Expand Up @@ -79,9 +81,15 @@ private static void orderBranches(MethodNode mth, IfRegion ifRegion) {
return;
}
}
boolean lastRegion = ifRegion == RegionUtils.getLastRegion(mth.getRegion());
boolean lastRegion = RegionUtils.hasExitEdge(ifRegion);
if (elseSize == 1 && lastRegion && mth.isVoidReturn()) {
// single return at method end will be removed later
InsnNode lastElseInsn = RegionUtils.getLastInsn(ifRegion.getElseRegion());
if (lastElseInsn != null && lastElseInsn.getType() == InsnType.THROW) {
// move `throw` into `then` block
invertIfRegion(ifRegion);
} else {
// single return at method end will be removed later
}
return;
}
if (!lastRegion) {
Expand Down
14 changes: 0 additions & 14 deletions jadx-core/src/main/java/jadx/core/utils/RegionUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,20 +146,6 @@ public static IBlock getLastBlock(IContainer container) {
}
}

@Nullable
public static IContainer getLastRegion(@Nullable IContainer container) {
if (container == null) {
return null;
}
if (container instanceof IBlock || container instanceof IBranchRegion) {
return container;
}
if (container instanceof IRegion) {
return getLastRegion(Utils.last(((IRegion) container).getSubBlocks()));
}
throw new JadxRuntimeException(unknownContainerType(container));
}

public static boolean isExitBlock(MethodNode mth, IContainer container) {
if (container instanceof BlockNode) {
return BlockUtils.isExitBlock(mth, (BlockNode) container);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package jadx.tests.integration.conditions;

import org.junit.jupiter.api.Test;

import jadx.api.ICodeWriter;
import jadx.tests.api.IntegrationTest;

import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;

public class TestElseIfCodeStyle extends IntegrationTest {

@SuppressWarnings("unused")
public static class TestCls {

public void test(String str) {
if ("a".equals(str)) {
call(1);
} else if ("b".equals(str)) {
call(2);
} else if ("c".equals(str)) {
call(3);
}
}

private void call(int i) {
}
}

@Test
public void test() {
noDebugInfo();
assertThat(getClassNode(TestCls.class))
.code()
.doesNotContain("!\"c\".equals(str)")
.doesNotContain("{" + ICodeWriter.NL + indent(2) + "} else {"); // no empty `then` block
}
}

0 comments on commit 151c171

Please sign in to comment.