Skip to content

Commit

Permalink
Better support for ImmutableSortedSet and ImmutableSortedMap. Use .co…
Browse files Browse the repository at this point in the history
…pyOfSorted when setting from a SortedSet or SortedMap. Use .naturalOrder when constructing a builder.

Fixes #666.

RELNOTES=Use ImmutableSortedSet.copyOfSorted and .naturalOrder where appropriate.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=249632356
  • Loading branch information
eamonnmcmanus authored and ronshapiro committed May 27, 2019
1 parent e0c631a commit 7564c4c
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.ImmutableTable;
import com.google.common.testing.EqualsTester;
import com.google.common.testing.SerializableTester;
Expand All @@ -53,7 +55,12 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.NavigableMap;
import java.util.NavigableSet;
import java.util.NoSuchElementException;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.TreeSet;
import javax.annotation.Nullable;
import org.junit.BeforeClass;
import org.junit.Test;
Expand Down Expand Up @@ -2220,15 +2227,89 @@ public void testBuilderWithCopyingSetters() {
BuilderWithCopyingSetters.Builder<Integer> builder = BuilderWithCopyingSetters.builder(23);

BuilderWithCopyingSetters<Integer> a = builder.setThings(ImmutableSet.of(1, 2)).build();
assertEquals(ImmutableSet.of(1, 2), a.things());
assertEquals(ImmutableList.of(17, 23.0), a.numbers());
assertEquals(ImmutableMap.of("foo", 23), a.map());
assertThat(a.things()).containsExactly(1, 2);
assertThat(a.numbers()).containsExactly(17, 23.0).inOrder();
assertThat(a.map()).containsExactly("foo", 23);

BuilderWithCopyingSetters<Integer> b = builder.setThings(Arrays.asList(1, 2)).build();
assertEquals(a, b);
assertThat(b).isEqualTo(a);

BuilderWithCopyingSetters<Integer> c = builder.setThings(1, 2).build();
assertEquals(a, c);
assertThat(c).isEqualTo(a);
}

@AutoValue
public abstract static class BuilderWithImmutableSorted<T extends Comparable<T>> {
public abstract ImmutableSortedSet<T> sortedSet();

public abstract ImmutableSortedMap<T, Integer> sortedMap();

public static <T extends Comparable<T>> Builder<T> builder() {
return new AutoValue_AutoValueTest_BuilderWithImmutableSorted.Builder<T>()
.setSortedSet(new TreeSet<T>())
.setSortedMap(new TreeMap<T, Integer>());
}

@AutoValue.Builder
public interface Builder<T extends Comparable<T>> {
@SuppressWarnings("unchecked")
Builder<T> setSortedSet(T... x);

Builder<T> setSortedSet(NavigableSet<T> x);

ImmutableSortedSet.Builder<T> sortedSetBuilder();

Builder<T> setSortedMap(SortedMap<T, Integer> x);

Builder<T> setSortedMap(NavigableMap<T, Integer> x);

ImmutableSortedMap.Builder<T, Integer> sortedMapBuilder();

BuilderWithImmutableSorted<T> build();
}
}

@Test
public void testBuilderWithImmutableSorted_Varargs() {
BuilderWithImmutableSorted<String> x =
BuilderWithImmutableSorted.<String>builder().setSortedSet("foo", "bar", "baz").build();
assertThat(x.sortedSet()).containsExactly("bar", "baz", "foo").inOrder();
}

@Test
public void testBuilderWithImmutableSorted_SetSet() {
BuilderWithImmutableSorted<String> x =
BuilderWithImmutableSorted.<String>builder()
.setSortedSet(new TreeSet<String>(String.CASE_INSENSITIVE_ORDER))
.build();
assertThat(x.sortedSet().comparator()).isEqualTo(String.CASE_INSENSITIVE_ORDER);
}

@Test
public void testBuilderWithImmutableSorted_SetMap() {
BuilderWithImmutableSorted<String> x =
BuilderWithImmutableSorted.<String>builder()
.setSortedMap(new TreeMap<String, Integer>(String.CASE_INSENSITIVE_ORDER))
.build();
assertThat(x.sortedMap().comparator()).isEqualTo(String.CASE_INSENSITIVE_ORDER);
}

@Test
public void testBuilderWithImmutableSorted_SetCollectionBuilder() {
BuilderWithImmutableSorted.Builder<String> builder =
BuilderWithImmutableSorted.<String>builder();
builder.sortedSetBuilder().add("is", "ea", "id");
BuilderWithImmutableSorted<String> x = builder.build();
assertThat(x.sortedSet()).containsExactly("ea", "id", "is").inOrder();
}

@Test
public void testBuilderWithImmutableSorted_MapCollectionBuilder() {
BuilderWithImmutableSorted.Builder<String> builder =
BuilderWithImmutableSorted.<String>builder();
builder.sortedMapBuilder().put("two", 2).put("one", 1);
BuilderWithImmutableSorted<String> x = builder.build();
assertThat(x.sortedMap()).containsExactly("one", 1, "two", 2).inOrder();
}

@AutoValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,26 +526,28 @@ private ImmutableList<ExecutableElement> copyOfMethods(
if (!targetType.getKind().equals(TypeKind.DECLARED)) {
return ImmutableList.of();
}
String copyOf;
ImmutableSet<String> copyOfNames;
Optionalish optionalish = Optionalish.createIfOptional(targetType);
if (optionalish == null) {
copyOf = "copyOf";
copyOfNames = ImmutableSet.of("copyOfSorted", "copyOf");
} else {
VariableElement parameterElement = Iterables.getOnlyElement(setter.getParameters());
boolean nullable =
AutoValueOrOneOfProcessor.nullableAnnotationFor(
parameterElement, parameterElement.asType())
.isPresent();
copyOf = nullable ? optionalish.ofNullable() : "of";
copyOfNames = ImmutableSet.of(nullable ? optionalish.ofNullable() : "of");
}
TypeElement targetTypeElement = MoreElements.asType(typeUtils.asElement(targetType));
ImmutableList.Builder<ExecutableElement> copyOfMethods = ImmutableList.builder();
for (ExecutableElement method :
ElementFilter.methodsIn(targetTypeElement.getEnclosedElements())) {
if (method.getSimpleName().contentEquals(copyOf)
&& method.getParameters().size() == 1
&& method.getModifiers().contains(Modifier.STATIC)) {
copyOfMethods.add(method);
for (String copyOfName : copyOfNames) {
for (ExecutableElement method :
ElementFilter.methodsIn(targetTypeElement.getEnclosedElements())) {
if (method.getSimpleName().contentEquals(copyOfName)
&& method.getParameters().size() == 1
&& method.getModifiers().contains(Modifier.STATIC)) {
copyOfMethods.add(method);
}
}
}
return copyOfMethods.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ public String getCopyAll() {
// (1) It must have an instance method called `build()` that returns `Bar`. If the type of
// `bar()` is `Bar<String>` then the type of `build()` must be `Bar<String>`.
// (2) `BarBuilder` must have a public no-arg constructor, or `Bar` must have a static method
// `builder()` or `newBuilder()` that returns `BarBuilder`.
// `naturalOrder(), `builder()`, or `newBuilder()` that returns `BarBuilder`. The
// `naturalOrder()` case is specifically for ImmutableSortedSet and ImmutableSortedMap.
// (3) `Bar` must have an instance method `BarBuilder toBuilder()`, or `BarBuilder` must be a
// Guava immutable builder like `ImmutableSet.Builder`. (See TODO below for relaxing the
// requirement on having a `toBuilder()`.
Expand Down Expand Up @@ -324,10 +325,10 @@ Optional<PropertyBuilder> makePropertyBuilder(ExecutableElement method, String p
}

private static final ImmutableSet<String> BUILDER_METHOD_NAMES =
ImmutableSet.of("builder", "newBuilder");
ImmutableSet.of("naturalOrder", "builder", "newBuilder");

// (2) `BarBuilder must have a public no-arg constructor, or `Bar` must have a visible static
// method `builder()` or `newBuilder()` that returns `BarBuilder`.
// method `naturalOrder(), `builder()`, or `newBuilder()` that returns `BarBuilder`.
private Optional<ExecutableElement> builderMaker(
Map<String, ExecutableElement> barNoArgMethods, TypeElement barBuilderTypeElement) {
for (String builderMethodName : BUILDER_METHOD_NAMES) {
Expand Down

0 comments on commit 7564c4c

Please sign in to comment.