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.

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

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

Closed
mdedetrich opened this issue Jul 21, 2024 · 4 comments · Fixed by #4111

Comments

@mdedetrich
Copy link
Contributor

mdedetrich commented Jul 21, 2024

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 apache/pekko#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

@mdedetrich mdedetrich changed the title rewrite.rules = [RedundantBraces] causes regression for single line Tuple2 values inside a try rewrite.rules = [RedundantBraces] causes regression for single line tuple values inside a try Jul 21, 2024
@kitbellew
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

mdedetrich commented Jul 22, 2024

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.

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

Successfully merging a pull request may close this issue.

2 participants