2.4.0: scalafmtAll failing to run on code that compiles

[rossabaker]

This template is a guideline, not a strict requirement.

• Version: 2.4.0
• Integration: sbt-scalafmt
• Configuration:

version = 2.4.0

style = default

maxColumn = 100

// Vertical alignment is pretty, but leads to bigger diffs
align = none

danglingParentheses = false

rewrite.rules = [
  AvoidInfix
  RedundantBraces
  RedundantParens
  AsciiSortImports
  PreferCurlyFors
]

project.excludeFilters = [
   "scalafix-inputs",
   "scalafix-outputs"
]

Steps

Given code like this

          val policy = RetryPolicy[IO]({ attempts: Int =>
            if (attempts >= 2) None
            else Some(Duration.Zero)
          }, retriable)

When I run scalafmt like this:

client/scalafmtAll

Problem

Scalafmt fails like this:

sbt:http4s> client/scalafmtAll
[info] Formatting 15 Scala sources...
[info] Formatting 26 Scala sources...
[info] Reformatted 9 Scala sources
[error] /home/ross/src/http4s/client/src/test/scala/org/http4s/client/middleware/RetrySpec.scala:86: error: identifier expected but if found
[error]             if (attempts >= 2) None
[error]             ^: /home/ross/src/http4s/client/src/test/scala/org/http4s/client/middleware/RetrySpec.scala
[error] (client / Test / scalafmt) /home/ross/src/http4s/client/src/test/scala/org/http4s/client/middleware/RetrySpec.scala:86: error: identifier expected but if found
[error]             if (attempts >= 2) None
[error]             ^: /home/ross/src/http4s/client/src/test/scala/org/http4s/client/middleware/RetrySpec.scala
[error] Total time: 1 s, completed Feb 15, 2020 11:34:47 AM

Expectation

Can successfully run scalafmtAll on any source tree that compiles.

Workaround

Stay on 2.3 for the moment. 😄

Notes

Also happens on the ember-client project:

sbt:http4s> ember-client/scalafmtAll
[info] Formatting 4 Scala sources...
[error] /home/ross/src/http4s/ember-client/src/main/scala/org/http4s/ember/client/EmberClientBuilder.scala:92: error: ) expected but ( found
[error]               .requestKeyToSocketWithKey[F](
[error]                                            ^: /home/ross/src/http4s/ember-client/src/main/scala/org/http4s/ember/client/EmberClientBuilder.scala
[error] (ember-client / Compile / scalafmt) /home/ross/src/http4s/ember-client/src/main/scala/org/http4s/ember/client/EmberClientBuilder.scala:92: error: ) expected but ( found
[error]               .requestKeyToSocketWithKey[F](
[error]                                            ^: /home/ross/src/http4s/ember-client/src/main/scala/org/http4s/ember/client/EmberClientBuilder.scala
[error] Total time: 0 s, completed Feb 15, 2020 11:43:52 AM

scalafmtAll runs on other modules on this commit, but the output fails to compile (report).

[kitbellew]

@rossabaker please try v2.4.1

[rossabaker]

Thanks! It fixes the crash, but the result still doesn't compile. It's turning assignment into a named argument. I think that's a separate bug, which I can file if you'd like. The essence is this:

       var bytes: Vector[Byte] = null
       val readBytes = IO(bytes)
       F.runAsync(stream.compile.toVector) {
-          case Right(bs) => IO { bytes = bs }
+          case Right(bs) => IO(bytes = bs)
           case Left(t) => IO.raiseError(t)
         }

turns into

[error] /home/ross/src/http4s/laws/src/main/scala/org/http4s/laws/discipline/ArbitraryInstances.scala:725:38: unknown parameter name: bytes
[error] Note that assignments in argument position are no longer allowed since Scala 2.13.
[error] To express the assignment expression, wrap it in brackets, e.g., `{ bytes = ... }`.
[error]           case Right(bs) => IO(bytes = bs)
[error]                                      ^

[regadas]

Would it be possible to have scalafmt respect the {}? Through some option perhaps?

[kitbellew]

Would it be possible to have scalafmt respect the {}? Through some option perhaps?

it only does this if RedundantBraces rewrite is enabled. @regadas

[regadas]

Oh I see! thanks for pointing that out!

However i believe that in this specific case {} are not really redundant and it's not inline with the previous meaning of the rule. https://scalameta.org/scalafmt/docs/configuration.html#redundantbraces. Also makes some code really unreadable 😢

Should we come up with a different rule?

[kitbellew]

@regadas i don't mind but that requires input from the repo maintainers, such as @olafurpg @tanishiking @poslegm who have been quite conservative about introducing new options.

[kitbellew]

@regadas see #1723

[regadas]

@kitbellew awesome! 🎉 Thanks!