Skip to content

scalafmt: version from the config file is not dynamically used #2460

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.

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

Open
gaeljw opened this issue Apr 4, 2025 · 4 comments
Open

scalafmt: version from the config file is not dynamically used #2460

gaeljw opened this issue Apr 4, 2025 · 4 comments

Comments

@gaeljw
Copy link
Contributor

gaeljw commented Apr 4, 2025

Scalafmt binaries (JARs) are supposed to parse the version declared in the config file and use this version to dynamically download the appropriate scalafmt release to then run.

This behaviour works fine in with sbt or IDEs but it doesn't seem to work with Spotless: the version declared in the config file (.scalafmt.conf) is not used.

Reproduction case

Have the following .scalafmt.conf:

version = "3.2.0"
runner.dialect = scala213

unindentTopLevelOperators  = true

And in the pom.xml (Maven):

<plugin>
        <groupId>com.diffplug.spotless</groupId>
        <artifactId>spotless-maven-plugin</artifactId>
        <version>2.44.3</version>
        <configuration>
            <scala>
              <scalafmt>
                  <version>3.2.0</version>
                  <file>${project.basedir}/.scalafmt.conf</file>
                  <scalaMajorVersion>2.13</scalaMajorVersion>
              </scalafmt>
            </scala>
        </configuration>
      </plugin>

Then, running mvn spotless:apply works fine.

Now, change the version in the POM to 3.4.0 which doesn't support anymore the unindentTopLevelOperators key, mvn spotless:apply fails:

[ERROR] Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.44.3:apply (default-cli) on project cop-full-export-spark-app: Unable to format file /home/xxx/Yyy.scala: java.lang.reflect.InvocationTargetException: <input>:17:0 error: found option 'unindentTopLevelOperators' which wasn't expected, or isn't valid in this context.
[ERROR] unindentTopLevelOperators  = true
[ERROR] ^

Whereas it should have run in "3.2.0 mode" because it's the version specified in the config file.

References

https://scalameta.org/scalafmt/docs/configuration.html#version

The version parameter specifies the release of the formatter to be used. If the version requested is different from the version of the installed formatter, the correct release will be downloaded dynamically.

Possible fixes

Scalafmt uses a "scalafmt-dynamic" JAR to do this. My guess is that Spotless doesn't use it.

They document how to use it from Java: https://scalameta.org/scalafmt/docs/installation.html#calling-from-java

Additional info

I guess the other way around may also happen: in the config file has a newer version than the one declared in the POM.

IMHO, in medium term, the POM version should be removed (it's already optional) and it should aways use the default version hardcoded in Spotless to then dynamically download the version mentioned in the config file.

@nedtwigg
Copy link
Member

nedtwigg commented Apr 8, 2025

Happy to take any PRs that improve the situation, but I lean towards the idea that it should be an error if the versions disagree. Of all fixes, the easiest to implement is probably just a precondition check that gives a nice error message if the version in the config file exists and is different than the version Spotless is using.

@gaeljw
Copy link
Contributor Author

gaeljw commented Apr 8, 2025

@nedtwigg indeed, sounds like a reasonable way to avoid the error and easy and future-proof to implement.

I'll give it a try then :)

@gaeljw
Copy link
Contributor Author

gaeljw commented Apr 9, 2025

I'm not familiar with the codebase, should the check be implemented in ScalaFmtStep (main module) or rather ScalafmtFormatterFunc (scalafmt module)?

I was going for ScalaFmtStep but this would mean either parsing the config file without Scalafmt library or adding Scalafmt library as a dependency of the main module which is likely not something we want to do I guess.

@nedtwigg
Copy link
Member

nedtwigg commented Apr 9, 2025

Either would work fine.

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

No branches or pull requests

2 participants