Always break get/set definitions onto lines below the property declaration. (#279)

Summary:
This change fixes a crash that occurs on code like `var foo: Int; get() = 0; set(x) { }`

Pull Request resolved: https://github.com/facebookincubator/ktfmt/pull/279

Test Plan:
ktfmt-diff: D34172040

The diff has a nice side effect: it removes semicolons before comments (didn't happen before)

Reviewed By: strulovich

Differential Revision: D34150588

Pulled By: cgrushko

fbshipit-source-id: 4f59224370804cc0852bf0e00078b9fd3cd2c21c

3 files changed, 28 insertions, 32 deletions

diff --git a/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt b/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt
index c9293fa..d50c784 100644
--- a/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt
+++ b/core/src/main/java/com/facebook/ktfmt/format/Formatter.kt
@@ -82,7 +82,12 @@ object Formatter {
     val lfCode = StringUtilRt.convertLineSeparators(code)
     val sortedImports = sortedAndDistinctImports(lfCode)
     val pretty = prettyPrint(sortedImports, options, "\n")
-    val noRedundantElements = dropRedundantElements(pretty, options)
+    val noRedundantElements =
+        try {
+          dropRedundantElements(pretty, options)
+        } catch (e: ParseError) {
+          throw IllegalStateException("Failed to re-parse code after pretty printing:\n $pretty", e)
+        }
     return prettyPrint(noRedundantElements, options, Newlines.guessLineSeparator(code)!!)
   }
 
diff --git a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt
index f244a66..d21fa0c 100644
--- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt
+++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt
@@ -1218,7 +1218,6 @@ class KotlinInputAstVisitor(
     }
 
     // for example `by lazy { compute() }`
-    var hasSemicolon = false
     if (delegate != null) {
       builder.space()
       builder.token("by")
@@ -1229,7 +1228,6 @@ class KotlinInputAstVisitor(
         builder.breakOp(Doc.FillMode.UNIFIED, " ", expressionBreakIndent)
         builder.block(expressionBreakIndent) { visit(delegate) }
       }
-      hasSemicolon = delegate.nextSibling?.text == ";" == true
     } else if (initializer != null) {
       builder.space()
       builder.token("=")
@@ -1239,40 +1237,39 @@ class KotlinInputAstVisitor(
         builder.breakOp(Doc.FillMode.UNIFIED, " ", expressionBreakIndent)
         builder.block(expressionBreakIndent) { visit(initializer) }
       }
-      hasSemicolon = initializer.nextSibling?.text == ";" == true
     }
     if (name != null) {
       builder.close() // close block for named values
     }
-    if (hasSemicolon) {
-      builder.token(";")
-    }
     // for example `private set` or `get = 2 * field`
     if (accessors?.isNotEmpty() == true) {
-      for (accessor in accessors) {
-        builder.block(blockIndent) {
-          if (hasSemicolon) {
-            builder.space()
-          } else {
-            builder.forcedBreak()
+      builder.block(blockIndent) {
+        for (accessor in accessors) {
+          builder.forcedBreak()
+          // The semicolon must come after the newline, or the output code will not parse.
+          builder.guessToken(";")
+
+          builder.block(ZERO) {
+            visitFunctionLikeExpression(
+                accessor.modifierList,
+                accessor.namePlaceholder.text,
+                null,
+                null,
+                null,
+                accessor.bodyExpression != null || accessor.bodyBlockExpression != null,
+                accessor.parameterList,
+                null,
+                accessor.bodyBlockExpression,
+                accessor.bodyExpression,
+                accessor.returnTypeReference,
+                accessor.bodyBlockExpression?.lBrace != null)
           }
-          visitFunctionLikeExpression(
-              accessor.modifierList,
-              accessor.namePlaceholder.text,
-              null,
-              null,
-              null,
-              accessor.bodyExpression != null || accessor.bodyBlockExpression != null,
-              accessor.parameterList,
-              null,
-              accessor.bodyBlockExpression,
-              accessor.bodyExpression,
-              accessor.returnTypeReference,
-              accessor.bodyBlockExpression?.lBrace != null)
         }
       }
     }
 
+    builder.guessToken(";")
+
     if (isField) {
       builder.blankLineWanted(OpsBuilder.BlankLineWanted.conditional(verticalAnnotationBreak))
     }
diff --git a/core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt b/core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt
index c7f5c8f..344a467 100644
--- a/core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt
+++ b/core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt
@@ -69,12 +69,6 @@ internal class RedundantSemicolonDetector {
       return false // Example: `foo(0); { dead -> lambda }`
     }
 
-    // do not remove `;` in `val a = 5; private set`
-    val nextSibling = element.nextSibling
-    if (nextSibling != null && '\n' !in nextSibling.text) {
-      return false
-    }
-
     return true
   }
 }