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

Help wanted: Formatting inconsistency in 3.8.2 that's not in 3.8.1 #4067

Closed
henricook opened this issue Jun 19, 2024 · 3 comments · Fixed by #4068
Closed

Help wanted: Formatting inconsistency in 3.8.2 that's not in 3.8.1 #4067

henricook opened this issue Jun 19, 2024 · 3 comments · Fixed by #4068

Comments

@henricook
Copy link

Configuration (required)

Please paste the contents of your .scalafmt.conf file here:

version = "3.8.2"
runner.dialect = scala213

#
# See http://scalameta.org/scalafmt/#Configuration for details
#
project {
  includeFilters = [
    ".*.\\.scala$"
    ".*\\..sbt$"
  ]
}

# An unconventional number but good for our setting where we're all on similar machines with large screen and do no mobile review.
maxColumn = 120

# Vertical alignment, options: none, some, more
#
# This awkward, self-contradictory, configuration ensures that only
# the common sbt tokens get aligned, and not "production" code.
#
align {
  preset = more
  arrowEnumeratorGenerator = true
  multiline = true
  tokens."+" = [
    {code = "="},
    {code = "<-"},
    {code = "->"}
  ]
}

# If true, the margin character | is aligned with the opening triple quote string literals
assumeStandardLibraryStripMargin = true

# Must be true for 'newlines.source = keep' which grants the developer more control over when new lines should (not) be inserted.
includeCurlyBraceInSelectChains = true

continuationIndent {
  callSite = 2
  defnSite = 2
  extendSite = 4
}

danglingParentheses.preset = true

newlines {
  inInterpolation = avoid
  ignoreInSyntax = true
  source = keep
  alwaysBeforeMultilineDef = false
  sometimesBeforeColonInMethodReturnType = true
  penalizeSingleSelectMultiArgList = false
  alwaysBeforeElseAfterCurlyIf = false
  neverInResultType = false
  topLevelStatementBlankLines = [
    { blanks { before = 1 } }
  ]
}

spaces {
  afterKeywordBeforeParen = true
}

binPack {
  parentConstructors = true
  literalArgumentLists = true
}

optIn {
  breaksInsideChains = false
  breakChainOnFirstMethodDot = true
  configStyleArguments = true
}

runner {
  optimizer {
    # Set to -1 to disable. Number of characters needed to trigger "config-style" formatting
    # see: http://scalameta.org/scalafmt/#runner.optimizer.forceConfigStyleOnOffset
    forceConfigStyleOnOffset = 150

    # minimum number of func arguments before config-style (look at top of file) is enabled
    forceConfigStyleMinArgCount = 2
  }
}

rewrite {
  rules = [
    SortImports
    SortModifiers
    # if your for has more than one single <- then it gets transformed into a multit-line curly brace one
    # PreferCurlyFors
    RedundantParens
  ]
  trailingCommas.style = multiple
}

I've written a unit test for this:

  val myStyle = ScalafmtConfig
    .fromHoconFile(Paths.get("/home/henri/repos/myProj/.scalafmt.conf"))
    .get

  test("meep meep") {
    val original =
      """throw new Exception(
        |  s"Can't create a new SpecialThingamer when forwarding without specialUserId/specialDetails. SpecialUserID: ${x.maybeSpecialUserId}, Special details count: ${maybeSpecialDetails.map(_.length).getOrElse(0)}"
        |)""".stripMargin
    val expected =
      """throw new Exception(
        |  s"Can't create a new SpecialThingamer when forwarding without specialUserId/specialDetails. SpecialUserID: ${x.maybeSpecialUserId}, Special details count: ${maybeSpecialDetails.map(_.length).getOrElse(0)}"
        |)""".stripMargin // v3.8.2 only injects a line break after the `x` in `x.maybeSpecialUserId`
    val obtained = Scalafmt.format(original, myStyle).get
    assertNoDiff(obtained, expected)
  }

Notes

As noted in the comment here, a line break gets injected into this long line string after the 'x' of x.maybeSpecialUserId - this happens in master with the config file attached, but doesn't happen with the exact same file in v3.8.1. Assuming this kind of breaking change wasn't intended between 3.8.2 and 3.8.1 as they're patch versions... I think there's a bug here.

Things that I'd love to know to make me a more effective bug reporter:

  • Can I turn on a debug mode and see which rule or module is making the code change? That might have let me narrow the code down and find the fix myself in the 3.8.1->3.8.2 diff

Otherwise thanks for your time and all thoughts on the potential cause greatly appreciated 🙏🏻

@kitbellew
Copy link
Collaborator

Things that I'd love to know to make me a more effective bug reporter:

Ideally, you'd provide the smallest possible set of configuration parameters to reproduce the problem.

@henricook
Copy link
Author

Things that I'd love to know to make me a more effective bug reporter:

Ideally, you'd provide the smallest possible set of configuration parameters to reproduce the problem.

Well that absolutely sounds like something that should have been obvious to me, i'll cut bits of that file out tomorrow until it stops happening and come back to you here!

@henricook
Copy link
Author

Fantastic, thanks @kitbellew !

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