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

unexpected changes in 3.8.4 #4717

Closed
mr-git opened this issue Jan 14, 2025 · 13 comments · Fixed by #4723
Closed

unexpected changes in 3.8.4 #4717

mr-git opened this issue Jan 14, 2025 · 13 comments · Fixed by #4723

Comments

@mr-git
Copy link

mr-git commented Jan 14, 2025

Required: Configuration

Please paste the smallest possible set of .scalafmt.conf
configuration parameters that reproduces the problem:

version = 3.8.4
...

Steps

Given code like this:

case class X(
  field: Int
)

Problem

Scalafmt formats code like this:

case class X(
    field: Int
)

There are extra 2 spaces added before field: Int

Expectation

I would like the formatted output to look like this:

case class X(
  field: Int
)

Workaround

Have to add extra config:

indent.defnSite = 2
@kitbellew
Copy link
Collaborator

@mr-git what kind of fix are you requesting, specifically? restore the bug that was there before?

@mr-git
Copy link
Author

mr-git commented Jan 14, 2025

oh, I didn't expected that the default was intended to be 4.

@ekrich
Copy link

ekrich commented Jan 14, 2025

I am getting unexpected doc changes too. I can create another issue if needed. https://github.com/ekrich/sconfig/pull/416/files

@kitbellew
Copy link
Collaborator

I am getting unexpected doc changes too. I can create another issue if needed. https://github.com/ekrich/sconfig/pull/416/files

@ekrich yes, please, separate issue.

@kitbellew
Copy link
Collaborator

kitbellew commented Jan 14, 2025

I am getting unexpected doc changes too. I can create another issue if needed. https://github.com/ekrich/sconfig/pull/416/files

@ekrich correction: for these changes, there's an issue submitted already. but you need to change

`Config}

to

`Config`

yourself.

@kitbellew
Copy link
Collaborator

i am closing this particular issue since it refers to a bug that had been properly fixed.

@mr-git
Copy link
Author

mr-git commented Jan 14, 2025

Only now I found the https://scalameta.org/scalafmt/docs/configuration.html#indentdefnsite and I've missed that 4 years ago in c61232c config name changed from continuationIndent to indent, but behaviour didn't change. Most of our projects have continuationIndent.defnSite = 2 😞

@kitbellew
Copy link
Collaborator

Only now I found the https://scalameta.org/scalafmt/docs/configuration.html#indentdefnsite and I've missed that 4 years ago in c61232c config name changed from continuationIndent to indent, but behaviour didn't change. Most of our projects have continuationIndent.defnSite = 2 😞

are you saying that continuationIndent.defnSite = 2 is not taking effect (it should)?
it wouldn't have been evident since you pasted your configuration file as

version = 3.8.4
...

@mr-git
Copy link
Author

mr-git commented Jan 15, 2025

probably something else is affecting the formatting in our case...

Our config is:

project.git = true

version = 3.8.3

maxColumn = 120

trailingCommas = always

continuationIndent {
  callSite = 2
  defnSite = 2
}

align.preset = some
align.tokenCategory {
  LeftArrow = Assign
  Equals = Assign
}
# Mostly subset of `align.preset=more`, but with extra settings for `=`
# For all settings see `AlignToken#default` in
# https://github.com/scalameta/scalafmt/blob/master/scalafmt-core/shared/src/main/scala/org/scalafmt/config/AlignToken.scala
align.tokens."+" = [
  { code = "%", owner = "Term.ApplyInfix" }  # This is for Dependencies.scala…
  { code = "%%", owner = "Term.ApplyInfix" } # … and this as well.
  { code = "%%%", owner = "Term.ApplyInfix" } # … and this as well.
  { code = "=>", owner = "(Case|Term.Function)" }
  { code = "<-", owner = "Enumerator.Generator" }
  { code = "=", owner = "(Defn.Val|Defn.Var|Type|Def|Enumerator.Val|Assign|Term.Param)" } # Defn.Val is mostly for Dependencies.scala,
  { code = "->", owner = "Term.ApplyInfix" }
]

newlines.implicitParamListModifierPrefer = before
newlines.beforeCurlyLambdaParams = multilineWithCaseOnly

indentOperator.topLevelOnly = true

docstrings.blankFirstLine = keep

includeCurlyBraceInSelectChains = true
includeNoParensInSelectChains = true
optIn.breakChainOnFirstMethodDot = true

runner.dialect = Scala213Source3
fileOverride {
  "glob:**/*.sbt" { runner.dialect = sbt1 }
}

When I change version to 3.8.4, all multi-line case class and function definitions get reformatted, which is unexpected.

When I replace continuationIndent with indent, I get expected formatting for case classes and function definitions.

I cannot repeat the issue on Scala 3 project with smaller config:

project.git = true

version = 3.8.4

maxColumn = 120

trailingCommas = always

continuationIndent {
  callSite = 2
  defnSite = 2
}

preset = default
align.preset = none

runner.dialect = Scala3
fileOverride {
  "glob:**/*.sbt" { runner.dialect = sbt1 }
}

Could it be, that something else affects application of continuationIndent when runner.dialect = Scala213Source3? Maybe some extra customization in first config blocks application of continuationIndent?

@mr-git
Copy link
Author

mr-git commented Jan 15, 2025

Honestly, I was not fully aware how the scalafmt config is defined - I inherited the project with config and made [wrong] assumption that default indent is 2 🤷

@kitbellew
Copy link
Collaborator

probably something else is affecting the formatting in our case...

Our config is:

project.git = true

version = 3.8.3

maxColumn = 120

trailingCommas = always

continuationIndent {
  callSite = 2
  defnSite = 2
}

align.preset = some
align.tokenCategory {
  LeftArrow = Assign
  Equals = Assign
}
# Mostly subset of `align.preset=more`, but with extra settings for `=`
# For all settings see `AlignToken#default` in
# https://github.com/scalameta/scalafmt/blob/master/scalafmt-core/shared/src/main/scala/org/scalafmt/config/AlignToken.scala
align.tokens."+" = [
  { code = "%", owner = "Term.ApplyInfix" }  # This is for Dependencies.scala…
  { code = "%%", owner = "Term.ApplyInfix" } # … and this as well.
  { code = "%%%", owner = "Term.ApplyInfix" } # … and this as well.
  { code = "=>", owner = "(Case|Term.Function)" }
  { code = "<-", owner = "Enumerator.Generator" }
  { code = "=", owner = "(Defn.Val|Defn.Var|Type|Def|Enumerator.Val|Assign|Term.Param)" } # Defn.Val is mostly for Dependencies.scala,
  { code = "->", owner = "Term.ApplyInfix" }
]

newlines.implicitParamListModifierPrefer = before
newlines.beforeCurlyLambdaParams = multilineWithCaseOnly

indentOperator.topLevelOnly = true

docstrings.blankFirstLine = keep

includeCurlyBraceInSelectChains = true
includeNoParensInSelectChains = true
optIn.breakChainOnFirstMethodDot = true

runner.dialect = Scala213Source3
fileOverride {
  "glob:**/*.sbt" { runner.dialect = sbt1 }
}

When I change version to 3.8.4, all multi-line case class and function definitions get reformatted, which is unexpected.

When I replace continuationIndent with indent, I get expected formatting for case classes and function definitions.

I cannot repeat the issue on Scala 3 project with smaller config:

project.git = true

version = 3.8.4

maxColumn = 120

trailingCommas = always

continuationIndent {
  callSite = 2
  defnSite = 2
}

preset = default
align.preset = none

runner.dialect = Scala3
fileOverride {
  "glob:**/*.sbt" { runner.dialect = sbt1 }
}

Could it be, that something else affects application of continuationIndent when runner.dialect = Scala213Source3? Maybe some extra customization in first config blocks application of continuationIndent?

i don't see anything that springs at me. is your case available to check out and try? paldies.

@mr-git
Copy link
Author

mr-git commented Jan 16, 2025

you can try with https://github.com/evolution-gaming/kafka-journal, I just tried to update version to 3.8.4 and ran fmt alisas in SBT - all case classes and function definitions were reformatted.

@mr-git
Copy link
Author

mr-git commented Jan 16, 2025

evolution-gaming/kafka-journal#711 - PR for upgrade to 3.8.4 - it, unexpectedly, affected 128 files :(

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.

3 participants