Provide explicit support for Groovy source code about spotless HOT 22 CLOSED

Hi. I am unfortunately also not a groovy expert. Actually I just wanted to know more about gradle, so I also had a look at Groovy. When I started to write my first dummy plugins, I searched for a better way to format code automatically (still used ANT...), so I found spotless and try to learn from its source how to write gradle plugins.
Since I found the p2.asmaven provided by @nedtwigg fascinating, I started to write a formatter step using the GrEclipse plugin.
Anyhow, I am cleaning up my prove-of-concept and committing it step by step to my fork.
Maybe (with some help ;-) ) I can manage to push it to a usable state within a few week(end)s.

from spotless.

I just added the skeleton of a Groovy DSL on the branch feature/groovy. I don't like Groovy, and I don't know anything about it, so this is as far as I can take it personally. CONTRIBUTING.md includes instructions for setting up within Eclipse and running all the tests. I'm happy to help with whatever you need 👍

from spotless.

Removed the feature/groovy branch, as it had gotten out-of-date. Just look at JavaExtension or FreshMarkExtension to see an updated example of adding a file-specific DSL.

from spotless.

@fvgh Ooh, nice! I'm glad someone's decided to have a go at this. Looking forward to anything you're able to dig up/code up. (No pressure!) 😉

Maybe (with some help ;-) ) I can manage to push it to a usable state within a few week(end)s.

Yes, please, if you do need any help, especially with integrating with Spotless, do let us know; I'm sure @nedtwigg or I can at least point you in the right direction if you get stuck. 😃

from spotless.

Great! We'll be happy to help you get your stuff merged, @fvgh. Have a look at the _ext/eclipse-jdt folder to see how we bundle jdt into a single jar for consumption by spotless. We'd probably need something similar for groovy-eclipse, I imagine.

from spotless.

I provided a proposal for the formatter step implementation, where I made a few decisions which can be discussed/changed.

1. I am using the latest GrEclipse release version which is actually from 2014. Hence for example only Groovy 2.3 is supported. There is recently quite some progress on the project. Groovy 2.4 is available on the snapshot. So I would propose to stick with the release for now, but I can also provide a snapshot version. It should only be a reconfiguration of the properties.
2. I am using the groovy compiler coming with GrEclipse in stead of the one of the local gradle installation.
3. The Eclipse for groovy formatter is independent from the one of Eclipse -JDT.
4. I accomplished the patching of the JDT required by GrEclipse by overriding the class files in the fat JAR, instead of using multiple JARs and specific load order.
5. I had to rewrite two GrEclipse classes myself to avoid further Eclipse dependencies and undetected errors
6. GrEclipse does not provide a release source code with p2. Since their patching of the JDT and my patching of their code makes it hard to debug the formatter step, I decided to provide also the original Eclipse/GrEclipse sources with the source bundle, using the same overriding principal on the java files, I used already for the classes.

So in the end the JAR is independent for the gradle or Eclipse version and can be debugged easily. Drawback is the size.

• JAR: 18 MB
• SOURCES: 11MB

We could discuss whether it would be better to share for example the Eclipse classes.

@nedtwigg I think that I finally understood the p2AsMaven and Eclipse plugin. Could you check my final changes?

from spotless.

The way you are using p2AsMaven and eclipse looks good to me. It is non-standard, but it looks like you have found a good solution to the problem of patches to JDT required by GrEclipse.

I don't think the 18MB filesize is a problem. The greclipse jar will only get downloaded by people who want the Groovy formatter. Good work on including src, I don't think we did that for eclipse-jdt plugin, and it's very nice to have.

Here's the things left to do, if I understand correctly

• I need to upload the greclipse bundle to jcenter / mavencentral (can use mavenlocal for dev in the meantime)
• Add a com.diffplug.spotless.extra.groovy package to lib-extra, and some integration tests there.
• Add groovy support to plugin-gradle along with some docs

Since this work includes refactoring some existing functionality, I'd like to look at it closely and merge it in steps:

1. A PR with the changes to FileSignature
2. A PR which introduces ExtFormatterState
3. A PR with GrEclipse

I'm gonna be slammed from now til Devoxx around 3/21. If you can cherry-pick your commits into these easy-to-review chunks, @jbduncan and I can merge them one at a time. If you're too busy, I'll be able to do this after 3/21.

from spotless.

I was planing to deliver the missing code+tests+docs as well. As you pointed out, the work can be split into work-packages. So I was planing to deliver the following packages as soon as the the previous once are agreed.

1. com.diffplug.spotless.extra.groovy + tests: Tests for now linked to mavenLocal Provisioner and require a refactoring.

2. GroovyExtension for plugin-gradle I also have, but really just as a draft. Test are completely missing.

3. Documentation of GroovyExtenstion

4. Integration of GroovyExtension into spotlessSelf.gradle (not for a groovy project of course, but for the gradle files)

My personal goal is just to get a good overview of gradle. So all these packages are proposals and I am not offended if you reject or correct them. Especially the fourth point is of course just optional and it is your decision whether you want to take the overhead into your build process. I just will implement it as a proof of concept.

Two remaining issues on the GrEclipse:

• I still get some [p2.mirror] Unable to satisfy dependency... , but only on packages I do not require ( toolingorg.eclipse... ). It seems to me some platform dependent stuff, I am not interested in at the moment. Is it OK for you if I ignore them, or can you point me in a direction how to get rid of theses build messages?

• I put myself into the developer section. Just wanted to show my willingness to maintain that extension (and provide the groovy 2.4 support and too take the blame 😄 ). Let me now what you would like to see there (you, me, both of us, ...).

from spotless.

Sounds great on all points! On integration of spotlessSelf.gradle, I like to eat as much dogfood as I can, so I'm all for it. For adding new GrEclipse functionality, you've got a very wide berth - you need it, you're building it, I'll mostly defer to you. For modifying and refactoring our existing production-tested code, I'll be quite a stickler (as we've discussed here and in your clone). So long as we do the refactor in small steps, I think we'll be able to merge the changes you've proposed quickly.

Re: Unable to satisfy dependency... I think it's fine to ignore. I get them too, and I wish I could figure out how to suppress / fix them, but no harm is done.

Re: developer section, thanks very much for taking it on! Once we've merged a few PR's together, I'll add you as a committer as well. I'd still like to go through the PR code review process, but between me, @jbduncan, and now you, we can have our "bus factor" all the way to 3!

from spotless.

Thanks for the confidence 😃
I issued my first PR. Have to call it a day.

from spotless.

Hi @nedtwigg and @fvgh, sorry for being quiet lately. I just wanted to say that I am most definitely interested in properly poring over your changes @fvgh, but I'm not sure when I'll have time to review it all properly, so would you be willing to allow me a few days, say a week at most, to review them and get back?

If it's not possible for you to wait that long, or if it takes me longer than a week, then please feel free to continue collaborating amongst yourselves, and I'll do a post-commit review if or when I can. :)

from spotless.

(No, sorry, please do continue collaborating regardless of whether I do get around to reviewing this!)

from spotless.

@jbduncan I look forward to your feedback, but I think the thread is a little hard to follow from just the commit history of the experimental branch. @fvgh has a convincing story to tell through a series of PRs which will better explain his idea. Whenever a PR is uncontroversial, I'm going to merge it right away. When something has a broader impact, I'll be sure to let it sit for 12-24 hrs before merging so that you and anyone else who'd like to leave comment has a chance. He's got a nice fat feature for us, and requires only a modest amount of refactoring to make it work cleanly - I think it will be fairly easy to review after he's had a chance to submit in bite-sized chunks.

from spotless.

@jbduncan There is no hurry. Most the initial PRs are anyway independent from each other and I can continue with the missing implementation on a dedicated branch.
This is the story @nedtwigg is referring to, but I will also try to provide self-contained information with each PR.

from spotless.

This will be deployed in a few minutes in 3.3.0-SNAPSHOT: https://oss.sonatype.org/content/repositories/snapshots/com/diffplug/spotless/spotless-plugin-gradle/

Let's make sure it works like the docs say, then I'll turn the release crank.

from spotless.

I played briefly with the example provided. If you omit the excludeJava you get of course errors since googleJavaFormat and greclipse are not compatible (using default configurations) and apply different rules on a java file. But I think that is OK, since a sanity check of the spotless configuration is not possible due to its flexibility.
I just played briefly with it and have not yet my own project ready where I want to use greclipse. So in the end the status "incubation" would be describe the situation best. For the logic I added to the plugin, I provided unit-tests, so it should be principally sane.
My biggest concern is more on the instabilities of the groovy-eclipse formatter itself, which is outside the scope of spotless. Here I need another look whether they really can be tackled by paddedCell.

from spotless.

Published in 3.3.0. Feel free to reopen or open new issues with subtasks for this 👍

from spotless.

Thanks for adding Groovy support! I've created junit-team/junit5#843 to use it for JUnit. The task is "up-for-grabs", pull requests are welcome! 😉

from spotless.

Thanks @marcphilipp for "reminding" 😉 me that I should add the Groovy support for JUnit. I am afraid, looking briefly at it today, I found two issues that require enhancements. Should be quickly done.

from spotless.

@nedtwigg What is your schedule for 3.4.0? I may need 2 changes:

• Fix check whether Groovy plugin is available (will provide PR asap, change works already)
• Either a change of exception treatment in GrEclipse or the support of the current GrEclipse Snapshot version, but I have to have another look.

I still have too look on the latter issue, but have no time today. Can give you at least a final estimation this week-end. OK for you?

from spotless.

I'm happy to release 3.4.0 anytime you would like, no schedule pressure from me. Btw, travis builds are broken for now. This security bug meant that I had to rotate all my secrets, and I haven't gotten around to updating all my opensource projects yet. I'll try to do that this evening.

from spotless.

Travis is working again.

from spotless.