From 41f917a98a07fd5e47dfa59f5c67407f0f2a81c1 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 24 Dec 2021 11:16:14 -0800 Subject: [PATCH 1/2] Add tests for overflow comments and various wrap --- .../src/test/resources/unit/Comment.stat | 123 +++++++++++++++++- 1 file changed, 121 insertions(+), 2 deletions(-) diff --git a/scalafmt-tests/src/test/resources/unit/Comment.stat b/scalafmt-tests/src/test/resources/unit/Comment.stat index 3a67b14e9b..7633f58b76 100644 --- a/scalafmt-tests/src/test/resources/unit/Comment.stat +++ b/scalafmt-tests/src/test/resources/unit/Comment.stat @@ -530,7 +530,7 @@ object a { * IDs is requsted for MD5 and SHA1 and not for LUID */ ) } -<<< binpack=oneline, with wrapped comment, narrower +<<< binpack=oneline, with trailing-wrapped comment, narrower maxColumn = 90 indent.callSite = 2 comments.wrap = trailing @@ -560,7 +560,67 @@ object a { * 'ATT' IDs is requsted for MD5 and SHA1 and not for LUID */ ) } -<<< binpack=oneline, defn, with wrapped comment, narrower +<<< binpack=oneline, trailing-wrapped comment, narrower +maxColumn = 90 +indent.callSite = 2 +comments.wrap = standalone +binPack.unsafeCallSite = oneline +=== +object a { + val foo = Seq( // First 2 will not be taken into account since this app for 'Do Not Sell' + CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete), + CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), /* Next 2 will + * not be taken into account since 'filterDoNotSellBy' is not set */ + CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell), + CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell) + /* Only last record will be taken into account but not the next one since filtering of 'ATT' + * IDs is requsted for MD5 and SHA1 and not for LUID */ + ) +} +>>> +object a { + val foo = + Seq( // First 2 will not be taken into account since this app for 'Do Not Sell' + CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete), + CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), + /* Next 2 will not be taken into account since 'filterDoNotSellBy' is not set */ + CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell), + CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell) + /* Only last record will be taken into account but not the next one since filtering + * of 'ATT' IDs is requsted for MD5 and SHA1 and not for LUID */ + ) +} +<<< binpack=oneline, no comment wrapping, narrower +maxColumn = 90 +indent.callSite = 2 +binPack.unsafeCallSite = oneline +=== +object a { + val foo = Seq( // First 2 will not be taken into account since this app for 'Do Not Sell' + CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete), + CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), /* Next 2 will + * not be taken into account since 'filterDoNotSellBy' is not set */ + CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell), + CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell) + /* Only last record will be taken into account but not the next one since filtering of 'ATT' + * IDs is requsted for MD5 and SHA1 and not for LUID */ + ) +} +>>> +object a { + val foo = + Seq( // First 2 will not be taken into account since this app for 'Do Not Sell' + CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete), + CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), + /* Next 2 will + * not be taken into account since 'filterDoNotSellBy' is not set */ + CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell), + CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell) + /* Only last record will be taken into account but not the next one since filtering of 'ATT' + * IDs is requsted for MD5 and SHA1 and not for LUID */ + ) +} +<<< binpack=oneline, defn, with trailing-wrapped comment, narrower maxColumn = 51 indent.callSite = 2 align.preset = none @@ -592,3 +652,62 @@ object a { * and SHA1 and not for LUID */ ) = ??? } +<<< binpack=oneline, defn, with standalone-wrapped comment, narrower +maxColumn = 51 +indent.callSite = 2 +align.preset = none +comments.wrap = standalone +binPack.unsafeDefnSite = oneline +=== +object a { + def foo( // First 2 will not be taken into account + foo1: Int, + foo2: String, /* Next 2 will + * not be taken into account since 'filterDoNotSellBy' is not set */ + foo3: Map[Int, Int], + foo4: Set[Int] + /* Only last record will be taken into account but not the next one since filtering of 'ATT' + * IDs is requsted for MD5 and SHA1 and not for LUID */ + ) = ??? +} +>>> +object a { + def foo( // First 2 will not be taken into account + foo1: Int, foo2: String, + /* Next 2 will not be taken into account + * since 'filterDoNotSellBy' is not set */ + foo3: Map[Int, Int], foo4: Set[Int] + /* Only last record will be taken into + * account but not the next one since + * filtering of 'ATT' IDs is requsted for MD5 + * and SHA1 and not for LUID */ + ) = ??? +} +<<< binpack=oneline, defn, no comment wrapping, narrower +maxColumn = 51 +indent.callSite = 2 +align.preset = none +binPack.unsafeDefnSite = oneline +=== +object a { + def foo( // First 2 will not be taken into account + foo1: Int, + foo2: String, /* Next 2 will + * not be taken into account since 'filterDoNotSellBy' is not set */ + foo3: Map[Int, Int], + foo4: Set[Int] + /* Only last record will be taken into account but not the next one since filtering of 'ATT' + * IDs is requsted for MD5 and SHA1 and not for LUID */ + ) = ??? +} +>>> +object a { + def foo( // First 2 will not be taken into account + foo1: Int, foo2: String, + /* Next 2 will + * not be taken into account since 'filterDoNotSellBy' is not set */ + foo3: Map[Int, Int], foo4: Set[Int] + /* Only last record will be taken into account but not the next one since filtering of 'ATT' + * IDs is requsted for MD5 and SHA1 and not for LUID */ + ) = ??? +} From c07b8c8615288b067497814ac957e3c5efe25036 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 24 Dec 2021 11:16:53 -0800 Subject: [PATCH 2/2] State: don't waive comment overflow for binPack --- .../scala/org/scalafmt/internal/State.scala | 10 ++++- .../src/test/resources/unit/Comment.stat | 40 +++++++++---------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/State.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/State.scala index 9b26a194ea..904a9361da 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/State.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/State.scala @@ -90,7 +90,15 @@ final case class State( else (style.comments.wrap eq Comments.Wrap.trailing) || style.comments.willWrap && nextSplit.isNL - } + } && (nextSplit.isNL || { + // do not waive overflow in binpacking case + val owner = tok.meta.rightOwner + val ownerLast = tokens.getLastNonTrivial(owner).left + val isBracket = ownerLast.is[Token.RightBracket] + !(isBracket || ownerLast.is[Token.RightParen]) || + style.binPack.defnSite(isBracket).isNever && isDefnSite(owner) || + style.binPack.callSite(isBracket).isNever && isCallSite(owner) + }) } ) { (math.max(0, delayedPenalty), 0) // fits inside column diff --git a/scalafmt-tests/src/test/resources/unit/Comment.stat b/scalafmt-tests/src/test/resources/unit/Comment.stat index 7633f58b76..9095e0ce9a 100644 --- a/scalafmt-tests/src/test/resources/unit/Comment.stat +++ b/scalafmt-tests/src/test/resources/unit/Comment.stat @@ -519,16 +519,16 @@ object a { } >>> object a { - val requestedIds = Seq( /* First 2 will not be taken into account since this app for 'Do Not Sell' - * and 'ATT' only */ - CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete), - CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), - /* Next 2 will not be taken into account since 'filterDoNotSellBy' is not set */ - CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell), - CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell) - /* Only last record will be taken into account but not the next one since filtering of 'ATT' - * IDs is requsted for MD5 and SHA1 and not for LUID */ - ) + val requestedIds = + Seq( // First 2 will not be taken into account since this app for 'Do Not Sell' and 'ATT' only + CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete), + CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), + /* Next 2 will not be taken into account since 'filterDoNotSellBy' is not set */ + CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell), + CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell) + /* Only last record will be taken into account but not the next one since filtering of 'ATT' + * IDs is requsted for MD5 and SHA1 and not for LUID */ + ) } <<< binpack=oneline, with trailing-wrapped comment, narrower maxColumn = 90 @@ -549,16 +549,16 @@ object a { } >>> object a { - val foo = Seq( /* First 2 will not be taken into account since this app for 'Do Not - * Sell' */ - CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete), - CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), - /* Next 2 will not be taken into account since 'filterDoNotSellBy' is not set */ - CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell), - CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell) - /* Only last record will be taken into account but not the next one since filtering of - * 'ATT' IDs is requsted for MD5 and SHA1 and not for LUID */ - ) + val foo = + Seq( // First 2 will not be taken into account since this app for 'Do Not Sell' + CcpaRequestIds("luid", "1", None, date, version, RequestType.Delete), + CcpaRequestIds("hemlmd5", "MD5-1", None, date, version, RequestType.Delete), + /* Next 2 will not be taken into account since 'filterDoNotSellBy' is not set */ + CcpaRequestIds("luid", "2", date, None, version, RequestType.DoNotSell), + CcpaRequestIds("hemlmd5", "MD5-2", date, None, version, RequestType.DoNotSell) + /* Only last record will be taken into account but not the next one since filtering + * of 'ATT' IDs is requsted for MD5 and SHA1 and not for LUID */ + ) } <<< binpack=oneline, trailing-wrapped comment, narrower maxColumn = 90