rzwitserloot / lombok.ast

Robust parser + AST for the java language.
http://projectlombok.org/
MIT License
71 stars 22 forks source link

Misc changes to Lombok.AST used by Android Lint. #8

Closed tnorbye closed 8 years ago

tnorbye commented 9 years ago

Here are the changes that went into Lombok.AST for use by Lint. You may not want all of these changes (e.g. in CL e972060 it for example stops packaging Guava and Parboiled as part of the core library), and the two version number CLs (from 0.2.1 to 0.2.2 and then to 0.2.3) are there to match what is in the libraries we use in Android Lint. But at least the ECJ related changes might be useful. There are two Guava library version changes: from 0.9 to 15.0, and from 15.0 to 17.0. Even though version 18.0 is out now I set it to 17.0 since that's what we use everywhere else (e.g. in IntelliJ 14, and in the various Android tools related libraries). I'd understand if you'd want that to be 18.0 now; we'll probably need to make that migration soon.

tnorbye commented 9 years ago

(Btw I haven't used github very much so I don't know if it's easy to selectively include/exclude commits when they're done this way; if it's a problem I can make another pull request with just the CLs that you agree with, if any.)

eighthave commented 8 years ago

@rzwitserloot any update on this pull request?

I'm in the process of figuring out how to package lombok.ast for Debian, as part of the larger effort of packaging the Android SDK and getting it all included in Debian. We're packaging lombok.ast since some of the Android tools use it. It would be best if we could package the canonical version of it, and have it work for the Androids tools too. Otherwise, for now, we'll have to package @tnorbye 's version.

You can track our progress here: https://wiki.debian.org/AndroidTools

rzwitserloot commented 8 years ago

I'll have a look this Monday or Tuesday. On Nov 20, 2015 11:16 AM, "Hans-Christoph Steiner" notifications@github.com wrote:

@rzwitserloot https://github.com/rzwitserloot any update on this pull request?

I'm in the process of figuring out how to package lombok.ast for Debian, as part of the larger effort of packaging the Android SDK and getting it all included in Debian. We're packaging lombok.ast since some of the Android tools use it. It would be best if we could package the canonical version of it, and have it work for the Androids tools too. Otherwise, for now, we'll have to package @tnorbye https://github.com/tnorbye 's version.

You can track our progress here: https://wiki.debian.org/AndroidTools

— Reply to this email directly or view it on GitHub https://github.com/rzwitserloot/lombok.ast/pull/8#issuecomment-158348367 .

tnorbye commented 8 years ago

FYI, for the Android SDK bundled version of the Lombok binary I applied a couple of buildtime tweaks to drop Guava and a few other dependencies from being packaged into the lombok library jar, since in the case of Guava it's already provided on the classpath from other Android tools, and some other parts like parboiled and cmdlinereader aren't used in our case, so it makes everything smaller for our purposes.

I didn't submit that as a patch on github since the Lombok project probably don't want it, but thought you should know in case you're trying to replicate the binaries.

On Fri, Nov 20, 2015 at 2:16 AM, Hans-Christoph Steiner < notifications@github.com> wrote:

@rzwitserloot https://github.com/rzwitserloot any update on this pull request?

I'm in the process of figuring out how to package lombok.ast for Debian, as part of the larger effort of packaging the Android SDK and getting it all included in Debian. We're packaging lombok.ast since some of the Android tools use it. It would be best if we could package the canonical version of it, and have it work for the Androids tools too. Otherwise, for now, we'll have to package @tnorbye https://github.com/tnorbye 's version.

You can track our progress here: https://wiki.debian.org/AndroidTools

— Reply to this email directly or view it on GitHub https://github.com/rzwitserloot/lombok.ast/pull/8#issuecomment-158348367 .

rspilker commented 8 years ago

@tnorbye We are willing to create two separate jars. One just contains the convertor, and the other one that also contains the parser and some dependencies. Would that help?

tnorbye commented 8 years ago

Yeah, that would be great. Which external specific dependencies (other than Guava) will the converters have?

On Sun, Nov 22, 2015 at 12:20 PM, Roel Spilker notifications@github.com wrote:

@tnorbye https://github.com/tnorbye We are willing to create two separate jars. One the just contains the convertor, and the other one that also contains the parser and some dependencies. Would that help?

— Reply to this email directly or view it on GitHub https://github.com/rzwitserloot/lombok.ast/pull/8#issuecomment-158794900 .

rspilker commented 8 years ago

@tnorbye It might depend a bit on the exact split. IIRC, you're only reading from a lombok AST that's been converted from a ecj (or javac) AST, right? Do you use the EcjTreePrinter? If not we might even consider removing Guava, since in the remaining code it is mainly used for the convenient factory methods for creating maps and lists like Lists.newArrayList()

Currently, the only runtime deps are Guava, Parboiled, cmdreader and lombok.utils. The latter might also not be necessary, since it's used for processing javadoc when printing a file we parsed ourselves using javac.

rspilker commented 8 years ago

The current holdup is the failing tests. The new ecj version has some extra bits and a some fields like trailingStarPosition and extendedDimensions. Also the Integer.MIN_VALUE is handled differently. I'm looking into those.

rspilker commented 8 years ago

I've pushed the modifications to master, including some new code to make ecj the tests work. The only think I've reverted is the deletion of the three jars. I've created issue #10 to make a smaller jar.

eighthave commented 8 years ago

Excellent, thanks! Can you tag a new release and we'll package that for Debian? Perhaps it makes sense to make the new version one past the versions Android made. I believe that would make this release be 0.2.4 then. Here are Android's public releases:

https://jcenter.bintray.com/com/android/tools/external/lombok/lombok-ast/