Refactor LocatedItems::mergeItemsIntoNext

Add test for mergeInto to make sure the order is kept.
This commit is contained in:
Assaf 2018-07-08 16:01:34 +03:00 committed by Ben Gruver
parent dd242aa735
commit af8bc1d9cd
5 changed files with 81 additions and 27 deletions

View File

@ -3,7 +3,7 @@ package org.jf.dexlib2.builder;
public class LocatedDebugItems extends LocatedItems<BuilderDebugItem> { public class LocatedDebugItems extends LocatedItems<BuilderDebugItem> {
@Override @Override
protected String addLocatedItemError() { protected String getAddLocatedItemError() {
return "Cannot add a debug item that has already been added to a method." + return "Cannot add a debug item that has already been added to a method." +
"You must remove it from its current location first."; "You must remove it from its current location first.";
} }

View File

@ -13,16 +13,14 @@ public abstract class LocatedItems<T extends ItemWithLocation> {
@Nullable @Nullable
private List<T> items = null; private List<T> items = null;
@Nonnull private void initItemsIfNull() {
private List<T> getMutableItems() {
if (items == null) { if (items == null) {
items = new ArrayList<>(1); items = new ArrayList<>(1);
} }
return items;
} }
@Nonnull @Nonnull
private List<T> getImmutableItems() { private List<T> getItems() {
if (items == null) { if (items == null) {
return ImmutableList.of(); return ImmutableList.of();
} }
@ -32,23 +30,27 @@ public abstract class LocatedItems<T extends ItemWithLocation> {
public Set<T> getModifiableItems(MethodLocation newItemsLocation) { public Set<T> getModifiableItems(MethodLocation newItemsLocation) {
return new AbstractSet<T>() { return new AbstractSet<T>() {
@Nonnull @Nonnull
@Override public Iterator<T> iterator() { @Override
final Iterator<T> it = getImmutableItems().iterator(); public Iterator<T> iterator() {
final Iterator<T> it = getItems().iterator();
return new Iterator<T>() { return new Iterator<T>() {
private @Nullable private @Nullable
T currentItem = null; T currentItem = null;
@Override public boolean hasNext() { @Override
public boolean hasNext() {
return it.hasNext(); return it.hasNext();
} }
@Override public T next() { @Override
public T next() {
currentItem = it.next(); currentItem = it.next();
return currentItem; return currentItem;
} }
@Override public void remove() { @Override
public void remove() {
if (currentItem != null) { if (currentItem != null) {
currentItem.setLocation(null); currentItem.setLocation(null);
} }
@ -57,30 +59,37 @@ public abstract class LocatedItems<T extends ItemWithLocation> {
}; };
} }
@Override public int size() { @Override
return getImmutableItems().size(); public int size() {
return getItems().size();
} }
@Override public boolean add(@Nonnull T item) { @Override
public boolean add(@Nonnull T item) {
if (item.isPlaced()) { if (item.isPlaced()) {
throw new IllegalArgumentException(addLocatedItemError()); throw new IllegalArgumentException(getAddLocatedItemError());
} }
item.setLocation(newItemsLocation); item.setLocation(newItemsLocation);
getMutableItems().add(item); initItemsIfNull();
items.add(item);
return true; return true;
} }
}; };
} }
protected abstract String addLocatedItemError(); protected abstract String getAddLocatedItemError();
public void mergeItemsInto(@Nonnull MethodLocation newLocation, LocatedItems<T> otherLocatedItems) { public void mergeItemsIntoNext(@Nonnull MethodLocation nextLocation, LocatedItems<T> otherLocatedItems) {
if (items != null || otherLocatedItems.items != null) { if (otherLocatedItems == this) {
List<T> otherItems = otherLocatedItems.getMutableItems(); return;
for (T item: getImmutableItems()) {
item.setLocation(newLocation);
otherItems.add(item);
} }
if (items != null) {
for (T item : items) {
item.setLocation(nextLocation);
}
List<T> mergedItems = items;
mergedItems.addAll(otherLocatedItems.getItems());
otherLocatedItems.items = mergedItems;
items = null; items = null;
} }
} }

View File

@ -2,7 +2,7 @@ package org.jf.dexlib2.builder;
public class LocatedLabels extends LocatedItems<Label> { public class LocatedLabels extends LocatedItems<Label> {
@Override @Override
protected String addLocatedItemError() { protected String getAddLocatedItemError() {
return "Cannot add a label that is already placed." + return "Cannot add a label that is already placed." +
"You must remove it from its current location first."; "You must remove it from its current location first.";
} }

View File

@ -46,7 +46,6 @@ public class MethodLocation {
int index; int index;
private final LocatedItems<Label> labels; private final LocatedItems<Label> labels;
@Nullable
private final LocatedItems<BuilderDebugItem> debugItems; private final LocatedItems<BuilderDebugItem> debugItems;
MethodLocation(@Nullable BuilderInstruction instruction, int codeAddress, int index) { MethodLocation(@Nullable BuilderInstruction instruction, int codeAddress, int index) {
@ -70,9 +69,9 @@ public class MethodLocation {
return index; return index;
} }
void mergeInto(@Nonnull MethodLocation other) { void mergeInto(@Nonnull MethodLocation nextLocation) {
labels.mergeItemsInto(other, other.labels); labels.mergeItemsIntoNext(nextLocation, nextLocation.labels);
debugItems.mergeItemsInto(other, other.debugItems); debugItems.mergeItemsIntoNext(nextLocation, nextLocation.debugItems);
} }
@Nonnull @Nonnull

View File

@ -0,0 +1,46 @@
package org.jf.dexlib2.builder;
import com.google.common.collect.Sets;
import org.jf.dexlib2.builder.debug.BuilderLineNumber;
import org.junit.Assert;
import org.junit.Test;
import java.util.ArrayList;
import java.util.List;
public class LocatedItemsTest {
private List<BuilderDebugItem> createItems(int count) {
List<BuilderDebugItem> items = new ArrayList<>();
for(int i = 0; i < count; ++i) {
items.add(new BuilderLineNumber(i));
}
return items;
}
private void doTestMergeIntoKeepsOrderOfDebugItems(int countLocation1, int countLocation2) {
MethodLocation location1 = new MethodLocation(null, 123, 1);
MethodLocation location2 = new MethodLocation(null, 456, 2);
List<BuilderDebugItem> items1 = createItems(countLocation1);
List<BuilderDebugItem> items2 = createItems(countLocation2);
location1.getDebugItems().addAll(items1);
location2.getDebugItems().addAll(items2);
location1.mergeInto(location2);
Assert.assertEquals(Sets.newHashSet(), location1.getDebugItems());
// items1 appear BEFORE items2
List<BuilderDebugItem> expectedItems = new ArrayList<>(items1);
expectedItems.addAll(items2);
Assert.assertEquals(expectedItems, new ArrayList<>(location2.getDebugItems()));
}
@Test
public void testMergeIntoKeepsOrderOfDebugItems() {
doTestMergeIntoKeepsOrderOfDebugItems(2, 2);
doTestMergeIntoKeepsOrderOfDebugItems(0, 0);
doTestMergeIntoKeepsOrderOfDebugItems(0, 2);
doTestMergeIntoKeepsOrderOfDebugItems(2, 0);
}
}