Extensibility #237

rharter · 2015-05-15T20:14:29Z

This is a first pass implementation of issue #202, providing two options for extending AutoValue through the use of compile time plugins.

The first method, is by having your extension create an intermediate class between the abstract template class and the final AutoValue implementation of the class. The provides a good mechanism for adding functionality to an AutoValue class, while still keeping the implementation segregated.

The second method is to alter the final AutoValue class itself. This was required to support Android Parcelable implementations, as Parcels are required to have static factory objects, and this isn't achievable via inheritance. Using this method, extensions have to option of adding imports, interfaces and code to the resulting AutoValue class implementation. You can see a working example of one such plugin here.

You can view a test Android app that makes use of the auto-parcel extension here.

googlebot · 2015-05-15T20:14:30Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

rharter · 2015-05-15T20:16:49Z

I signed it!

googlebot · 2015-05-15T20:16:51Z

CLAs look good, thanks!

eamonnmcmanus · 2015-05-15T20:49:26Z

I think the Travis failure indicates that you need to merge in recent changes.

eamonnmcmanus · 2015-05-15T20:54:35Z

value/src/main/java/com/google/auto/value/AutoValueExtension.java

+ */
+public interface AutoValueExtension {
+
+  public interface Context {


The public modifier is redundant.

Conflicts: value/pom.xml

rharter · 2015-07-02T22:04:20Z

@eamonnmcmanus I think this is in a good state to check out. To see an example of a working extension implementation, check out my auto-parcel code.

I'm still having a problem with my Parcelable extension in that the annotated class has to implement the Parcelable interface, but AutoValue takes the int describeContents() method from the interface to be something that it should generate. I understand the need to include inherited abstract methods in the properties list, but I'm not sure how to tell AutoValue that it doesn't need to worry about this particular interface method with no parameters that returns an int. For now, I'm working around this by simply implementing the super simple version in my annotated class. The generated class also generates an implementation.

eamonnmcmanus · 2015-07-08T00:55:36Z

value/src/main/java/com/google/auto/value/AutoValueExtension.java

+ * An AutoValueExtension allows for extra functionality to be created during the generation
+ * of an AutoValue class.
+ *
+ * Extensions are discovered at compile time using the {@link java.util.ServiceLoader} APIs,


Can you insert a <p> at the start of the first line of each paragraph here (except the first)? Like this:

* <p>Extensions are discovered...

rharter · 2015-07-31T14:17:10Z

Sorry, I guess I didn't comment when those were resolved, but they should all be resolved in the commits from that same day.

eamonnmcmanus · 2015-07-31T14:21:46Z

OK, I'll take a look later today. Sorry about the misunderstanding.

eamonnmcmanus · 2015-07-31T23:28:32Z

value/com/example/annotations/AutoAnnotation_Erroneous_newEmpty.java

@@ -0,0 +1,40 @@
+


What is this?

eamonnmcmanus · 2015-07-31T23:53:59Z

I'm expecting @cgruber to make a PR shortly that will export Google's internal changes. Merging with those changes won't be too hard, I hope, and after that I think we should be good to go.

rharter · 2015-08-01T12:53:22Z

Changes are in. I think that erroneous empty file is the result of a test and was added through a careless git add .. Seems wrong that tests would create stuff outside the target dir.

eamonnmcmanus · 2015-08-04T21:07:55Z

Regarding the erroneous file, it must have been created by AutoAnnotationCompilationTest. That creates the file using File.createTempFile — is it possible that your system creates temporary files in the current directory??

rharter · 2015-08-05T15:25:04Z

That's certainly possible. My system is a Mac, I haven't tested the default File.createTempFile functionality there.

eamonnmcmanus · 2015-08-05T15:32:59Z

Meanwhile we're running into issues merging in the latest Google changes. Those changes are unfortunately more extensive than I thought, so you might be in for a bit of merging work when they go in. I'm reluctant to merge this PR first, though, for fear of making that other merge even worse.

eamonnmcmanus · 2015-08-05T18:27:26Z

Could you resolve the merge conflicts? Then I'll have another look. Hopefully Travis issue travis-ci/travis-ci#4621 will be solved soon so we'll be able to get a correct indication of test status.

Conflicts: value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java value/src/main/java/com/google/auto/value/processor/BuilderSpec.java

rharter · 2015-08-05T20:38:07Z

Merged and passing tests locally.

rharter · 2015-08-06T01:26:09Z

@eamonnmcmanus Does that bug cause Travis to hang? Shows that it's still building after 5 hours.

eamonnmcmanus · 2015-08-06T01:34:44Z

Seems Travis has been very slow of late. I waited about as long as that yesterday for a build.

rharter · 2015-08-07T15:14:42Z

Looks like Travis is back up and all checks pass.

Add an initial version of the Extensibility API to AutoValue.

eamonnmcmanus · 2015-08-07T15:42:05Z

Thanks for your patience with this!

frankiesardo · 2015-08-09T19:04:16Z

Thank you both this CR is well worth the wait!

eamonnmcmanus · 2015-08-10T14:27:13Z

@frankiesardo are you able to obtain the same results with this as with your @AutoParcelable fork?

frankiesardo · 2015-08-11T20:32:51Z

I pushed a branch that uses the new Extension api. So far so good.

The only problem is related to the Parcelable interface method int describeComponents() as @rharter already pointed out here rharter/auto-value-parcel@bfaba14#commitcomment-12614380).

Forcing the user to implement it, even if it's a simple method, it's an annoyance. Not sure if AutoValue can do something about it.

frankiesardo · 2015-08-11T20:36:04Z

In #202 you mention briefly the possibility for an extension to claim a list of abstract methods. That would be helpful in this case.

rharter · 2015-08-11T22:00:08Z

I'm working on that.

On Tue, Aug 11, 2015, 3:36 PM Frankie Sardo notifications@github.com
wrote:

In #202 #202 you mention briefly
the possibility for an extension to claim a list of abstract methods. That
would be helpful in this case.

—
Reply to this email directly or view it on GitHub
#237 (comment).

Add an initial version of the Extensibility API to AutoValue. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=100543793

*** Reason for rollback *** Google's internal test rig detected substantial numbers of failures, and so this commit was auto-rolled-back. *** Original change description *** Merge pull request #237 from rharter/extensibility Add an initial version of the Extensibility API to AutoValue. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=100545729

*** Reason for rollback *** Re-attempt original commit, but with internal build fixes to avoid sending the SPI code to android builds, where javac/javax types don't exist. *** Original change description *** Automated g4 rollback of changelist 100543793. *** Reason for rollback *** Many tests lost their lives securing this commit. *** Original change description *** Merge pull request #237 from rharter/extensibility Add an initial version of the Extensibility API to AutoValue. *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=101824430

rharter added 7 commits May 1, 2015 16:52

Adds basic extension support.

6be525b

Updates version.

eead9f2

Adds extension tests.

ad0f55f

Fixes issue causing base autovalue class name to have dollar prepend.

10ea0d1

adds more fixes.

3acd541

Moves the service loader to init.

16910e5

Removes test logging.

899c470

googlebot added the cla: no label May 15, 2015

googlebot added cla: yes and removed cla: no labels May 15, 2015

eamonnmcmanus reviewed May 15, 2015
View reviewed changes

rharter added 7 commits June 4, 2015 10:36

Updates extensions to match requirements.

42ab1d9

Merge branch 'master' into extensibility

b401ce9

Conflicts: value/pom.xml

Adds some documentation and support for final/abstract extensions.

05fb4ea

Adds documentation for the AutoValueExtension.

ba6df28

Merge branch 'master' into extensibility

81fbc06

Fixes test error after changing AutoValueExtension API.

155c8f0

fixes tests.

efba4ce

eamonnmcmanus reviewed Jul 8, 2015
View reviewed changes

eamonnmcmanus reviewed Jul 31, 2015
View reviewed changes

value/com/example/annotations/AutoAnnotation_Erroneous_newEmpty.java

@@ -0,0 +1,40 @@

Copy link

Member

eamonnmcmanus Jul 31, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this?

Removed test file and cleans up some typos in comments.

7881674

Merge branch 'master' into extensibility

402dbc0

Conflicts: value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java value/src/main/java/com/google/auto/value/processor/BuilderSpec.java

eamonnmcmanus added a commit that referenced this pull request Aug 7, 2015

Merge pull request #237 from rharter/extensibility

068717f

Add an initial version of the Extensibility API to AutoValue.

eamonnmcmanus merged commit 068717f into google:master Aug 7, 2015

felipecsl mentioned this pull request Aug 12, 2015

Add Android Parcelable support #87

Closed

cgruber mentioned this pull request Aug 15, 2015

Extensibility API for AutoValue #202

Closed

cgruber added a commit that referenced this pull request Sep 11, 2015

Merge pull request #237 from rharter/extensibility

ecc5570

Add an initial version of the Extensibility API to AutoValue. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=100543793

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extensibility #237

Extensibility #237

rharter commented May 15, 2015

googlebot commented May 15, 2015

rharter commented May 15, 2015

googlebot commented May 15, 2015

eamonnmcmanus commented May 15, 2015

eamonnmcmanus May 15, 2015

rharter commented Jul 2, 2015

eamonnmcmanus Jul 8, 2015

rharter commented Jul 31, 2015

eamonnmcmanus commented Jul 31, 2015

eamonnmcmanus Jul 31, 2015

eamonnmcmanus commented Jul 31, 2015

rharter commented Aug 1, 2015

eamonnmcmanus commented Aug 4, 2015

rharter commented Aug 5, 2015

eamonnmcmanus commented Aug 5, 2015

eamonnmcmanus commented Aug 5, 2015

rharter commented Aug 5, 2015

rharter commented Aug 6, 2015

eamonnmcmanus commented Aug 6, 2015

rharter commented Aug 7, 2015

eamonnmcmanus commented Aug 7, 2015

frankiesardo commented Aug 9, 2015

eamonnmcmanus commented Aug 10, 2015

frankiesardo commented Aug 11, 2015

frankiesardo commented Aug 11, 2015

rharter commented Aug 11, 2015

Extensibility #237

Extensibility #237

Conversation

rharter commented May 15, 2015

googlebot commented May 15, 2015

rharter commented May 15, 2015

googlebot commented May 15, 2015

eamonnmcmanus commented May 15, 2015

eamonnmcmanus May 15, 2015

Choose a reason for hiding this comment

rharter commented Jul 2, 2015

eamonnmcmanus Jul 8, 2015

Choose a reason for hiding this comment

rharter commented Jul 31, 2015

eamonnmcmanus commented Jul 31, 2015

eamonnmcmanus Jul 31, 2015

Choose a reason for hiding this comment

eamonnmcmanus commented Jul 31, 2015

rharter commented Aug 1, 2015

eamonnmcmanus commented Aug 4, 2015

rharter commented Aug 5, 2015

eamonnmcmanus commented Aug 5, 2015

eamonnmcmanus commented Aug 5, 2015

rharter commented Aug 5, 2015

rharter commented Aug 6, 2015

eamonnmcmanus commented Aug 6, 2015

rharter commented Aug 7, 2015

eamonnmcmanus commented Aug 7, 2015

frankiesardo commented Aug 9, 2015

eamonnmcmanus commented Aug 10, 2015

frankiesardo commented Aug 11, 2015

frankiesardo commented Aug 11, 2015

rharter commented Aug 11, 2015