Skip to content
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

Update to Scalafmt 3.8.3 #1631

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Update to Scalafmt 3.8.3 #1631

wants to merge 3 commits into from

Commits on Oct 25, 2024

  1. Update to Scalafmt 3.8.3 

    Ensures that all Scalafmt targets succeed under every supported Scala
version. Part of bazelbuild#1482.

Scala 2.11 remains on Scalafmt 2.7.5, since there's not a more recent
compatible version.

Between 3.0.0 and 3.8.3, Scalafmt's `FileOps` moved packages,
`Config.fromHoconFile` moved to `ScalafmtConfig`, and the methods now
take a `java.nio.file.Path` parameter. As a result, this required
extracting a thin `ScalafmtAdapter` object, with one implementation for
Scala 2.11 and Scalafmt 2.7.5, and one for Scalafmt 3.8.3.

This change also adds the file path to the `Scalafmt.format()` call,
allowing error messages to show the actual file path instead of
`<input>`.

Removed the `verticalMultiline.newlineBeforeImplicitKW = true` and
`unindentTopLevelOperators = false` options from `.scalafmt.conf`.
They don't appear to be available in Scalafmt 3.8.3.

Also removes some `@io_bazel_rules_scala` references to make the
internal implementation less dependendent on that name. This will allow
Bzlmod clients to use `rules_scala` in a `bazel_dep()` without setting
`repo_name = "io_bazel_rules_scala"`.

---

Scalafmt 3.0.0 works with the current default Scala version 2.12.19,
but breaks under Scala 2.13.14:

```txt
$ bazel test --repo_env=SCALA_VERSION=2.13.14 //test/scalafmt/...

ERROR: .../test/scalafmt/BUILD:49:20:
  ScalaFmt test/scalafmt/test/scalafmt/unformatted/unformatted-test.scala.fmt.output
  failed: (Exit 1): scalafmt failed: error executing command
  (from target //test/scalafmt:unformatted-test)
  bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/scala/scalafmt/scalafmt
  '--jvm_flag=-Dfile.encoding=UTF-8' ... (remaining 1 argument skipped)

java.util.NoSuchElementException: last of empty IndexedSeq
    at scala.collection.IndexedSeqOps.last(IndexedSeq.scala:110)
    at scala.collection.IndexedSeqOps.last$(IndexedSeq.scala:105)
    at scala.meta.tokens.Tokens.last(Tokens.scala:29)
    at org.scalafmt.internal.Router.$anonfun$getSplitsImpl$90(Router.scala:707)
    at scala.Option.map(Option.scala:242)
    at org.scalafmt.internal.Router.getSplitsImpl(Router.scala:706)
    at org.scalafmt.internal.Router.getSplits(Router.scala:2314)
    at org.scalafmt.internal.BestFirstSearch.$anonfun$routes$1(BestFirstSearch.scala:38)
    [ ...snip... ]
```

This matches:

- scala/community-build#1680

Which mentions apparent fixes in:

- scalameta/scalameta#3235
- scalameta/scalafmt#3581

So the fix was to upgrade the Scalafmt version. That said, I held its
Scalameta dependencies to 4.9.9 per the following link, even though
4.10.2 is out now.

- https://mvnrepository.com/artifact/org.scalameta/scalafmt-core_2.13/3.8.3

---

This change updates Scalameta to version 4.9.9 because between 4.9.9 and
4.10.2, Scalafmt breaks Scala 3 file formatting by unindenting some code
that it shouldn't. Or, our usage of it breaks somehow; I can't find any
open or closed issues in the Scalameta project that matches what happes
in rules_scala. (Perhaps I can file one eventually.)

- https://github.com/scalameta/scalafmt/issues

The solution was to keep the Scalameta dependencies at 4.9.9. FWIW, this
was one of the most time consuming bugs to pinpoint and rectify in the
entire Bzlmodification process.

This was the failing command inside `test_cross_version`:

```txt
$ bazel run //scalafmt:unformatted-test3.format-test

INFO: Analyzed target //scalafmt:unformatted-test3.format-test
  (0 packages loaded, 0 targets configured).
INFO: From ScalaFmt scalafmt/scalafmt/unformatted/unformatted-test3.scala.fmt.output:
```

The `test_cross_version/scalafmt/unformatted/unformatted-test3.scala`
file formatted by this test target looks like this:

```scala
import org.scalatest.flatspec._

class Test extends
  AnyFlatSpec:

        "Test"  should  "be formatted" in  {
     assert(true)
        }
```

I hacked `ScalaWorker.format()` to print the `code` variable, and could
see that  after the first `Scalafmt.format()` pass, the code looks like
this:

```scala
import org.scalatest.flatspec._

class Test extends AnyFlatSpec:

"Test" should "be formatted" in {
  assert(true)
}
```

Since the result doesn't match the original code, it tries to call
`Scalafmt.format()` again on this "formatted" code with the incorrect
indentation. That's when we get the following, which doesn't look
anything like the original file:

```txt
Unable to format file due to bug in scalafmt
scalafmt/unformatted/unformatted-test3.scala:3:
  error: [dialect scala3 [with overrides]] `;` expected but `:` found
class Test extends AnyFlatSpec:
                              ^
```

---

As it turns out, bumping to com.google.protobuf:protobuf-java:4.28.2
alone breaks the bump to Scalafmt 3.8.3. Then bumping to rules_proto
6.0.2, with the separate protobuf v21.7, fixes it, presumably by
recompiling `protoc`.

The in-between breakage happened in the `test_cross_build` project:

```txt
$ bazel build //scalafmt:formatted-binary2

INFO: Analyzed target //scalafmt:formatted-binary2
  (4 packages loaded, 64 targets configured).
ERROR: .../test_cross_build/scalafmt/BUILD:59:22:
  ScalaFmt scalafmt/scalafmt/formatted/formatted-binary2.scala.fmt.output
  failed: Worker process did not return a WorkResponse:

---8<---8<--- Start of log, file at .../bazel-workers/worker-3-ScalaFmt.log ---8<---8<---
Exception in thread "main" java.lang.NoSuchMethodError:
  'void com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.makeExtensionsImmutable()'
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.<init>(WorkerProtocol.java:1029)
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.<init>(WorkerProtocol.java:922)
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest$1.parsePartialFrom(WorkerProtocol.java:2482)
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest$1.parsePartialFrom(WorkerProtocol.java:2476)
    at com.google.protobuf.AbstractParser.parsePartialFrom(AbstractParser.java:192)
    at com.google.protobuf.AbstractParser.parsePartialDelimitedFrom(AbstractParser.java:232)
    at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:244)
    at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:249)
    at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:25)
    at com.google.protobuf.GeneratedMessage.parseDelimitedWithIOException(GeneratedMessage.java:367)
    at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.parseDelimitedFrom(WorkerProtocol.java:1438)
    at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:81)
    at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49)
    at io.bazel.rules_scala.scalafmt.ScalafmtWorker$.main(ScalafmtWorker.scala:12)
    at io.bazel.rules_scala.scalafmt.ScalafmtWorker.main(ScalafmtWorker.scala)
---8<---8<--- End of log ---8<---8<---
Target //scalafmt:formatted-binary2 failed to build
```
    @mbland
    mbland committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    3d8c880 View commit details
    Browse the repository at this point in the history

  2. Add attributes to .scalafmt.conf files 

    Per suggestions from @WojciechMazur in bazelbuild#1631.
    @mbland
    mbland committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    3e63222 View commit details
    Browse the repository at this point in the history

  3. Bump Scala 2 libs after bazelbuild#1636 

    Brings all the Scalafmt 3.8.3 updates from bazelbuild#1631 up to date with the
Scala version bumps from bazelbuild#1636.
    @mbland
    mbland committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    9879592 View commit details
    Browse the repository at this point in the history