scalameta / scalafmt

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

Curried functions with implicit parameter lists are always unfolded even though the verticalMultiline.arityThreshold is not reached. #3520

Closed agilesteel closed 1 year ago

agilesteel commented 1 year ago

Configuration

version = "3.7.3"

runner.dialect = scala213

maxColumn = 100

newlines.implicitParamListModifierForce = [after]

verticalMultiline {
  arityThreshold = 3
  atDefnSite = yes
  newlineAfterOpenParen = yes
}

Steps

Given code like this:

object Foo {
  def bar(a: Int)(implicit b: Int): Int =
    a + b
}

Problem

Scalafmt formats code like this:

object Foo {
  def bar(
      a: Int
    )(implicit
      b: Int
    ): Int =
    a + b
}

Expectation

I would like the formatted output to look like this:

object Foo {
  def bar(a: Int)(implicit b: Int): Int =
    a + b
}

Workaround

I've found that by switching back all the way to 3.7.1 fixes the problem.

Notes

I think it's related to https://github.com/scalameta/scalafmt/pull/3478

kitbellew commented 1 year ago

@agilesteel perhaps it's because 3.7.1 and before handled this incorrectly. the parameter is named implicitParamListModifierForce, hence the newline is forced, it cannot be omitted. if you try to format without vertical multiline, it will introduce a newline as well.

agilesteel commented 1 year ago

Maybe this should be a feature request then? I think it makes sense to respect the threshold.

Changing it to newlines.implicitParamListModifierPrefer = after fixes the issue, but causes the following sometimes:

object Foo {
  def bar(
      a: Int
    )(implicit b: Int // b: Int should be on the next line
    ): Int =
    a + b
}
kitbellew commented 1 year ago

Maybe this should be a feature request then? I think it makes sense to respect the threshold.

The threshold forces the format, but it's not the only way; if the signature can't be formatted on one line (such as when a newline is forced), that will cause this formatting as well.

object Foo {
  def bar(
      a: Int
    )(implicit b: Int // b: Int should be on the next line
    ): Int =
    a + b
}

Why did you expect b: Int to be on the next line? The entire implicit b: Int fits on one line so newlines.implicitParamListModifierPrefer should not apply, according to the documentation.

Also, if dangling parens parameter is set appropriately, this will also happen:

object Foo {
  def bar(
      a: Int
    )(b: Int
    ): Int =
    a + b
}

The reason is that the formatter doesn't force a newline when indentation would be the same or larger, compare with:

object Foo {
  def bar(
      a: Int
    )(
      b: Int
    ): Int =
    a + b

  def bar(
      a: Int
    )(
      implicit b: Int // foo
    ): Int =
    a + b
}
agilesteel commented 1 year ago

So how would I make it behave as it did before? Basically all I'd like to have is that it doesn't break the line immediately just because there is an implicit parameter involved, but when it does need to break the line either because the line is too long or the arity threshold is reached or for whatever reason, then it should break it the same as it does now in 3.7.3.

kitbellew commented 1 year ago

@agilesteel "it doesn't break" means you shouldn't use Force. can you demonstrate the second part with an example, please?

agilesteel commented 1 year ago

So I'm copy pasting this from the problem section in this issue.

object Foo {
  def bar(
      a: Int
    )(implicit
      b: Int
    ): Int =
    a + b
}

I like HOW the formatting is done here. This is how it was both before 3.7.3 as well as it is now in 3.7.3 (assuming the conditions in the next paragraph were met). The only difference is WHEN formatting should happen.

Before no formatting was occurring unless the line was too long or unless my arity threshold was reached, so sth like the following would remain untouched:

object Foo {
  def bar(a: Int)(implicit b: Int): Int =
    a + b
}

You said this used to be due a bug that has now been fixed. I guess I shouldn't use force anymore, but what do I use to make it behave like it did before?

kitbellew commented 1 year ago

So I'm copy pasting this from the problem section in this issue.

object Foo {
  def bar(
      a: Int
    )(implicit
      b: Int
    ): Int =
    a + b
}

I like HOW the formatting is done here. This is how it was both before 3.7.3 as well as it is now in 3.7.3 (assuming the conditions in the next paragraph were met). The only difference is WHEN formatting should happen.

Before no formatting was occurring unless the line was too long or unless my arity threshold was reached, so sth like the following would remain untouched:

object Foo {
  def bar(a: Int)(implicit b: Int): Int =
    a + b
}

You said this used to be due a bug that has now been fixed. I guess I shouldn't use force anymore, but what do I use to make it behave like it did before?

nothing currently. implicit b: Int fits on a line and hence will not cause a break within.

agilesteel commented 1 year ago

Right, which brings us back to Maybe this should be a feature request then? 😉

kitbellew commented 1 year ago

please discuss this proposal on the scalafmt discord channel, and re-open if the community supports this.