From 2e574b4404863f98b26266c548a53d5ffeb9a51b Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sun, 1 Mar 2020 11:11:33 -0800 Subject: [PATCH 1/3] Add failing tests from issue #1756 --- .../rewrite/RedundantBraces-ParenLambdas.stat | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat index 7b936a3037..5ba804e6d9 100644 --- a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat +++ b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat @@ -185,3 +185,63 @@ F.runAsync(stream.compile.toVector) { def old = foo.map { a => b: SomeType => blah() } >>> def old = foo.map { a => b: SomeType => blah() } +<<< #1756 val within lambda +object a { + childContext.onDone { _ => + val _ = self.withChildren(_.remove(childContext)) + } +} +>>> +test does not parse +object a { + childContext.onDone(_ => val _ = self.withChildren(_.remove(childContext))) +} +<<< #1756 def within lambda +object a { + childContext.onDone { _ => + def a = self.withChildren(_.remove(childContext)) + } +} +>>> +test does not parse +object a { + childContext.onDone(_ => def a = self.withChildren(_.remove(childContext))) +} +<<< #1756 type within lambda +object a { + childContext.onDone { _ => + type a = Int + } +} +>>> +test does not parse +object a { + childContext.onDone(_ => type a = Int) +} +<<< #1756 val without lambda +object a { + childContext.onDone { val _ = self.withChildren(_.remove(childContext)) } +} +>>> +test does not parse +object a { + childContext.onDone(val _ = self.withChildren(_.remove(childContext))) +} +<<< #1756 def without lambda +object a { + childContext.onDone { def a = self.withChildren(_.remove(childContext)) } +} +>>> +test does not parse +object a { + childContext.onDone(def a = self.withChildren(_.remove(childContext))) +} +<<< #1756 type without lambda +object a { + childContext.onDone { type a = Int } +} +>>> +test does not parse +object a { + childContext.onDone(type a = Int) +} From 472f54bd16e683047bf0b5e8dd1b0937b78a6800 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sun, 1 Mar 2020 11:47:48 -0800 Subject: [PATCH 2/3] FormatWriter: refactor the {} => () block match --- .../org/scalafmt/internal/FormatWriter.scala | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatWriter.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatWriter.scala index 76d4a03fa4..6984103116 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatWriter.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatWriter.scala @@ -173,15 +173,13 @@ class FormatWriter(formatOps: FormatOps) { tok.left match { case rb: T.RightBrace => // look for "foo { bar }" tok.meta.leftOwner match { - case b: Term.Block if RedundantBraces.canRewriteWithParens(b) => - b.parent match { - case Some(ta: Term.Apply) - if TreeOps.isSingleElement(ta.args, b) && - (ta.tokens.last eq rb) => - val beg = tokens(matching(rb)) - lookup.update(beg.meta.idx, tok.meta.idx -> lineOffset) - case _ => - } + case b: Term.Block if b.parent.exists { + case ta: Term.Apply if ta.tokens.last eq rb => + TreeOps.isSingleElement(ta.args, b) + case _ => false + } && RedundantBraces.canRewriteWithParens(b) => + val beg = tokens(matching(rb)) + lookup.update(beg.meta.idx, tok.meta.idx -> lineOffset) case _ => } case _: T.LeftBrace => From 2c31086a9fbb1d02b7e2dc3c693b6f616b25e21a Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sun, 1 Mar 2020 11:12:17 -0800 Subject: [PATCH 3/3] RedundantBraces: don't rewrite {} => () with Defn Fixes #1756. --- .../org/scalafmt/rewrite/RedundantBraces.scala | 7 ++++--- .../rewrite/RedundantBraces-ParenLambdas.stat | 18 ++++++------------ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala index 7a9a25a730..00510f6962 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala @@ -27,13 +27,13 @@ object RedundantBraces extends Rewrite { case _ => false } - def canRewriteWithParens(b: Term.Block): Boolean = { + def canRewriteWithParens(b: Term.Block): Boolean = getBlockSingleStat(b).exists { - case f: Term.Function => RedundantBraces.canRewriteWithParens(f) + case f: Term.Function => canRewriteWithParens(f) case _: Term.Assign => false // disallowed in 2.13 + case _: Defn => false case _ => true } - } /* guard for statements requiring a wrapper block * "foo { x => y; z }" can't become "foo(x => y; z)" */ @@ -41,6 +41,7 @@ object RedundantBraces extends Rewrite { def canRewriteWithParens(f: Term.Function, nested: Boolean = false): Boolean = !needParensAroundParams(f) && (getTermSingleStat(f.body) match { case Some(t: Term.Function) => canRewriteWithParens(t, true) + case Some(_: Defn) => false case x => nested || x.isDefined }) diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat index 5ba804e6d9..5246271bd2 100644 --- a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat +++ b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-ParenLambdas.stat @@ -192,9 +192,8 @@ object a { } } >>> -test does not parse object a { - childContext.onDone(_ => val _ = self.withChildren(_.remove(childContext))) + childContext.onDone { _ => val _ = self.withChildren(_.remove(childContext)) } } <<< #1756 def within lambda object a { @@ -203,9 +202,8 @@ object a { } } >>> -test does not parse object a { - childContext.onDone(_ => def a = self.withChildren(_.remove(childContext))) + childContext.onDone { _ => def a = self.withChildren(_.remove(childContext)) } } <<< #1756 type within lambda object a { @@ -214,34 +212,30 @@ object a { } } >>> -test does not parse object a { - childContext.onDone(_ => type a = Int) + childContext.onDone { _ => type a = Int } } <<< #1756 val without lambda object a { childContext.onDone { val _ = self.withChildren(_.remove(childContext)) } } >>> -test does not parse object a { - childContext.onDone(val _ = self.withChildren(_.remove(childContext))) + childContext.onDone { val _ = self.withChildren(_.remove(childContext)) } } <<< #1756 def without lambda object a { childContext.onDone { def a = self.withChildren(_.remove(childContext)) } } >>> -test does not parse object a { - childContext.onDone(def a = self.withChildren(_.remove(childContext))) + childContext.onDone { def a = self.withChildren(_.remove(childContext)) } } <<< #1756 type without lambda object a { childContext.onDone { type a = Int } } >>> -test does not parse object a { - childContext.onDone(type a = Int) + childContext.onDone { type a = Int } }