diff --git a/scalariform/src/main/scala/scalariform/formatter/ExprFormatter.scala b/scalariform/src/main/scala/scalariform/formatter/ExprFormatter.scala index d5c61382..f1762bf6 100755 --- a/scalariform/src/main/scala/scalariform/formatter/ExprFormatter.scala +++ b/scalariform/src/main/scala/scalariform/formatter/ExprFormatter.scala @@ -475,7 +475,7 @@ trait ExprFormatter { self: HasFormattingPreferences with AnnotationFormatter wi (formattingPreferences(DanglingCloseParenthesis) == Preserve) && hiddenPredecessors(rparen).containsNewline && contents.nonEmpty - if (firstTokensAreOnNewline || shouldPreserveNewline) + if (firstTokensAreOnNewline || shouldPreserveNewline || parenArguments.hasTrailingComma) formatResult ++= formatResult.before(rparen, formatterState.currentIndentLevelInstruction) formatResult @@ -1211,23 +1211,34 @@ trait ExprFormatter { self: HasFormattingPreferences with AnnotationFormatter wi } } - // Handle placing the first argument on a newline, if requested. - if (multilineArguments) { - formattingPreferences(FirstParameterOnNewline) match { - case Force => - formatResult ++= formatResult.before( - firstParamOption.get.firstToken, - paramFormatterState.indent(paramIndent).currentIndentLevelInstruction - ) - case Prevent => - formatResult ++= formatResult.before(firstParamOption.get.firstToken, Compact) - case Preserve => // no-op. + val hasTrailingComma = paramClause.trailComa.nonEmpty + + if (hasTrailingComma) { + // Trailing comma should always trigger new line for the first arg. + formatResult ++= formatResult.before( + firstParamOption.get.firstToken, + paramFormatterState.indent(paramIndent).currentIndentLevelInstruction + ) + } else { + // Handle placing the first argument on a newline, if requested. + if (multilineArguments) { + formattingPreferences(FirstParameterOnNewline) match { + case Force => + formatResult ++= formatResult.before( + firstParamOption.get.firstToken, + paramFormatterState.indent(paramIndent).currentIndentLevelInstruction + ) + case Prevent => + formatResult ++= formatResult.before(firstParamOption.get.firstToken, Compact) + case Preserve => // no-op. + } } } val hasContent = implicitOption.isDefined || firstParamOption.isDefined val shouldIndentParen = hiddenPredecessors(rparen).containsComment || + hasTrailingComma || ((hiddenPredecessors(rparen).containsNewline && formattingPreferences(DanglingCloseParenthesis) == Preserve) || formattingPreferences(DanglingCloseParenthesis) == Force) @@ -1237,7 +1248,7 @@ trait ExprFormatter { self: HasFormattingPreferences with AnnotationFormatter wi // Place rparen on its own line if this is a multi-line param clause with content, and the // preferences say to. - if ((multilineArguments || firstTokenIsOnNewline) && hasContent && shouldIndentParen) + if ((multilineArguments || firstTokenIsOnNewline || hasTrailingComma) && hasContent && shouldIndentParen) formatResult = formatResult.before(rparen, paramFormatterState.currentIndentLevelInstruction) val groupedParams = groupParams(paramClause, alignParameters) diff --git a/scalariform/src/main/scala/scalariform/parser/AstNodes.scala b/scalariform/src/main/scala/scalariform/parser/AstNodes.scala index 8f474660..db590ee0 100644 --- a/scalariform/src/main/scala/scalariform/parser/AstNodes.scala +++ b/scalariform/src/main/scala/scalariform/parser/AstNodes.scala @@ -1,6 +1,6 @@ package scalariform.parser -import scalariform.lexer.Token +import scalariform.lexer.{ Token, Tokens } import scalariform.utils.Range sealed trait AstNode extends Product { @@ -154,6 +154,8 @@ case class BlockArgumentExprs(contents: List[ExprElement]) extends ArgumentExprs case class ParenArgumentExprs(lparen: Token, contents: List[ExprElement], rparen: Token) extends ArgumentExprs { lazy val tokens: List[Token] = flatten(lparen, contents, rparen) + + lazy val hasTrailingComma: Boolean = flatten(contents).lastOption.exists(_.tokenType == Tokens.COMMA) } case class Argument(expr: Expr) extends AstNode with ExprElement { @@ -303,7 +305,7 @@ case class ParamClauses(newlineOpt: Option[Token], paramClausesAndNewlines: List } case class ParamClause(lparen: Token, implicitOption: Option[Token], firstParamOption: Option[Param], otherParams: List[(Token, Param)], rparen: Token, trailComa: Option[Token]) extends AstNode { - lazy val tokens: List[Token] = flatten(lparen, implicitOption, firstParamOption, otherParams, rparen) + lazy val tokens: List[Token] = flatten(lparen, implicitOption, firstParamOption, otherParams, trailComa, rparen) } case class Param(annotations: List[Annotation], modifiers: List[Modifier], valOrVarOpt: Option[Token], id: Token, paramTypeOpt: Option[(Token, Type)], defaultValueOpt: Option[(Token, Expr)]) extends AstNode { @@ -378,7 +380,7 @@ case class TemplateParents(typeAndArgs: (Type, List[ArgumentExprs]), withTypesAn } case class ImportClause(importToken: Token, importExpr: ImportExpr, otherImportExprs: List[(Token, ImportExpr)], trailOpt: Option[Token]) extends AstNode with Stat { - lazy val tokens: List[Token] = flatten(importToken, importExpr, otherImportExprs) + lazy val tokens: List[Token] = flatten(importToken, importExpr, otherImportExprs, trailOpt) } sealed trait ImportExpr extends AstNode diff --git a/scalariform/src/test/scala/scalariform/formatter/TrailingCommasTest.scala b/scalariform/src/test/scala/scalariform/formatter/TrailingCommasTest.scala index a56d4fc8..ffd0b007 100644 --- a/scalariform/src/test/scala/scalariform/formatter/TrailingCommasTest.scala +++ b/scalariform/src/test/scala/scalariform/formatter/TrailingCommasTest.scala @@ -1,15 +1,123 @@ package scalariform.formatter + import scalariform.parser._ // format: OFF class TrailingCommasTest extends AbstractFormatterTest { + // Trailing comma must always be followed by a newline. + // Scalariform will insert a newline if it is missing, to make + // such code valid Scala. + + // The test cases with List() cover trailing commas in function invocations. + + "val x = List()" ==> "val x = List()" + "val x = List(1)" ==> "val x = List(1)" + "val x = List(1, 2)" ==> "val x = List(1, 2)" + + "val x = List(1,)" ==> + """val x = List(1, + |)""" + + """val x = List( + | 1 + |)""" ==> + """val x = List( + | 1)""" + + """val x = List( + | 1, + |)""" ==> + """val x = List( + | 1, + |)""" + + "val x = List(1, 2,)" ==> + """val x = List(1, 2, + |)""" + + """val x = List( + | 1, 2 + |)""" ==> + """val x = List( + | 1, 2)""" + + """val x = List( + | 1, 2, + |)""" ==> + """val x = List( + | 1, 2, + |)""" + + """val x = List( + | 1, + | 2, + |)""" ==> + """val x = List( + | 1, + | 2, + |)""" + + """val x = List( + | 1, + | 2 + |)""" ==> + """val x = List( + | 1, + | 2)""" + + """val x = List( + | 1, + | 2,)""" ==> + """val x = List( + | 1, + | 2, + |)""" + + // The test cases with case class cover trailing commas in parameter declarations. + + "case class A" ==> "case class A" + "case class A()" ==> "case class A()" + "case class A(x: Int)" ==> "case class A(x: Int)" + "case class A(x: Int,)" ==> + """case class A( + | x: Int, + |)""" + + "case class A(x: Int, y: Int)" ==> "case class A(x: Int, y: Int)" + "case class A(x: Int, y: Int,)" ==> + """case class A( + | x: Int, y: Int, + |)""" + + """case class A( + | x: Int, + | y: Int, + |)""" ==> + """case class A( + | x: Int, + | y: Int, + |)""" + + """case class A( + | x: Int, + | y: Int,)""" ==> + """case class A( + | x: Int, + | y: Int, + |)""" + + """case class A( + | x: Int, y: Int, + |)""" ==> + """case class A( + | x: Int, y: Int, + |)""" - """val foos = List( - | f1, - | f2,)""" ==> - """val foos = List( - | f1, - | f2, )""" + """case class A( + | x: Int, y: Int,)""" ==> + """case class A( + | x: Int, y: Int, + |)""" // format: ON