New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Convert scalafmt integration to use a compile-only sourceset #1283

Merged

nedtwigg merged 12 commits into diffplug:main from mdedetrich:scalafmt-compile-time-only-sourceset

Aug 23, 2022

Contributor

mdedetrich commented Aug 22, 2022 •

edited

Convert scalafmt integration to use a compile-only sourceset (Use custom source sets and compile-only dependencies instead of reflection #524)
- Just like other PRs that convert from reflection to using a compile-only source set, this PR also removes code related to old versions of Scalafmt. This is also necessary due to the fact that scalafmt's API is completely different from major versions so spotless wouldn't even work with scalafmt versions older than 3.
Possible solution to Force scalaVersion when using scala scalafmt extension #1273 (still need to test)

mdedetrich force-pushed the scalafmt-compile-time-only-sourceset branch from fcb77ca to 848061e Compare

August 22, 2022 11:25

mdedetrich mentioned this pull request

Force scalaVersion when using scala scalafmt extension #1273

Closed

mdedetrich commented

View reviewed changes

testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java Outdated Show resolved Hide resolved

mdedetrich commented

View reviewed changes

lib/src/scalafmt/java/com/diffplug/spotless/glue/scalafmt/ScalafmtFormatterFunc.java

+              	public String apply(String input) throws Exception {
+              		ScalafmtConfig config;
+              		if (configSignature.files().isEmpty()) {
+              			// Note that reflection is used here only because Scalafmt has a method called

Contributor Author

mdedetrich Aug 22, 2022

One way around this may be that instead of defining ScalafmtFormatterFunc as a Java source file, instead we can define it as a Scala source file (which with a basic class Java has no issues calling). Not sure if this is possible or even desirable

Member

nedtwigg Aug 23, 2022

I'd be okay putting Scala into our codebase if it was really useful for some task, but this is small enough that I'd prefer the reflection. Note the little refactor I did in a75877c.

mdedetrich force-pushed the scalafmt-compile-time-only-sourceset branch 2 times, most recently from ea185c5 to d0e5667 Compare

August 22, 2022 11:40

mdedetrich commented

View reviewed changes

lib/src/main/java/com/diffplug/spotless/scala/ScalaFmtStep.java Show resolved Hide resolved

mdedetrich force-pushed the scalafmt-compile-time-only-sourceset branch from d0e5667 to 966f33d Compare

August 22, 2022 11:43


          Convert scalafmt integration to use a compile-only sourceset

781eb68

mdedetrich force-pushed the scalafmt-compile-time-only-sourceset branch from 966f33d to 781eb68 Compare

August 22, 2022 11:57

mdedetrich commented

View reviewed changes

testlib/src/main/resources/scala/scalafmt/scalafmt.conf Show resolved Hide resolved

mdedetrich commented

View reviewed changes

lib/src/scalafmt/java/com/diffplug/spotless/glue/scalafmt/ScalafmtFormatterFunc.java Show resolved Hide resolved

mdedetrich commented

View reviewed changes

lib/src/main/java/com/diffplug/spotless/scala/ScalaFmtStep.java Outdated Show resolved Hide resolved

Member

nedtwigg commented Aug 22, 2022 •

edited

NBD, but please push new commits instead of force-pushing. Easier to review only what has changed that way :)

Member

nedtwigg commented Aug 22, 2022

Also thanks for this very good PR, it will definitely get merged :D

mdedetrich and others added 2 commits

August 23, 2022 01:06


          Allow configuration of scalaMajorVersion

137392f


          Merge branch 'main' into scalafmt-compile-time-only-sourceset

ecece44

Contributor Author

mdedetrich commented Aug 22, 2022

So I just added a commit which implements the scalaMajorVersion configuration as well as resolving conflicts from master branch. Let me know if anything else is needed!

mdedetrich commented

View reviewed changes

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/ScalaExtension.java Show resolved Hide resolved

mdedetrich added 3 commits

August 23, 2022 01:16


          Resolve formatting issues

abe0be9


          More formatting issues

df59712


          Fix accidental removal of addStep

677415e

Contributor Author

mdedetrich commented Aug 22, 2022

CI is passing now, forgot about the newly formatted files and accidentally deleted a line which I didn't realize.

mdedetrich requested a review from nedtwigg

August 22, 2022 23:56

mdedetrich added 2 commits

August 23, 2022 11:40


          Add documentation for majorScalaVersion

30d72d8


          Update scalafmt versions in documentation

872424a

Contributor Author

mdedetrich commented Aug 23, 2022 •

edited

Just pushed some commits regarding updates/changes in the documentation.

nedtwigg mentioned this pull request

Use custom source sets and compile-only dependencies instead of reflection #524

Closed

nedtwigg added 2 commits

August 23, 2022 12:32


          Add info about changes to scalafmt default version.

24ca9ec


          Update the tool-specific changelogs.

735c7b5


          Massive speedup. FormatterFunc is instantiated lazily and is not invo…

a75877c

…lved in serialization/equality, so good to eagerly initialize in the constructor.

Member

nedtwigg commented Aug 23, 2022

Thanks very much! I'm ready to merge this when you are. My planned commit message:

Add support for setting scala version for scalafmt, and also convert to compile-time (#1283 fixes #1273 helps with #524)

Contributor Author

mdedetrich commented Aug 23, 2022

Perfect, happy with the commit message so feel free to merge and thanks for being responsive on the PR!


          Fix test.

e9d3150

nedtwigg enabled auto-merge

August 23, 2022 20:03

nedtwigg approved these changes

View reviewed changes

nedtwigg merged commit 24b8d3a into diffplug:main

mdedetrich deleted the scalafmt-compile-time-only-sourceset branch

August 23, 2022 21:25

Member

nedtwigg commented Aug 23, 2022

Published in plugin-gradle 6.10.0 and plugin-maven 2.25.0.

mdedetrich mentioned this pull request

Support Scalafix #591

Closed

mdedetrich mentioned this pull request

gradle-scala plugin overrides isolated gradle provisioner scala version in spotless gradle/gradle#21502

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment