Skip to content

Commit

Permalink
Merge pull request #1122 from japgolly/topic/redundant-braces
Browse files Browse the repository at this point in the history
Remove redundant braces from if-else and case expressions
  • Loading branch information
olafurpg committed Mar 18, 2018
2 parents 65f5f75 + 0366376 commit 8f1c4ba
Show file tree
Hide file tree
Showing 14 changed files with 672 additions and 77 deletions.
1 change: 1 addition & 0 deletions build.sbt
Expand Up @@ -14,6 +14,7 @@ lazy val buildSettings = Seq(
libraryDependencies += scalatest.value % Test,
triggeredMessage in ThisBuild := Watched.clearWhenTriggered,
scalacOptions in (Compile, console) := compilerOptions :+ "-Yrepl-class-based",
scalacOptions in (Compile, console) --= Seq("-Xlint", "-Ywarn-dead-code"),
assemblyJarName in assembly := "scalafmt.jar",
testOptions in Test += Tests.Argument("-oD")
)
Expand Down
3 changes: 2 additions & 1 deletion project/plugins.sbt
Expand Up @@ -6,7 +6,8 @@ resolvers ++= Seq(
addSbtPlugin("org.foundweekends" % "sbt-bintray" % "0.5.1")
addSbtPlugin("com.dwijnand" % "sbt-dynver" % "1.2.0")
addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.7.0")
addSbtPlugin("io.get-coursier" % "sbt-coursier" % "1.0.0-RC12")
addSbtPlugin(
"io.get-coursier" % "sbt-coursier" % coursier.util.Properties.version)
addSbtPlugin("com.eed3si9n" % "sbt-doge" % "0.1.5")
addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "0.14.5")
addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.2.27")
Expand Down
2 changes: 1 addition & 1 deletion project/project/plugins.sbt
@@ -1 +1 @@
addSbtPlugin("io.get-coursier" % "sbt-coursier" % "1.0.0-RC3")
addSbtPlugin("io.get-coursier" % "sbt-coursier" % "1.0.2")
7 changes: 7 additions & 0 deletions readme/Changelog10.scalatex
Expand Up @@ -7,6 +7,13 @@
@ul
@li
Please document your contribution here.
@li
@pr(1122) Update @code{RedundantBraces} to remove redundant braces from
all expressions. If you'd like to restrict @code{RedundantBraces} behaviour
to just method bodies and/or string interpolation as it was before this
change, you can do so via new setting:
@code{rewrite.redundantBraces.generalExpressions = false}
By @user{japgolly}.
@sect{1.4.0}
@p
See @lnk("merged PRs", "https://github.com/scalameta/scalafmt/milestone/19?closed=1").
Expand Down
Expand Up @@ -4,7 +4,8 @@ import metaconfig._

@DeriveConfDecoder
case class RedundantBracesSettings(
methodBodies: Boolean = true,
includeUnitMethods: Boolean = true,
maxLines: Int = 100,
stringInterpolation: Boolean = false
)
stringInterpolation: Boolean = false,
generalExpressions: Boolean = true)
@@ -0,0 +1,10 @@
package org.scalafmt.internal

sealed abstract class Side {
def isLeft: Boolean = this == Side.Left
}

object Side {
case object Right extends Side
case object Left extends Side
}
@@ -0,0 +1,71 @@
package org.scalafmt.internal

import scala.meta.Tree

sealed trait SyntacticGroup {
def categories: List[String]
def precedence: Double
}

object SyntacticGroup {
sealed trait Type extends SyntacticGroup {
def categories = List("Type")
}
object Type {
case object ParamTyp extends Type { def precedence = 0 }
case object Typ extends Type { def precedence = 1 }
case object AnyInfixTyp extends Type { def precedence = 1.5 }
case class InfixTyp(operator: String) extends Type { def precedence = 2 }
case object RefineTyp extends Type { def precedence = 3 }
case object WithTyp extends Type { def precedence = 3.5 }
case object AnnotTyp extends Type { def precedence = 4 }
case object SimpleTyp extends Type { def precedence = 6 }
}
sealed trait Term extends SyntacticGroup {
def categories = List("Term")
}
object Term {
case object Expr extends Term { def precedence = 0 }
case object Expr1 extends Term { def precedence = 1 }
case object Ascription extends Term { def precedence = 2 }
case object PostfixExpr extends Term { def precedence = 2 }
case class InfixExpr(operator: String) extends Term { def precedence = 3 }
case class PrefixExpr(operator: String) extends Term { def precedence = 4 }
object PrefixArg {
def apply(tree: Tree): PrefixArg =
PrefixArg(tree, TreeSyntacticGroup(tree))
}
case class PrefixArg(tree: Tree, innerGroup: SyntacticGroup) extends Term {
def precedence = innerGroup.precedence
}
case object SimpleExpr extends Term { def precedence = 5 }
case object SimpleExpr1 extends Term { def precedence = 6 }
}
sealed trait Pat extends SyntacticGroup {
def categories = List("Pat")
}
object Pat {
case object Pattern extends Pat { def precedence = 0 }
case object Pattern1 extends Pat { def precedence = 1 }
case object Pattern2 extends Pat { def precedence = 2 }
case object AnyPattern3 extends Pat { def precedence = 2.5 }
case class Pattern3(operator: String) extends Pat { def precedence = 3 }
case object SimplePattern extends Pat { def precedence = 6 }
}
case object Literal extends Term with Pat {
override def categories = List("Term", "Pat"); def precedence = 6
}
require(
Literal.precedence == Term.SimpleExpr1.precedence &&
Literal.precedence == Pat.SimplePattern.precedence
)
case object Path extends Type with Term with Pat {
override def categories = List("Type", "Term", "Pat");
def precedence = 6
}
require(
Path.precedence == Type.SimpleTyp.precedence &&
Path.precedence == Term.SimpleExpr1.precedence &&
Path.precedence == Pat.SimplePattern.precedence
)
}
@@ -0,0 +1,101 @@
package org.scalafmt.internal

import scala.meta.{Term, Tree, Lit}
import org.scalafmt.internal.{SyntacticGroup => g}
import scala.meta.internal.ast.Helpers._

object SyntacticGroupOps {

def operatorNeedsParenthesis(
outerOperator: String,
innerOperator: String,
customAssociativity: Boolean,
customPrecedence: Boolean,
side: Side,
forceRight: Boolean = false
): Boolean = {

// The associativity of an operator is determined by the operator's last character.
// Operators ending in a colon ‘:’ are right-associative. All
// other operators are left-associative.
// https://www.scala-lang.org/files/archive/spec/2.13/06-expressions.html#infix-operations
def isLeftAssociative(name: String): Boolean =
if (customAssociativity) name.last != ':' else true

def precedence(name: String): Int =
if (customPrecedence) Term.Name(name).precedence else 0

val outerOperatorIsLeftAssociative = isLeftAssociative(outerOperator)
val innerOperatorIsLeftAssociative = isLeftAssociative(innerOperator)

if (outerOperatorIsLeftAssociative ^ innerOperatorIsLeftAssociative) true
else {
val isLeft = outerOperatorIsLeftAssociative
val isRight = !outerOperatorIsLeftAssociative

val outerOperatorPrecedence = precedence(outerOperator)
val innerOperatorPrecedence = precedence(innerOperator)

if (outerOperatorPrecedence < innerOperatorPrecedence) {
isRight
} else if (outerOperatorPrecedence == innerOperatorPrecedence) {
isLeft ^ side.isLeft
} else {
isLeft || forceRight
}
}
}

def startsWithNumericLiteral(tree: Tree): Boolean = {
tree match {
case _: Lit.Int | _: Lit.Long | _: Lit.Double | _: Lit.Float |
_: Lit.Byte | _: Lit.Short =>
true
case Term.Select(tree0, _) => startsWithNumericLiteral(tree0)
case _ => false
}
}

def groupNeedsParenthesis(
outerGroup: SyntacticGroup,
innerGroup: SyntacticGroup,
side: Side
): Boolean = (outerGroup, innerGroup) match {
case (g.Term.InfixExpr(outerOperator), g.Term.InfixExpr(innerOperator)) =>
operatorNeedsParenthesis(
outerOperator,
innerOperator,
customAssociativity = true,
customPrecedence = true,
side,
forceRight = true
)
case (g.Type.InfixTyp(outerOperator), g.Type.InfixTyp(innerOperator)) =>
operatorNeedsParenthesis(
outerOperator,
innerOperator,
customAssociativity = true,
customPrecedence = false,
side
)
case (g.Pat.Pattern3(outerOperator), g.Pat.Pattern3(innerOperator)) =>
operatorNeedsParenthesis(
outerOperator,
innerOperator,
customAssociativity = true,
customPrecedence = true,
side
)

case (_: g.Term.PrefixExpr, g.Term.PrefixArg(_, _: g.Term.PrefixExpr)) =>
true

case (g.Term.PrefixExpr("-"), g.Term.PrefixArg(Term.Select(tree, _), _))
if startsWithNumericLiteral(tree) =>
true

case _ =>
outerGroup.precedence > innerGroup.precedence
}

}
@@ -0,0 +1,81 @@
package org.scalafmt.internal

import scala.meta.Lit
import scala.meta.Pat
import scala.meta.Term
import scala.meta.Tree
import scala.meta.Type
import org.scalafmt.internal.{SyntacticGroup => g}

object TreeSyntacticGroup {
def apply(tree: Tree): SyntacticGroup = tree match {
case _: Lit => g.Literal
// Term
case _: Term.Name => g.Path
case _: Term.Select => g.Path
case _: Term.Interpolate => g.Term.SimpleExpr1
case _: Term.Xml => g.Term.SimpleExpr1
case _: Term.Apply => g.Term.SimpleExpr1
case _: Term.ApplyType => g.Term.SimpleExpr1
case t: Term.ApplyInfix => g.Term.InfixExpr(t.op.value)
case t: Term.ApplyUnary => g.Term.PrefixExpr(t.op.value)
case _: Term.Assign => g.Term.Expr1
case _: Term.Return => g.Term.Expr1
case _: Term.Throw => g.Term.Expr1
case _: Term.Ascribe => g.Term.Expr1
case _: Term.Annotate => g.Term.Expr1
case _: Term.Block => g.Term.SimpleExpr1
case _: Term.Tuple => g.Term.SimpleExpr1 // ???, breaks a op ((b, c))
// case _: Term.Tuple => g.Term.Expr1 // ??? Was SimpleExpr1, which is buggy for `a op ((b, c))
case _: Term.If => g.Term.Expr1
case _: Term.Match => g.Term.Expr1
case _: Term.TryWithCases => g.Term.Expr1
case _: Term.TryWithTerm => g.Term.Expr1
case _: Term.Function => g.Term.Expr
case _: Term.PartialFunction => g.Term.SimpleExpr
case _: Term.While => g.Term.Expr1
case _: Term.Do => g.Term.Expr1
case _: Term.For => g.Term.Expr1
case _: Term.ForYield => g.Term.Expr1
case _: Term.New => g.Term.SimpleExpr
case _: Term.Placeholder => g.Term.SimpleExpr1
case _: Term.Eta => g.Term.SimpleExpr
case _: Term.Arg.Repeated => g.Term.PostfixExpr
case _: Term.Param => g.Path // ???
// Type
case _: Type.Name => g.Path
case _: Type.Select => g.Type.SimpleTyp
case _: Type.Project => g.Type.SimpleTyp
case _: Type.Singleton => g.Type.SimpleTyp
case _: Type.Apply => g.Type.SimpleTyp
case t: Type.ApplyInfix => g.Type.InfixTyp(t.op.value)
case _: Type.Function => g.Type.Typ
case _: Type.Tuple => g.Type.SimpleTyp
case _: Type.With => g.Type.WithTyp
case _: Type.And => g.Type.InfixTyp("&")
case _: Type.Or => g.Type.InfixTyp("|")
case _: Type.Refine => g.Type.RefineTyp
case _: Type.Existential => g.Type.Typ
case _: Type.Annotate => g.Type.AnnotTyp
case _: Type.Placeholder => g.Type.SimpleTyp
case _: Type.Bounds => g.Path // ???
case _: Type.Arg.Repeated => g.Type.ParamTyp
case _: Type.Arg.ByName => g.Type.ParamTyp
case _: Pat.Var.Type => g.Type.ParamTyp
case _: Type.Param => g.Path // ???
// Pat
case _: Pat.Var => g.Pat.SimplePattern
case _: Pat.Wildcard => g.Pat.SimplePattern
case _: Pat.Bind => g.Pat.Pattern2
case _: Pat.Alternative => g.Pat.Pattern
case _: Pat.Tuple => g.Pat.SimplePattern
case _: Pat.Extract => g.Pat.SimplePattern
case t: Pat.ExtractInfix => g.Pat.Pattern3(t.op.value)
case _: Pat.Interpolate => g.Pat.SimplePattern
case _: Pat.Xml => g.Pat.SimplePattern
case _: Pat.Typed => g.Pat.Pattern1

// Misc
case _ => g.Path
}
}

0 comments on commit 8f1c4ba

Please sign in to comment.