Introduce `ImmutableCollection` templates #40

mussatto · 2022-02-18T09:38:22Z

Introduce templates for {Set,List,Map}.of(...) and Collection.{emptyList,emptyMap,emptySet}() to Immutable{Set,List.Map}.of(...) from zero up to 5 elements.

EnricSala

Could it make sense to also do List.of(...) -> ImmutableList.of(...) and similar for other collections? (e.g. Set, Map)

(Not sure if already exists or planned for another PR)

rickie · 2022-02-18T09:59:51Z

That sounds like a good addition! So as you mention, not only List.of() but also with params. The question then is, how far do we go with the number of parameters? Currently, there is not a nice way to rewrite for n number of parameters... So that means we have to add multiple Refaster templates. I don't think that is a problem though.

rickie

I was pretty sure that I was reviewing the last version of the code. Apparently, something went wrong and some comments are on an older version.. Sorry for that.

These templates will have a nice impact on the code 🚀 !

rickie · 2022-02-22T14:12:47Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

+    }
+
+    @AfterTemplate
+    ImmutableCollection<T> after(T item) {


In this case having ImmutableList as return type would be more clear/honest about the rewrite we are doing here. Changing the return type to ImmutableList would be consistent with the template above (ImmutableListOf).

rickie · 2022-02-22T14:24:18Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

+  static final class ImmutableListOf2<T> {
+    @BeforeTemplate
+    List<T> before(T item, T item2) {
+      return List.of(item, item2);


For the parameter names, we should have a consistent way of naming them. Instead of item or i1 we could use e1, e2, etc. as they also do in the actual implementation of the ImmutableList.

Based on other templates in this repo (mainly in AssertJTemplates.java), we can say that a sort of (unwritten) convention is that we "prefer" using the word element.

Ah, for the ImmutableSet templates that are added here the e1 and e2 are used, so changing this would make it consistent 😄

rickie · 2022-02-22T14:49:03Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

@@ -186,4 +187,76 @@ private ImmutableListTemplates() {}
      return stream.collect(toImmutableSet()).asList();
    }
  }
+
+  static final class ImmutableListOf<T> {


The other templates in this class have a Javadoc, let's add it for the other templates we introduce in this PR as well.

Perhaps we could do something like this example:

/** * Prefer {@link ImmutableList#of(Object)} over alternatives that don't communicate the * immutability of the resulting list at the type level. */

The implementation of List.of(element ...) is:

static <E> List<E> of(E e1, E e2, E e3) { return new ImmutableCollections.ListN<>(e1, e2, e3); }

So the example Javadoc has some similarities with the Refaster templates we are adding in this PR.

This also applies for the other introduced Refaster templates in Immutable{Map,Set}.java.

For most templates the Javadoc is now good. Only for this case, the emptyList() I'm not 100% sure. So the List.of(..) calls uses the ImmutableCollections under the hood. However, for the emptyList() this is not the case:

public static final <T> List<T> emptyList() { return (List<T>) EMPTY_LIST; }

I'm not sure what to write in the Javadoc there, so also leaving this open for other reviewers to give their input :)

I think we should be very careful here (also when considering List.of() as before template, see other comment). We have three different List implementations with different behavior:

Existing @Before Collections.emptyList(): uses Java's Collections.EmptyList which silently ignores calls to sort(), replaceAll() and removeIf() as there are no elements anyway.

Suggested @Before List.of(): uses Java's ImmutableCollections.ListN which throws UnsupportedOperationException on those methods.

Existing @After ImmutableList.of(): uses Guava's ImmutableList which throws UnsupportedOperationException on those methods.

My suggestion (after further implementation investigation):
List.of() and ImmutableList.of() have the most similar implementation and I would be happy to make that conversion. I would create an ErrorProne check for Collections.emptyList() to suggest using ImmutableList.of() warning them for possible side-effects due to differences in List implementations. WDYT?

Regarding JavaDoc, I'm happy with current state on this method, except that it mentions {@link ImmutableList#of(Object)} instead of {@link ImmutableList#of()}.

Good remark, we should do this 😄.

For the Collections.emptyList() we can use the same setup as FluxFlatMapUsageCheck.java. We should do two suggested fixes there, such that it will say: "Do you want to use ImmutableList.of() or add a SuppressWarnings("...")"?

Lets create a new PR to create an error prone rule to validate Collections.emptyList() and suggest ImmutableList.of()

And on this PR I will change current refaster template to List.of() -> ImmutableList.of()

rickie · 2022-02-22T14:49:32Z

...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java

+    ImmutableMap<K, V> after(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4, K k5, V v5) {
+      return ImmutableMap.of(k1, v1, k2, v2, k3, v3, k4, v4, k5, v5);
+    }
+  }


Let's add a newline between the class and the comments.

rickie · 2022-02-22T15:04:31Z

...b/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java

@@ -79,4 +81,32 @@
  ImmutableList<Integer> testStreamToDistinctImmutableList() {
    return Stream.of(1).distinct().collect(toImmutableList());
  }
+
+  List<?> testImmutableListOf() {


We discussed this offline a bit; this was a hard case to test, but the question was: given the implementation of emptyList() is there a way we can also test the <T> with something else then the default Object.

The implementation:

public static final <T> List<T> emptyList() { return (List<T>) EMPTY_LIST; }

Simply returning the default would result in this:

List<Object> test() { return Collections.emptyList(); }

So what we could do is add the following in the *TestInput.java:

ImmutableSet<List<?>> testImmutableListOf() { return ImmutableSet.of(Collections.emptyList(), Collections.<String>emptyList()); }

The second case is of type List<String>.

So in this case we would test what we discussed the first time. However, not 100% certain this does add a lot. So will leave this as an open question to other reviewers 😄.

:)
I am leaving as is until then

I think it'll be good to make some changes before that. The current state (having two different methods test one Refaster template) is not something we want. In this case testImmutableListOf and testImmutableListOfTyped test one Refaster template. We want one test method per Refaster template, so it would be nice to merge these methods in one.

Besides that, we can be more precise with the List<?>. The default is Object and in the second example with the <String>emptyList(), List<Object would still be good as a return type. In general, it is better to be more precise about returntypes where possible. So I'd suggest to change the ? and be more precise.

rickie · 2022-02-22T15:07:34Z

...b/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java

+    return Collections.emptyList();
+  }
+
+  List<String> testImmutableListOfTyped() {


This is a comment to give some additional context on having multiple test methods for one Refaster template (explanation copied over from here:

Useful to know: if we want to have multiple test cases for one Refaster template, we generally wrap these cases in an ImmutableSet.of(...) and return that. This way we have the test cases together in one single method.
Within this class there are other methods that do this. See for example:
ImmutableSet<Duration> testZeroDuration() {
As a result the tests are more concise and it is easier to see how a Refaster template is tested.

rickie · 2022-02-22T15:12:30Z

...ib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestInput.java

+    return Collections.emptyMap();
+  }
+
+  ImmutableSet<Map<?, ?>> testImmutableMapOfN() {


Since we are only using Strings here as arguments, it's safe to use ImmutableSet<Map<String, String>> here as return type.

The <?, ?> is mostly used for templates that work with AssertJ.

rickie · 2022-02-22T15:26:33Z

...ib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableMapTemplatesTestInput.java

+
+  ImmutableSet<Map<?, ?>> testImmutableMapOfN() {
+    return ImmutableSet.of(
+        Collections.emptyMap(),


Ah, this got updated while reviewing. My assumption is that this change is based on this feedback in the other PR. However, there is a small and important distinction to make here.

So now we have one test, that contains multiple test cases for multiple Refaster templates.
The important line from the comment in the other PR: "if we want to have multiple test cases for one Refaster template", we should wrap the test cases in a ImmutableSet.
Notice the difference that is highlighted.

In summary; the previous state was already in a good shape, because we had a test method with a test case for each Refaster template. If we want to add multiple test cases per Refaster template, we should put the test cases in the same method by using a ImmutableSet.

Was the assumption correct? Let me know if the explanation is clear 🙂

Now I see that in the other PR my comment wasn't clear enough, which makes it clear that is the reason for making this change. Will make the explanation more explicit next time 🙂.

rickie · 2022-02-24T07:36:55Z

...b/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java

@@ -79,4 +81,32 @@
  ImmutableList<Integer> testStreamToDistinctImmutableList() {
    return Stream.of(1).distinct().collect(toImmutableList());
  }
+
+  List<?> testImmutableListOf() {


I think it'll be good to make some changes before that. The current state (having two different methods test one Refaster template) is not something we want. In this case testImmutableListOf and testImmutableListOfTyped test one Refaster template. We want one test method per Refaster template, so it would be nice to merge these methods in one.

Besides that, we can be more precise with the List<?>. The default is Object and in the second example with the <String>emptyList(), List<Object would still be good as a return type. In general, it is better to be more precise about returntypes where possible. So I'd suggest to change the ? and be more precise.

rickie · 2022-02-24T07:47:21Z

...ib/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableSetTemplatesTestInput.java

@@ -76,23 +75,23 @@
    return Collections.emptySet();
  }

-  Collection<String> testImmutableSetOfItems1() {
+  Set<String> testImmutableSetOfItems1() {


For ImmutableList and ImmutableMap templates the name is of format: Immutable{List,Map}OfN where N is the number.

For ImmutableSet the naming is a bit different, so I'd suggest to update that to be consistent withImmutableList and ImmutableMap. In this case it would be ImmutableSetOf instead of ImmutableSetOfItems1.

rickie · 2022-02-24T07:50:44Z

Sorry, meant to comment but pressed approve 😬

rickie · 2022-02-25T10:39:54Z

...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java

+   * Prefer {@link ImmutableSet#of(Object)} over alternatives that don't communicate the
+   * immutability of the resulting list at the type level.
+   */
+  static final class ImmutableSetOfItems1<T> {


Here there are a few templates that still have Items in the name. The tests are updated, but here we do not reflect these changes.

rickie · 2022-02-25T10:44:30Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

@@ -186,4 +187,76 @@ private ImmutableListTemplates() {}
      return stream.collect(toImmutableSet()).asList();
    }
  }
+
+  static final class ImmutableListOf<T> {


For most templates the Javadoc is now good. Only for this case, the emptyList() I'm not 100% sure. So the List.of(..) calls uses the ImmutableCollections under the hood. However, for the emptyList() this is not the case:

public static final <T> List<T> emptyList() { return (List<T>) EMPTY_LIST; }

I'm not sure what to write in the Javadoc there, so also leaving this open for other reviewers to give their input :)

Badbond

Challenging but very worthwhile PR. Great to see the collaboration 💪
Adding premature review to place comment in a discussion thread that should be considered rather sooner than later.

...b/src/test/resources/tech/picnic/errorprone/bugpatterns/ImmutableListTemplatesTestInput.java

Badbond · 2022-02-25T13:17:59Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

+    List<T> before() {
+      return Collections.emptyList();
+    }
+


Shall we also consider List.of() as @BeforeTemplate here?

Suggested change

@BeforeTemplate

List<T> before() {

return List.of();

}

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

Badbond · 2022-02-25T13:59:38Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

@@ -186,4 +187,76 @@ private ImmutableListTemplates() {}
      return stream.collect(toImmutableSet()).asList();
    }
  }
+
+  static final class ImmutableListOf<T> {


I think we should be very careful here (also when considering List.of() as before template, see other comment). We have three different List implementations with different behavior:

Existing @Before Collections.emptyList(): uses Java's Collections.EmptyList which silently ignores calls to sort(), replaceAll() and removeIf() as there are no elements anyway.

Suggested @Before List.of(): uses Java's ImmutableCollections.ListN which throws UnsupportedOperationException on those methods.

Existing @After ImmutableList.of(): uses Guava's ImmutableList which throws UnsupportedOperationException on those methods.

My suggestion (after further implementation investigation):
List.of() and ImmutableList.of() have the most similar implementation and I would be happy to make that conversion. I would create an ErrorProne check for Collections.emptyList() to suggest using ImmutableList.of() warning them for possible side-effects due to differences in List implementations. WDYT?

Regarding JavaDoc, I'm happy with current state on this method, except that it mentions {@link ImmutableList#of(Object)} instead of {@link ImmutableList#of()}.

werli · 2022-02-25T15:40:07Z

Fly-by-comment: I was expecting templates for the Immutables library that we're using "only" to find templates dealing with Guava's ImmutableCollections.
What about /s/Immutables/ImmutableCollection in the PR title? 🙂

Also, very nice seeing this collaboration in this repository, exciting times ahead. 😉

Badbond

Thanks for the discussion around Collections.emptyList. Went over the entire PR now and see the same reasoning for emptySet and emptyMap. Apart from this, LGTM.

Badbond · 2022-02-25T15:04:11Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

@@ -186,4 +186,100 @@ private ImmutableListTemplates() {}
      return stream.collect(toImmutableSet()).asList();
    }
  }
+
+  /**
+   * Prefer {@link ImmutableList#of(Object)} over alternatives that don't communicate the


Suggested change

* Prefer {@link ImmutableList#of(Object)} over alternatives that don't communicate the

* Prefer {@link ImmutableList#of()} over alternatives that don't communicate the

Badbond · 2022-02-28T17:11:59Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

+
+    @AfterTemplate
+    ImmutableList<T> after(T e1) {
+      return ImmutableList.of(e1);


Strictly speaking the signature uses element as parameter name. But I actually prefer this for consistency 😄 (Set also uses e1 for single argument method). Just wanted to point it out.

I also prefer this for consistency :)

Badbond · 2022-02-28T17:19:48Z

...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java

+  static final class ImmutableMapOf<K, V> {
+    @BeforeTemplate
+    Map<K, V> before() {
+      return Collections.emptyMap();


Same reasoning as with Collections.emptyList() this implementation (Collections.EmptyMap) differs significantly compared to RegularImmutableMap (e.g. clear(), replaceAll()). Would recommend to do an error prone check instead. Also here we can opt for Map.of() as @BeforeTemplate instead.

Thanks for the thorough investigation @Badbond! I'm fine with keeping the PR as-is given that I don't expect the difference to bite us in practice, but OTOH an Error Prone check would indeed enable custom error messages. (Though perhaps we should add support for custom error messages to Refaster; this could be done by having RefasterCheck understand some custom annotation.)

Yet on the other hand, an Error Prone check would avoid the current boilerplate and generalize to >5 arguments.

Badbond · 2022-02-28T17:22:42Z

...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java

+  static final class ImmutableSetOf<T> {
+    @BeforeTemplate
+    Set<T> before() {
+      return Collections.emptySet();


Same reasoning as with Collections.emptyList() (and Map) this implementation (Collections.EmptySet) differs significantly compared to RegularImmutableSet (e.g. clear(), removeIf()). Would recommend to do an error prone check instead. Also here we can opt for Set.of() as @BeforeTemplate instead.

Badbond

LGTM. Suggested commit message (feel free to change):

Introduce `Immutable{List,Map,Set}#of` Refaster templates

rickie

Nice work @mussatto, the templates look good! 🚀
Curious to see how they perform on the Picnic codebase 😄.

Was going over the tests and noticed that we more often use Integer in tests than Strings.
So pushed a suggestion that changed it for the Immutable{List,Set}. For the ImmutableMap I feel that it is nice to have the "k1" and "v1", so leaving that as is. WDYT?
Again, here we do not have a hard rule (yet) that says we have do this. In this case the Integer variant is a bit shorter and overall more used in these Test{Input,Output} files.

Tweaked the suggested commit message a bit.

Badbond · 2022-03-02T15:35:23Z

Might be nice to update PR description a bit based on the new changes btw 🙂

rickie · 2022-03-31T14:01:37Z

Discussed the original rewrites of this PR again, so the following three:

Collections.emptySet() --> ImmutableSet.of()
Collections.emptyList() --> ImmutableList.of()
Collections.emptyMap() --> ImmutableMap.of()

The fact that there are some minor differences between the API's is not that big of a problem.
There are more rewrites where there are such subtle differences.
We can add some extra information in the Javadoc (or an XXX) explaining that strictly speaking this is not a behavior preserving change. The changes that we see here are not "strong/impactful" enough to say let's create a BugPattern for this.
In comparison, creating a BugPattern for this rewrite costs much more time because we can make a simple Refaster template for this. Especially because there is not that big of a difference, it is not really worth our time and effort to create a BugPattern for this.

Badbond · 2022-03-31T14:26:44Z

The fact that there are some minor differences between the API's is not that big of a problem. There are more rewrites where there are such subtle differences.

I would not call going from silently ignoring to throwing exceptions a subtle/minor difference (reference comment), but I can imagine the thoughts as it is only on a select few methods. For our own internal projects, I won't expect an impact. But in the grand scheme of things when going open-source, I suspect that this might cause problems for some projects. But if the majority agrees on this, I won't hold this back 🙂

rickie

Nice @mussatto !

Made a suggestion with no strong reasoning behind it (other then sorting lexicographical 🤔). WDYT?

Stephan202

Rebased and resolved conflicts; review TBD.

Was going over the tests and noticed that we more often use Integer in tests than Strings.

Historically I switched between these just for some variation 😅.

Stephan202

Rebased and added a commit. I think an Error Prone check which identifies all these different ways in which to create a fixed-size collection might be worthwhile eventually. It could emit suitable customized warnings, identify different ways in which builders can be used, and work for >5 elements.

But this PR is certainly good enough for now, so: let's roll!

Alternative suggested commit message:

Introduce additional `Immutable{List,Map,Set}#of` Refaster templates (#40)

Stephan202 · 2022-04-10T15:19:23Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

+    }
+
+    @BeforeTemplate
+    List<T> before2() {


We can combine these @BeforeTemplate methods using Refaster.anyOf.

Stephan202 · 2022-04-10T15:23:31Z

...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java

+  static final class ImmutableMapOf<K, V> {
+    @BeforeTemplate
+    Map<K, V> before() {
+      return Collections.emptyMap();


Thanks for the thorough investigation @Badbond! I'm fine with keeping the PR as-is given that I don't expect the difference to bite us in practice, but OTOH an Error Prone check would indeed enable custom error messages. (Though perhaps we should add support for custom error messages to Refaster; this could be done by having RefasterCheck understand some custom annotation.)

Yet on the other hand, an Error Prone check would avoid the current boilerplate and generalize to >5 arguments.

Stephan202 · 2022-04-10T15:24:58Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

+    ImmutableList<T> after(T e1, T e2, T e3, T e4, T e5) {
+      return ImmutableList.of(e1, e2, e3, e4, e5);
+    }
+  }


Location-wise I would probably have located these templates further up in the file, as we generally start with "construction" checks. But since we're planning to auto-sort the templates lexicographically: let's roll with this :)

Based on the other files, I got the idea that we list templates with different number of params overloads at the end of a file.
(I know it is also because you auto-generated some of the templates 😄).

Nonetheless, good point about "construction" checks :).

Indeed, the auto-generated checks in AssertJTemplates are "special", and indeed at the bottom. (Additional candidates for either an Error Prone check or a Refaster extension.)

Stephan202 · 2022-04-10T15:26:27Z

...e-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListTemplates.java

+  static final class ImmutableListOf1<T> {
+    @BeforeTemplate
+    List<T> before(T e1) {
+      return List.of(e1);


We should also flag Collections.singleton* methods.

Edit: ah, there's a template for that further up. They should then be merged.

Stephan202 · 2022-04-10T15:27:33Z

...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java

@@ -242,6 +242,109 @@ private ImmutableMapTemplates() {}
    }
  }

+  /**
+   * Prefer {@link ImmutableMap#of()} over alternatives that don't communicate the immutability of
+   * the resulting list at the type level.


Suggested change

* the resulting list at the type level.

* the resulting map at the type level.

Etc.

Stephan202 · 2022-04-10T15:41:25Z

...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java

+   * Prefer {@link ImmutableMap#of(Object, Object)} over alternatives that don't communicate the
+   * immutability of the resulting list at the type level.
+   */
+  static final class ImmutableMapOf1<K, V> {


This check should be merged with PairToImmutableMap.

Stephan202 · 2022-04-10T15:42:03Z

...ne-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java

+   * Prefer {@link ImmutableMap#of()} over alternatives that don't communicate the immutability of
+   * the resulting list at the type level.
+   */
+  static final class ImmutableMapOf<K, V> {


This one should be merged with EmptyImmutableMap. Likewise for a number of other templates.

Stephan202 · 2022-04-10T16:22:03Z

@EnricSala anything else from your side? :)

EnricSala

🚀

rickie · 2022-04-12T09:42:10Z

Rebased.

EnricSala reviewed Feb 18, 2022

View reviewed changes

rickie added the WIP Work in progress label Feb 19, 2022

mussatto force-pushed the vmussatto/new-template-empty-to-immutable branch 2 times, most recently from e8ce80b to 8a3a724 Compare February 21, 2022 14:34

mussatto self-assigned this Feb 22, 2022

mussatto requested a review from rickie February 22, 2022 09:52

mussatto force-pushed the vmussatto/new-template-empty-to-immutable branch from 8a3a724 to 0b6b9f4 Compare February 22, 2022 13:39

rickie requested changes Feb 22, 2022

View reviewed changes

mussatto force-pushed the vmussatto/new-template-empty-to-immutable branch 2 times, most recently from 35dbaca to c76a46c Compare February 23, 2022 13:25

mussatto requested review from Stephan202, Badbond and rickie February 23, 2022 14:47

mussatto force-pushed the vmussatto/new-template-empty-to-immutable branch from c76a46c to b60d642 Compare February 23, 2022 16:49

rickie approved these changes Feb 24, 2022

View reviewed changes

rickie self-requested a review February 24, 2022 07:50

rickie requested changes Feb 25, 2022

View reviewed changes

Badbond requested changes Feb 25, 2022

View reviewed changes

rickie changed the title ~~Introduce new Immutables templates~~ Introduce new ImmutableCollection templates Feb 28, 2022

Badbond reviewed Feb 28, 2022

View reviewed changes

Badbond approved these changes Mar 1, 2022

View reviewed changes

mussatto requested a review from rickie March 1, 2022 13:16

rickie approved these changes Mar 2, 2022

View reviewed changes

rickie changed the title ~~Introduce new ImmutableCollection templates~~ Introduce ImmutableCollection templates Mar 2, 2022

rickie removed the WIP Work in progress label Mar 2, 2022

mussatto force-pushed the vmussatto/new-template-empty-to-immutable branch from 064f1ac to c98357f Compare April 1, 2022 13:10

rickie approved these changes Apr 4, 2022

View reviewed changes

rickie modified the milestone: 0.1.1 Apr 4, 2022

Stephan202 force-pushed the vmussatto/new-template-empty-to-immutable branch from 831be06 to d316e23 Compare April 10, 2022 14:44

Stephan202 reviewed Apr 10, 2022

View reviewed changes

Stephan202 approved these changes Apr 10, 2022

View reviewed changes

Stephan202 added this to the 0.1.0 milestone Apr 10, 2022

EnricSala approved these changes Apr 11, 2022

View reviewed changes

Vitor Mussatto and others added 15 commits April 12, 2022 11:38

Introduce new templates to use Immutables

29a031d

Adding listOf template

2904a75

Adding listOf templates for up to 5 items

be9117d

CR remarks, adding javadoc, updating tests

55a2437

Making method names consistent

eb4d03d

Removing typed tests

6630b6e

To object

1876587

CR remarks

a92c05e

List.of changes

a58862f

CR remarks

7aa801d

Immutable{Set,List}Test{Input,Output} use Integer instead of String

f4b0285

Re-add emptyList, emptySet and emptyMap templates

f8c2048

Suggestions

59ef9e9

Post-rebase fix

dbae0c1

Suggestions

920a2e1

rickie force-pushed the vmussatto/new-template-empty-to-immutable branch from ae27d22 to 920a2e1 Compare April 12, 2022 09:41

Stephan202 merged commit d7531ab into master Apr 12, 2022

Stephan202 deleted the vmussatto/new-template-empty-to-immutable branch April 12, 2022 09:51

	* Prefer {@link ImmutableList#of(Object)} over alternatives that don't communicate the
	* Prefer {@link ImmutableList#of()} over alternatives that don't communicate the

	* the resulting list at the type level.
	* the resulting map at the type level.

Introduce ImmutableCollection templates #40

Introduce ImmutableCollection templates #40

Conversation

mussatto commented Feb 18, 2022 • edited by rickie Loading

EnricSala left a comment

Choose a reason for hiding this comment

rickie commented Feb 18, 2022

rickie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Badbond Feb 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickie Feb 24, 2022 • edited Loading

Choose a reason for hiding this comment

rickie commented Feb 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Badbond left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Badbond Feb 25, 2022 • edited Loading

Choose a reason for hiding this comment

werli commented Feb 25, 2022

Badbond left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Badbond left a comment • edited by rickie Loading

Choose a reason for hiding this comment

rickie left a comment

Choose a reason for hiding this comment

Badbond commented Mar 2, 2022

rickie commented Mar 31, 2022

Badbond commented Mar 31, 2022 • edited Loading

rickie left a comment

Choose a reason for hiding this comment

Stephan202 left a comment

Choose a reason for hiding this comment

Stephan202 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stephan202 commented Apr 10, 2022

EnricSala left a comment

Choose a reason for hiding this comment

rickie commented Apr 12, 2022

Introduce `ImmutableCollection` templates #40

Introduce `ImmutableCollection` templates #40

mussatto commented Feb 18, 2022 •

edited by rickie

Loading

Badbond Feb 25, 2022 •

edited

Loading

rickie Feb 24, 2022 •

edited

Loading

Badbond Feb 25, 2022 •

edited

Loading

Badbond left a comment •

edited by rickie

Loading

Badbond commented Mar 31, 2022 •

edited

Loading