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

Scalafmt Reformats Valid Code to Code that is Invalid on Dotty #1509

Closed
adamgfraser opened this issue Sep 24, 2019 · 6 comments
Closed

Scalafmt Reformats Valid Code to Code that is Invalid on Dotty #1509

adamgfraser opened this issue Sep 24, 2019 · 6 comments

Comments

@adamgfraser
Copy link

With the changes in the latest version of Dotty regarding indentation and control structures we are seeing code that would be valid in 2.13 and Dotty reformatted to code that is still legal under 2.13 but not under Dotty. Minimized example below shows code that will compile under Dotty and will not compile. With our scalafmt settings the first is rewritten to the second, which is particularly problematic. Is there something we can change in our settings to prevent this? Is there a way we can go the other way, and get scalafmt to rewrite the second version to the first version, or automatically put braces around the first clause of the if statement?

I have included our settings below.

object Example {

  val okay =
    if (true)
      List(1, 2, 3) match {
        case x :: xs => ???
        case Nil => ???
      }
    else {
      2
    }

  val notOkay = 
    if (true)
      List(1, 2, 3) match {
        case x :: xs => ???
        case Nil => ???
    } else {
      2
    }
}
version = "2.0.1"
maxColumn = 120
align = most
continuationIndent.defnSite = 2
assumeStandardLibraryStripMargin = true
docstrings = JavaDoc
lineEndings = preserve
includeCurlyBraceInSelectChains = false
danglingParentheses = true
spaces {
  inImportCurlyBraces = true
}
optIn.annotationNewlines = true

rewrite.rules = [SortImports, RedundantBraces]
@ekrich
Copy link

ekrich commented Oct 2, 2019

I ran across this same thing but only we the latest 0.19.0-RC1 - prior versions worked.

sbt:sconfig-root> sconfigJVM/test
[info] Compiling 104 Scala sources and 1 Java source to /Users/eric/workspace/sconfig/sconfig/jvm/target/scala-0.19/classes ...
[error] -- Error: /Users/eric/workspace/sconfig/sconfig/shared/src/main/scala/org/ekrich/config/impl/PropertiesParser.scala:144:10 
[error] 144 |        } else
[error]     |          ^^^^
[error]     |          end of statement expected but else found
[error] -- [E040] Syntax Error: /Users/eric/workspace/sconfig/sconfig/shared/src/main/scala/org/ekrich/config/impl/PropertiesParser.scala:145:8 
[error] 145 |        value = ConfigImpl.fromAnyRef(pathMap.get(path),
[error]     |        ^^^^^
[error]     |        ';' expected, but identifier found

@ekrich
Copy link

ekrich commented Oct 2, 2019

I really think this should work given the error.

  val notOkay = 
    if (true)
      List(1, 2, 3) match {
        case x :: xs => ???
        case Nil => ???
    }; else {
      2
    }

But it doesn't.

@adamgfraser
Copy link
Author

I can confirm that we only observed the issue after upgrading to 0.19.0-RC1. I reached out to the Dotty team and they confirmed that they view this code as improperly indented and from their perspective failing to compile is the expected behavior. So it would be really nice to have options in scalafmt to avoid rewriting valid code to invalid code and ideally go the other way.

@olafurpg
Copy link
Member

olafurpg commented Oct 8, 2019

Thank you for reporting! Opened a PR with a fix #1520

olafurpg added a commit that referenced this issue Oct 8, 2019
Insert newline for else keyword, fixes #1509.
@adamgfraser
Copy link
Author

Thanks so much!

@ekrich
Copy link

ekrich commented Oct 8, 2019

@olafurpg Thank you kind sir! That will help a bunch of folks.

chbatey added a commit to chbatey/akka that referenced this issue Jan 15, 2020
The edition mainly works but some bugs have been fixed e.g.
scalameta/scalafmt#1509 that cause some
formatting changes
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

No branches or pull requests

3 participants