-
Notifications
You must be signed in to change notification settings - Fork 276
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
RedundantBraces: add noParams option to defnBodies #3031
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Added only some smaleer comments about the naming,
case d: Defn.Macro => checkBlockAsBody(b, d.body) | ||
case d: Defn.GivenAlias => checkBlockAsBody(b, d.body) | ||
case d: Defn.Var => | ||
defnBodies(true) && d.rhs.exists(checkBlockAsBody(b, _)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defnBodies(true) && d.rhs.exists(checkBlockAsBody(b, _)) | |
defnBodies(noParams = true) && d.rhs.exists(checkBlockAsBody(b, _)) |
Otherwise it's a bit unclear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do
case d: Defn.Var => | ||
defnBodies(true) && d.rhs.exists(checkBlockAsBody(b, _)) | ||
case d: Defn.Val => | ||
defnBodies(true) && checkBlockAsBody(b, d.rhs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defnBodies(true) && checkBlockAsBody(b, d.rhs) | |
defnBodies(noParams = true) && checkBlockAsBody(b, d.rhs) |
@@ -340,6 +344,25 @@ class RedundantBraces(ftoks: FormatTokens) extends FormatTokensRewrite.Rule { | |||
): Boolean = | |||
rhs.eq(b) && getSingleStatIfLineSpanOk(b).exists(innerOk(b)) | |||
|
|||
private def defnBodies( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private def defnBodies( | |
private def shouldRemoveDefnBraces( |
maybe a bit better explaining ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, will explain
case RedundantBracesSettings.DefnBodies.noParams => noParams | ||
} | ||
|
||
private def defnBodies( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private def defnBodies( | |
private def isNoDefnParams( |
@@ -149,7 +149,7 @@ class RedundantBraces(ftoks: FormatTokens) extends FormatTokensRewrite.Rule { | |||
!x.tokens.exists(_.is[Token.RightArrow])) => | |||
removeToken | |||
case t: Ctor.Secondary | |||
if t.stats.isEmpty && style.rewrite.redundantBraces.defnBodies => | |||
if t.stats.isEmpty && isDefnBodiesEnabled(noParams = false) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added explicit noParams =
here and everywhere else.
@@ -340,6 +348,25 @@ class RedundantBraces(ftoks: FormatTokens) extends FormatTokensRewrite.Rule { | |||
): Boolean = | |||
rhs.eq(b) && getSingleStatIfLineSpanOk(b).exists(innerOk(b)) | |||
|
|||
private def isDefnBodiesEnabled( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed this method from defnBodies
, should be better explained.
i used the word Enabled
to stress that this is a configuration check primarily, and not a complete determination that it's ok to remove braces here (since we are also calling checkBlockAsBody
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM again, thanks!
Fixes #3026.