Use @AutoValue instead of manual POJO generation

[dflemstr]

This refactors the processor to delegate generation of POJO classes to the AutoValue library.

This is of course a trade-off; it is good because there is less code in this repo and less need to test corner cases, but it means that the classes of data enum variants need to have package-private constructors.

Since AutoValue has a built-in linter for checking the soundness of POJOs, as part of this PR I noticed two problems:

• The "varargs" example actually creates a mutable POJO because it exposes an array directly. A warning about this behavior had to be suppressed manually as part of this PR.
• There is an example with a generic varargs constructor. This is very dangerous to support since the javac-generated synthetic trampoline method for this purpose may cause ClassCastExceptions at runtime with surprising circumstances. AutoValue flat out prohibits this behavior by refusing to compile. I chose to @Ignore that test until further discussion.

[pettermahlen]

Thanks for the PR! I think there are a couple of different things here:

1. It's not a given that mandating using auto-value as an annotation processor is a good thing; some users of this processor might not want to be forced to also use auto-value.
2. I wonder what the impact of having auto-value on the classpath is. Gradle (which is the most common build tool in Android) deals different with annotation processors than Maven, and, for instance, disallows or disencourages having actual annotation processor on the main classpath. It feels like we'd have to do some research to guarantee that there is no negative impact from that.
3. Regarding the non-generic varargs problem, I think there are a few options:
   1. Do safe copying on instantiation and on reads. This should include at least a compiler warning, I think, because it's unlikely to be what the user intended.
   2. Disallow it completely.
   3. Convert varargs arrays into some collection type, probably List.
4. Regarding the generic varargs arrays and ClassCastExceptions, could you provide a pointer to some relevant documentation about how the problem manifests itself?

It feels like a good idea could be to disallow varargs parameters, mandating the use of collection types instead, but I would like to know a little more before I make up my mind.

[dflemstr]

@pettermahlen

1. The other annotation processor is an implementation detail and won't be visible to the user (except of course the generated classes being on the class path)
2. This uses annotation processor composition and this processor depends on the other one as a Maven dependency. So from the users point of view there is no difference in how the processor gets included. See the integration test which I didn't change in this PR.
3. If we don't want to depend on Guava and want to expose an "immutable" list, I guess the only standard library option I can think of is to use ArrayList + UnmodifiableList, which incurs a lot of allocation. Maybe prohibiting this leads to the user having to make a conscious choice where they can choose ImmutableList (or Set etc)? Also, the Google pattern seems to be to use types in the getters that indicate that the returned type is immutable (e.g. returning ImmutableList instead of List) as discussed here and here
4. The most concise reference that I found is https://youtu.be/wbp-3BJWsU8 at 25:40. It's also in the book Java Puzzlers but I couldn't find an online legal version of that. The video showcases the contravariant case (calling a generic varargs method) but the the covariant case (something returns a generic array) causes similar problems.

[pettermahlen]

So, we've discussed this a bit, and what we want to do is:

1. Remove support for varargs to solve the issues you mention. The rationale is that even though DataEnum cannot provide any immutability guarantees (since people are free to use mutable types as parameters), using arrays is always going to go against the desire to have DataEnum cases be immutable.
2. Not use AutoValue for the generated types. The rationale is that we want to keep a degree of freedom in how the internals are built, as we think it's likely we'll want to explore some kind of extension mechanism. Using AutoValue now may be the wrong abstraction. It may make sense to do later.

It's great that you brought our attention to these problems!

[pettermahlen]

Created #4 to remove varargs support.

[dflemstr]

It might be interesting to look at the @AutoValue extension mechanisms (embracing NIH) but it is of course your prerogative to evaluate whether it suits your desired level of abstraction!

[dflemstr]

(see http://ryanharter.com/blog/2016/05/16/autovalue-extensions/ for example)

[pettermahlen]

Yes, we have been looking at autovalue-extensions, and part of our future plan is to support something similar in dataenum. Currently, our gut feel is that we'll need to plug that in deeper inside DataEnum than they allow out of the box, and that such changes might be easier with code that we own fully, but we will definitely take a closer look at this PR when we get to looking into that.