scalameta / scalafmt

Code formatter for Scala
http://scalameta.org/scalafmt
Apache License 2.0
1.42k stars 276 forks source link

`rewrite.rules = [RedundantBraces]` causes regression for single line tuple values inside a `try` #4108

Closed mdedetrich closed 1 month ago

mdedetrich commented 1 month ago

Configuration (required)

version                                  = 3.8.2
runner.dialect                           = scala213
project.git                              = true
style                                    = defaultWithAlign
docstrings.style                         = Asterisk
docstrings.wrap                          = false
indentOperator.preset                    = spray
maxColumn                                = 120
lineEndings                              = preserve
rewrite.rules                            = [RedundantParens, SortImports, AvoidInfix]
indentOperator.exemptScope               = all
align.preset                             = some
align.tokens."+"                         = [
  {
    code   = "~>"
    owners = [
      { regex = "Term.ApplyInfix" }
    ]
  }
]
literals.hexDigits                       = upper
literals.hexPrefix                       = lower
binPack.unsafeCallSite                   = always
binPack.unsafeDefnSite                   = always
binPack.indentCallSiteSingleArg          = false
binPack.indentCallSiteOnce               = true
newlines.avoidForSimpleOverflow          = [slc]
newlines.source                          = keep
newlines.beforeMultiline                 = keep
align.openParenDefnSite                  = false
align.openParenCallSite                  = false
align.allowOverflow                      = true
optIn.breakChainOnFirstMethodDot         = false
optIn.configStyleArguments               = false
danglingParentheses.preset               = false
spaces.inImportCurlyBraces               = true
rewrite.rules                            = [RedundantBraces]
rewrite.neverInfix.excludeFilters        = [
  and
  min
  max
  until
  to
  by
  eq
  ne
  "should.*"
  "contain.*"
  "must.*"
  in
  ignore
  be
  taggedAs
  thrownBy
  synchronized
  have
  when
  size
  only
  noneOf
  oneElementOf
  noElementsOf
  atLeastOneElementOf
  atMostOneElementOf
  allElementsOf
  inOrderElementsOf
  theSameElementsAs
  theSameElementsInOrderAs
]
rewriteTokens          = {
  "⇒": "=>"
  "→": "->"
  "←": "<-"
}
project.excludeFilters = [
  "scripts/authors.scala"
]
project.layout         = StandardConvention

Command-line parameters (required)

When I run scalafmt via CLI like this: scalafmt

Steps

Given code like this:

      try {
        (ActorSystem("other-system", otherConfig), otherSelection)
      } catch {
        case NonFatal(ex) if ex.getMessage.contains("Failed to bind") && retries > 0 =>
          selectionAndBind(config, thisSystem, probe, retries = retries - 1)
        case other =>
          throw other
      }

Problem

Scalafmt formats code like this:

      try
        (ActorSystem("other-system", otherConfig), otherSelection)
      catch {
        case NonFatal(ex) if ex.getMessage.contains("Failed to bind") && retries > 0 =>
          selectionAndBind(config, thisSystem, probe, retries = retries - 1)
        case other =>
          throw other
      }

Expectation

I would like the formatted output to look like this:

      try {
        (ActorSystem("other-system", otherConfig), otherSelection)
      } catch {
        case NonFatal(ex) if ex.getMessage.contains("Failed to bind") && retries > 0 =>
          selectionAndBind(config, thisSystem, probe, retries = retries - 1)
        case other =>
          throw other
      }

This is because removing the }{ in the try block causes a compile error, i.e.

[error] /home/mdedetrich/github/pekko/remote/src/test/scala/org/apache/pekko/remote/classic/RemotingSpec.scala:785:50: ')' expected but ',' found.
[error]         (ActorSystem("other-system", otherConfig), otherSelection)
[error]                                                  ^
[error] one error found

Workaround

None aside from making scalafmt not working on that part of the code

Notes

See PR at https://github.com/apache/pekko/pull/1408, failing code block is at https://github.com/mdedetrich/pekko/blob/5eb0aeea8a1043055d141bba3e287525e3ef5b47/remote/src/test/scala/org/apache/pekko/remote/classic/RemotingSpec.scala#L769-L792

kitbellew commented 1 month ago

Configuration (required)


version                                  = 3.8.2
runner.dialect                           = scala213

are you absolutely sure that you never build with any other versions of scala, such as those which require try {}?

https://github.com/apache/pekko/blob/main/project/Dependencies.scala#L47

mdedetrich commented 1 month ago

So to make it clear, the following command that fails to compile when the }{ is removed from try

 sbt ";+TestJdk9 / compile ; +compile:doc"

Its +compile:doc which is failing (since this source code is contained within docs) which iterates through crossScalaVersions

kitbellew commented 1 month ago

the point is, you're compiling this file for scala212, then why do you expect your dialect to be scala213?

i think we can once again qualify this as configuration error rather than a formatter bug.

mdedetrich commented 1 month ago

the point is, you're compiling this file for scala212, then why do you expect your dialect to be scala213?

i think we can once again qualify this as configuration error rather than a formatter bug.

If I set runner.dialect = scala212 it also removes the {} in the try block causing the code to fail to compile.