Add new experimental rule blank-line-before-declaration. This rule …

…requires a blank line before class, function or property declarations Closes #1939
pinterest · May 24, 2023 · cfa6414 · cfa6414
1 parent 0779055
commit cfa6414
Show file tree

Hide file tree

Showing 19 changed files with 527 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 
 ### Added
 
+* Add new experimental rule `blank-line-before-declaration`. This rule requires a blank line before class, function or property declarations ([#1939](https://github.com/pinterest/ktlint/issues/1939))
+
 ### Removed
 
 ### Fixed

diff --git a/documentation/snapshot/docs/rules/experimental.md b/documentation/snapshot/docs/rules/experimental.md
@@ -8,6 +8,56 @@ ktlint_experimental=enabled
 ```
 Also see [enable/disable specific rules](../configuration-ktlint/#disabled-rules).
 
+## Blank line before declarations
+
+Requires a blank line before any class or function declaration. No blank line is required between the class signature and the first declaration in the class. In a similar way, a blank line is required before any list of top level or class properties. No blank line is required before local properties or between consecutive properties.
+
+=== "[:material-heart:](#) Ktlint"
+
+    ```kotlin
+    const val foo1 = "foo1"
+
+    class FooBar {
+        val foo2 = "foo2"
+        val foo3 = "foo3"
+
+        fun bar1() {
+           val foo4 = "foo4"
+           val foo5 = "foo5"
+        }
+
+        fun bar2() = "bar"
+
+        val foo6 = "foo3"
+        val foo7 = "foo4"
+
+        enum class Foo {}
+    }
+    ```
+
+=== "[:material-heart-off-outline:](#) Disallowed"
+
+    ```kotlin
+    const val foo1 = "foo1"
+    class FooBar {
+        val foo2 = "foo2"
+        val foo3 = "foo3"
+        fun bar1() {
+           val foo4 = "foo4"
+           val foo5 = "foo5"
+        }
+        fun bar2() = "bar"
+        val foo6 = "foo3"
+        val foo7 = "foo4"
+        enum class Foo {}
+    }
+    ```
+
+Rule id: `blank-line-before-declaration` (`standard` rule set)
+
+!!! Note
+This rule is only run when `ktlint_code_style` is set to `ktlint_official` or when the rule is enabled explicitly.
+
 ## Discouraged comment location
 
 Detect discouraged comment locations (no autocorrect).

diff --git a/...ne/src/main/kotlin/com/pinterest/ktlint/cli/reporter/baseline/BaselineReporterProvider.kt b/...ne/src/main/kotlin/com/pinterest/ktlint/cli/reporter/baseline/BaselineReporterProvider.kt
@@ -5,6 +5,7 @@ import java.io.PrintStream
 
 public class BaselineReporterProvider : ReporterProviderV2<BaselineReporter> {
     override val id: String = "baseline"
+
     override fun get(
         out: PrintStream,
         opt: Map<String, String>,

diff --git a/...rc/main/kotlin/com/pinterest/ktlint/cli/reporter/checkstyle/CheckStyleReporterProvider.kt b/...rc/main/kotlin/com/pinterest/ktlint/cli/reporter/checkstyle/CheckStyleReporterProvider.kt
@@ -5,6 +5,7 @@ import java.io.PrintStream
 
 public class CheckStyleReporterProvider : ReporterProviderV2<CheckStyleReporter> {
     override val id: String = "checkstyle"
+
     override fun get(
         out: PrintStream,
         opt: Map<String, String>,

diff --git a/...ter-core/src/main/kotlin/com/pinterest/ktlint/cli/reporter/core/api/ReporterProviderV2.kt b/...ter-core/src/main/kotlin/com/pinterest/ktlint/cli/reporter/core/api/ReporterProviderV2.kt
@@ -11,6 +11,7 @@ import java.io.Serializable
  */
 public interface ReporterProviderV2<T : ReporterV2> : Serializable {
     public val id: String
+
     public fun get(
         out: PrintStream,
         opt: Map<String, String>,

diff --git a/...orter-html/src/main/kotlin/com/pinterest/ktlint/cli/reporter/html/HtmlReporterProvider.kt b/...orter-html/src/main/kotlin/com/pinterest/ktlint/cli/reporter/html/HtmlReporterProvider.kt
@@ -29,6 +29,7 @@ import java.io.PrintStream
 
 public class HtmlReporterProvider : ReporterProviderV2<HtmlReporter> {
     override val id: String = "html"
+
     override fun get(
         out: PrintStream,
         opt: Map<String, String>,

diff --git a/...orter-json/src/main/kotlin/com/pinterest/ktlint/cli/reporter/json/JsonReporterProvider.kt b/...orter-json/src/main/kotlin/com/pinterest/ktlint/cli/reporter/json/JsonReporterProvider.kt
@@ -5,6 +5,7 @@ import java.io.PrintStream
 
 public class JsonReporterProvider : ReporterProviderV2<JsonReporter> {
     override val id: String = "json"
+
     override fun get(
         out: PrintStream,
         opt: Map<String, String>,

diff --git a/...i-reporter-sarif/src/main/kotlin/com/pinterest/ktlint/cli/reporter/sarif/SarifReporter.kt b/...i-reporter-sarif/src/main/kotlin/com/pinterest/ktlint/cli/reporter/sarif/SarifReporter.kt
@@ -23,6 +23,7 @@ import kotlin.io.path.pathString
 import kotlin.io.path.relativeToOrSelf
 
 private const val SRCROOT = "%SRCROOT%"
+
 internal fun String.sanitize(): String =
     this.replace(File.separatorChar, '/')
         .let {

diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/ReporterProvider.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/ReporterProvider.kt
@@ -12,6 +12,7 @@ import java.io.Serializable
 @Deprecated("Deprecated since ktlint 0.49.0. Custom reporters have to be migrated to ReporterV2. See changelog 0.49.")
 public interface ReporterProvider<T : Reporter> : Serializable {
     public val id: String
+
     public fun get(
         out: PrintStream,
         opt: Map<String, String>,

diff --git a/...engine-core/src/main/kotlin/com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtension.kt b/...engine-core/src/main/kotlin/com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtension.kt
@@ -235,6 +235,7 @@ public fun ASTNode?.isWhiteSpaceWithNewline(): Boolean = this != null && element
 public fun ASTNode?.isWhiteSpaceWithoutNewline(): Boolean = this != null && elementType == WHITE_SPACE && !textContains('\n')
 
 public fun ASTNode.isRoot(): Boolean = elementType == ElementType.FILE
+
 public fun ASTNode.isLeaf(): Boolean = firstChildNode == null
 
 /**

diff --git a/...-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/KotlinPsiFileFactory.kt b/...-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/KotlinPsiFileFactory.kt
@@ -103,6 +103,7 @@ private class LoggerFactory : DiagnosticLogger.Factory {
                 message: String?,
                 t: Throwable?,
             ) {}
+
             override fun error(
                 message: String?,
                 vararg details: String?,

diff --git a/...engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/PositionInTextLocator.kt b/...engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/PositionInTextLocator.kt
@@ -49,6 +49,7 @@ private class SegmentTree(
             }
 
     fun get(i: Int): Segment = segments[i]
+
     fun indexOf(v: Int): Int = binarySearch(v, 0, segments.size - 1)
 
     private fun binarySearch(

diff --git a/...rc/main/kotlin/com/pinterest/ktlint/rule/engine/internal/rulefilter/RunAfterRuleFilter.kt b/...rc/main/kotlin/com/pinterest/ktlint/rule/engine/internal/rulefilter/RunAfterRuleFilter.kt
@@ -166,6 +166,7 @@ internal class RunAfterRuleFilter : RuleFilter {
                 }
             }.toSet()
     }
+
     private fun RuleProvider.canRunWith(loadedRuleIds: Set<RuleId>): Boolean =
         this
             .runAfterRules

diff --git a/ktlint-rule-engine/src/test/kotlin/com/pinterest/ktlint/rule/engine/api/KtLintTest.kt b/ktlint-rule-engine/src/test/kotlin/com/pinterest/ktlint/rule/engine/api/KtLintTest.kt
@@ -609,6 +609,7 @@ private data class RuleExecutionCall(
     val classIdentifier: String? = null,
 ) {
     enum class RuleMethod { BEFORE_FIRST, BEFORE_CHILDREN, VISIT, AFTER_CHILDREN, AFTER_LAST }
+
     enum class VisitNodeType { ROOT, CHILD }
 }
 

diff --git a/...ngine/src/test/kotlin/com/pinterest/ktlint/rule/engine/internal/RuleProviderSorterTest.kt b/...ngine/src/test/kotlin/com/pinterest/ktlint/rule/engine/internal/RuleProviderSorterTest.kt
@@ -455,6 +455,7 @@ class RuleProviderSorterTest {
         visitorModifiers,
     ) {
         constructor(ruleId: RuleId, visitorModifier: VisitorModifier) : this(ruleId, setOf(visitorModifier))
+
         override fun beforeVisitChildNodes(
             node: ASTNode,
             autoCorrect: Boolean,

diff --git a/.../test/kotlin/com/pinterest/ktlint/rule/engine/internal/ThreadSafeEditorConfigCacheTest.kt b/.../test/kotlin/com/pinterest/ktlint/rule/engine/internal/ThreadSafeEditorConfigCacheTest.kt
@@ -86,6 +86,7 @@ class ThreadSafeEditorConfigCacheTest {
         const val SOME_PROPERTY = "some-property"
 
         private fun String.resource() = Resource.Resources.ofPath(Paths.get(this), StandardCharsets.UTF_8)
+
         val FILE_1: Resource = "/some/path/to/file/1".resource()
         val FILE_2: Resource = "/some/path/to/file/2".resource()
 

diff --git a/...standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt b/...standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt
@@ -6,6 +6,7 @@ import com.pinterest.ktlint.rule.engine.core.api.RuleSetId
 import com.pinterest.ktlint.ruleset.standard.rules.AnnotationRule
 import com.pinterest.ktlint.ruleset.standard.rules.AnnotationSpacingRule
 import com.pinterest.ktlint.ruleset.standard.rules.ArgumentListWrappingRule
+import com.pinterest.ktlint.ruleset.standard.rules.BlankLineBeforeDeclarationRule
 import com.pinterest.ktlint.ruleset.standard.rules.BlockCommentInitialStarAlignmentRule
 import com.pinterest.ktlint.ruleset.standard.rules.ChainWrappingRule
 import com.pinterest.ktlint.ruleset.standard.rules.ClassNamingRule
@@ -88,6 +89,7 @@ public class StandardRuleSetProvider :
             RuleProvider { AnnotationRule() },
             RuleProvider { AnnotationSpacingRule() },
             RuleProvider { ArgumentListWrappingRule() },
+            RuleProvider { BlankLineBeforeDeclarationRule() },
             RuleProvider { BlockCommentInitialStarAlignmentRule() },
             RuleProvider { ChainWrappingRule() },
             RuleProvider { ClassNamingRule() },

diff --git a/...main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBeforeDeclarationRule.kt b/...main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBeforeDeclarationRule.kt
@@ -0,0 +1,113 @@
+package com.pinterest.ktlint.ruleset.standard.rules
+
+import com.pinterest.ktlint.rule.engine.core.api.ElementType
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.BLOCK
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.LBRACE
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.PROPERTY
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.PROPERTY_ACCESSOR
+import com.pinterest.ktlint.rule.engine.core.api.Rule
+import com.pinterest.ktlint.rule.engine.core.api.RuleId
+import com.pinterest.ktlint.rule.engine.core.api.indent
+import com.pinterest.ktlint.rule.engine.core.api.isPartOfComment
+import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace
+import com.pinterest.ktlint.rule.engine.core.api.nextCodeSibling
+import com.pinterest.ktlint.rule.engine.core.api.prevCodeSibling
+import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe
+import com.pinterest.ktlint.ruleset.standard.StandardRule
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+import org.jetbrains.kotlin.psi.psiUtil.siblings
+
+/**
+ * Insert a blank line before declarations. No blank line is inserted before between the class signature and the first declaration in the
+ * class. Also, no blank lines are inserted between consecutive properties.
+ */
+public class BlankLineBeforeDeclarationRule :
+    StandardRule("blank-line-before-declaration"),
+    Rule.Experimental,
+    Rule.OfficialCodeStyle {
+    override fun beforeVisitChildNodes(
+        node: ASTNode,
+        autoCorrect: Boolean,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+    ) {
+        when (node.elementType) {
+            CLASS,
+            FUN,
+            PROPERTY,
+            PROPERTY_ACCESSOR,
+            ->
+                visitDeclaration(node, autoCorrect, emit)
+        }
+    }
+
+    private fun visitDeclaration(
+        node: ASTNode,
+        autoCorrect: Boolean,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
+    ) {
+        if (node == node.firstCodeSiblingInClassBodyOrNull()) {
+            // Allow missing blank line between class signature and first code sibling in class body:
+            //   Class Foo {
+            //      fun bar() {}
+            //   }
+            return
+        }
+
+        if (node.isConsecutiveProperty()) {
+            // Allow consecutive properties:
+            //   val foo = "foo"
+            //   val bar = "bar"
+            return
+        }
+
+        if (node.isLocalProperty()) {
+            // Allow:
+            //   fun foo() {
+            //       bar()
+            //       val foobar = "foobar"
+            //   }
+            return
+        }
+
+        node
+            .siblings(false)
+            .takeWhile { it.isWhiteSpace() || it.isPartOfComment() }
+            .lastOrNull()
+            ?.let { previous ->
+                when {
+                    !previous.isWhiteSpace() -> previous
+                    !previous.text.startsWith("\n\n") -> node
+                    else -> null
+                }?.let { insertBeforeNode ->
+                    emit(insertBeforeNode.startOffset, "Expected a blank line for this declaration", true)
+                    if (autoCorrect) {
+                        insertBeforeNode.upsertWhitespaceBeforeMe("\n".plus(node.indent()))
+                    }
+                }
+            }
+    }
+
+    private fun ASTNode.firstCodeSiblingInClassBodyOrNull() =
+        treeParent
+            .takeIf { it.elementType == ElementType.CLASS_BODY }
+            ?.findChildByType(LBRACE)
+            ?.nextCodeSibling()
+
+    private fun ASTNode.isConsecutiveProperty() =
+        takeIf { it.propertyRelated() }
+            ?.prevCodeSibling()
+            ?.let { it.propertyRelated() || it.treeParent.propertyRelated() }
+            ?: false
+
+    private fun ASTNode.isLocalProperty() =
+        takeIf { it.propertyRelated() }
+            ?.treeParent
+            ?.let { it.elementType == BLOCK }
+            ?: false
+
+    private fun ASTNode.propertyRelated() = elementType == PROPERTY || elementType == PROPERTY_ACCESSOR
+}
+
+public val BLANK_LINE_BEFORE_DECLARATION_RULE_ID: RuleId = BlankLineBeforeDeclarationRule().ruleId