Wrap annotated function parameters and projection types (#1910)

Wrap annotated function parameters to a separate line in code style `ktlint_official` only Closes #1908 Wrap annotated projection types in type argument lists to a separate line `annotation` Closes #1909
pinterest · Apr 2, 2023 · 9b9dbd8 · 9b9dbd8
1 parent 728fa00
commit 9b9dbd8
Show file tree

Hide file tree

Showing 8 changed files with 223 additions and 128 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -277,6 +277,8 @@ Previously the default value for `.editorconfig` property `max_line_length` was
 * Allow value arguments with a multiline expression to be indented on a separate line `indent` ([#1217](https://github.com/pinterest/ktlint/issues/1217))
 * When enabled, the ktlint rule checking is disabled for all code surrounded by the formatter tags (see [faq](https://pinterest.github.io/ktlint/faq/#are-formatter-tags-respected)) ([#670](https://github.com/pinterest/ktlint/issues/670)) 
 * Remove trailing comma if last two enum entries are on the same line and trailing commas are not allowed. `trailing-comma-on-declaration-site` ([#1905](https://github.com/pinterest/ktlint/issues/1905))
+* Wrap annotated function parameters to a separate line in code style `ktlint_official` only. `function-signature`, `parameter-list-wrapping` ([#1908](https://github.com/pinterest/ktlint/issues/1908))
+* Wrap annotated projection types in type argument lists to a separate line `annotation` ([#1909](https://github.com/pinterest/ktlint/issues/1909))
 
 ### Changed
 * Wrap the parameters of a function literal containing a multiline parameter list (only in `ktlint_official` code style) `parameter-list-wrapping` ([#1681](https://github.com/pinterest/ktlint/issues/1681)).

diff --git a/...et-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/AnnotationRule.kt b/...et-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/AnnotationRule.kt
@@ -6,7 +6,6 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.ANNOTATION
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.ANNOTATION_ENTRY
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.FILE_ANNOTATION_LIST
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.MODIFIER_LIST
-import com.pinterest.ktlint.rule.engine.core.api.ElementType.TYPE_ARGUMENT_LIST
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.TYPE_PROJECTION
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.TYPE_REFERENCE
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_ARGUMENT
@@ -18,24 +17,24 @@ import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRu
 import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED
 import com.pinterest.ktlint.rule.engine.core.api.RuleId
 import com.pinterest.ktlint.rule.engine.core.api.children
+import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CODE_STYLE_PROPERTY
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_SIZE_PROPERTY
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPERTY
 import com.pinterest.ktlint.rule.engine.core.api.firstChildLeafOrSelf
 import com.pinterest.ktlint.rule.engine.core.api.isCodeLeaf
-import com.pinterest.ktlint.rule.engine.core.api.isPartOf
 import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace
 import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline
 import com.pinterest.ktlint.rule.engine.core.api.lastChildLeafOrSelf
 import com.pinterest.ktlint.rule.engine.core.api.nextCodeLeaf
 import com.pinterest.ktlint.rule.engine.core.api.nextLeaf
+import com.pinterest.ktlint.rule.engine.core.api.prevCodeLeaf
 import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
 import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceAfterMe
 import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe
 import com.pinterest.ktlint.rule.engine.core.util.safeAs
 import com.pinterest.ktlint.ruleset.standard.StandardRule
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
-import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
 import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
 import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationUseSiteTarget
@@ -90,9 +89,11 @@ public class AnnotationRule :
                 ),
             ),
     ) {
+    private var codeStyle = CODE_STYLE_PROPERTY.defaultValue
     private var indentConfig = IndentConfig.DEFAULT_INDENT_CONFIG
 
     override fun beforeFirstNode(editorConfig: EditorConfig) {
+        codeStyle = editorConfig[CODE_STYLE_PROPERTY]
         indentConfig = IndentConfig(
             indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
             tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
@@ -127,11 +128,18 @@ public class AnnotationRule :
         require(node.elementType == ANNOTATION_ENTRY)
 
         if (node.isAnnotationEntryWithValueArgumentList() &&
-            node.treeParent.treeParent.elementType != VALUE_PARAMETER && // fun fn(@Ann("blah") a: String)
-            node.treeParent.treeParent.elementType != VALUE_ARGUMENT && // fn(@Ann("blah") "42")
-            !node.isPartOf(TYPE_ARGUMENT_LIST) && // val property: Map<@Ann("blah") String, Int>
+            node.treeParent.treeParent.elementType != VALUE_PARAMETER &&
+            node.treeParent.treeParent.elementType != VALUE_ARGUMENT &&
             node.isNotReceiverTargetAnnotation()
         ) {
+            // Disallow:
+            //     @Foo class Bar
+            // Allow function parameters and arguments to have annotation on same line as identifier:
+            //     fun @receiver:Bar String.foo() {}
+            //     fun foo(
+            //         @Bar("bar") bar,
+            //         @Bar("bar") "42",
+            //     ) {}
             checkForAnnotationWithParameterToBePlacedOnSeparateLine(node, emit, autoCorrect)
         }
 
@@ -154,13 +162,16 @@ public class AnnotationRule :
             }
 
             if (node.treeParent.elementType != ANNOTATION &&
+                node.treeParent.treeParent.elementType != VALUE_PARAMETER &&
+                node.treeParent.treeParent.elementType != VALUE_ARGUMENT &&
                 node.isPrecededByOtherAnnotationEntryOnTheSameLine() &&
                 node.isLastAnnotationEntry()
             ) {
                 // Code below is disallowed
                 //   @Foo1 @Foo2 fun foo() {}
                 // But following is allowed:
                 //   @[Foo1 Foo2] fun foo() {}
+                //   fun foo(@Bar1 @Bar2 bar) {}
                 emit(
                     node.findAnnotatedConstruct().startOffset,
                     "Multiple annotations should not be placed on the same line as the annotated construct",
@@ -179,43 +190,39 @@ public class AnnotationRule :
                 }
             }
 
-            node
-                .typeProjectionOrNull()
-                ?.let { typeProjection ->
+            node.typeProjectionOrNull()
+                ?.prevCodeLeaf()
+                ?.let { startOfList ->
                     // Code below is disallowed
                     //   var foo: List<@Foo1 @Foo2 String>
                     // But following is allowed:
                     //   var foo: List<@[Foo1 Foo2] String>
+                    //   fun foo(@Bar1 @Bar2 bar) {}
                     if (node.isFollowedByOtherAnnotationEntryOnTheSameLine() &&
                         node.isFirstAnnotationEntry()
                     ) {
-                        emit(
-                            typeProjection.startOffset,
-                            "Annotations on a type reference should be placed on a separate line",
-                            true,
-                        )
+                        emit(startOfList.startOffset, "Expected newline after '${startOfList.text}'", true)
                         if (autoCorrect) {
-                            typeProjection
-                                .upsertWhitespaceBeforeMe(
+                            startOfList
+                                .upsertWhitespaceAfterMe(
                                     getNewlineWithIndent(node.treeParent).plus(indentConfig.indent),
                                 )
                         }
                     }
                     if (node.isPrecededByOtherAnnotationEntryOnTheSameLine() &&
                         node.isLastAnnotationEntry()
                     ) {
-                        val annotatedConstruct = node.findAnnotatedConstruct()
-                        emit(
-                            annotatedConstruct.startOffset,
-                            "Annotations on a type reference should be placed on a separate line",
-                            true,
-                        )
-                        if (autoCorrect) {
-                            annotatedConstruct
-                                .nextLeaf { it.isCodeLeaf() && it.elementType != ElementType.COMMA }!!
-                                .firstChildLeafOrSelf()
-                                .upsertWhitespaceBeforeMe(getNewlineWithIndent(node.treeParent))
-                        }
+                        node
+                            .findAnnotatedConstruct()
+                            .treeParent
+                            .lastChildLeafOrSelf()
+                            .nextLeaf { it.isCodeLeaf() && it.elementType != ElementType.COMMA }
+                            ?.let { codeLeaf ->
+                                emit(codeLeaf.startOffset, "Expected newline before '${codeLeaf.text}'", true)
+                                if (autoCorrect) {
+                                    codeLeaf.upsertWhitespaceBeforeMe(getNewlineWithIndent(node.treeParent))
+                                }
+                            }
                     }
                 }
         }
@@ -429,16 +436,17 @@ public class AnnotationRule :
     }
 
     private fun getNewlineWithIndent(modifierListRoot: ASTNode): String {
-        val nodeBeforeAnnotations = modifierListRoot.treeParent.treePrev as? PsiWhiteSpace
-        // If there is no whitespace before the annotation, the annotation is the first
-        // text in the file
-        val newLineWithIndent = nodeBeforeAnnotations?.text ?: "\n"
-        return if (newLineWithIndent.contains('\n')) {
-            // Make sure we only insert a single newline
-            newLineWithIndent.substring(newLineWithIndent.lastIndexOf('\n'))
-        } else {
-            newLineWithIndent
-        }
+        val nodeBeforeAnnotations =
+            modifierListRoot
+                .treeParent
+                .treePrev
+        // Make sure we only insert a single newline
+        val indentWithoutNewline =
+            nodeBeforeAnnotations
+                ?.text
+                .orEmpty()
+                .substringAfterLast('\n')
+        return "\n".plus(indentWithoutNewline)
     }
 }
 

diff --git a/...dard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRule.kt b/...dard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRule.kt
@@ -19,6 +19,8 @@ import com.pinterest.ktlint.rule.engine.core.api.IndentConfig.Companion.DEFAULT_
 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.children
+import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CODE_STYLE_PROPERTY
+import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CodeStyleValue.ktlint_official
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfigProperty
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_SIZE_PROPERTY
@@ -31,7 +33,6 @@ import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline
 import com.pinterest.ktlint.rule.engine.core.api.nextCodeLeaf
 import com.pinterest.ktlint.rule.engine.core.api.nextCodeSibling
 import com.pinterest.ktlint.rule.engine.core.api.nextLeaf
-import com.pinterest.ktlint.rule.engine.core.api.nextSibling
 import com.pinterest.ktlint.rule.engine.core.api.prevCodeLeaf
 import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
 import com.pinterest.ktlint.rule.engine.core.api.prevSibling
@@ -66,13 +67,15 @@ public class FunctionSignatureRule :
             ),
     ),
     Rule.Experimental {
+    private var codeStyle = CODE_STYLE_PROPERTY.defaultValue
     private var indentConfig = DEFAULT_INDENT_CONFIG
     private var maxLineLength = MAX_LINE_LENGTH_PROPERTY.defaultValue
     private var functionSignatureWrappingMinimumParameters =
         FORCE_MULTILINE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY.defaultValue
     private var functionBodyExpressionWrapping = FUNCTION_BODY_EXPRESSION_WRAPPING_PROPERTY.defaultValue
 
     override fun beforeFirstNode(editorConfig: EditorConfig) {
+        codeStyle = editorConfig[CODE_STYLE_PROPERTY]
         functionSignatureWrappingMinimumParameters = editorConfig[
             FORCE_MULTILINE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY,
         ]
@@ -159,7 +162,8 @@ public class FunctionSignatureRule :
 
         val forceMultilineSignature =
             node.hasMinimumNumberOfParameters() ||
-                node.containsParameterPrecededByAnnotationOnSeparateLine()
+                node.containsMultilineParameter() ||
+                (codeStyle == ktlint_official && node.containsAnnotatedParameter())
         if (isMaxLineLengthSet()) {
             val singleLineFunctionSignatureLength = calculateFunctionSignatureLengthAsSingleLineSignature(node, emit, autoCorrect)
             // Function signatures not having parameters, should not be reformatted automatically. It would result in function signatures
@@ -215,15 +219,25 @@ public class FunctionSignatureRule :
             tailNodesOfFunctionSignature.sumOf { it.text.length }
     }
 
-    private fun ASTNode.containsParameterPrecededByAnnotationOnSeparateLine(): Boolean =
+    private fun ASTNode.containsMultilineParameter(): Boolean =
         findChildByType(VALUE_PARAMETER_LIST)
             ?.children()
             .orEmpty()
             .filter { it.elementType == VALUE_PARAMETER }
-            .mapNotNull {
-                // If the value parameter contains a modifier then this list is followed by a white space
-                it.findChildByType(MODIFIER_LIST)?.nextSibling()
-            }.any { it.textContains('\n') }
+            .any { it.textContains('\n') }
+
+    private fun ASTNode.containsAnnotatedParameter(): Boolean =
+        findChildByType(VALUE_PARAMETER_LIST)
+            ?.children()
+            .orEmpty()
+            .filter { it.elementType == VALUE_PARAMETER }
+            .any { it.isAnnotated() }
+
+    private fun ASTNode.isAnnotated() =
+        findChildByType(MODIFIER_LIST)
+            ?.children()
+            .orEmpty()
+            .any { it.elementType == ANNOTATION_ENTRY }
 
     private fun calculateFunctionSignatureLengthAsSingleLineSignature(
         node: ASTNode,

diff --git a/.../src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRule.kt b/.../src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRule.kt
@@ -1,5 +1,6 @@
 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.FUNCTION_LITERAL
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUNCTION_TYPE
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.LPAR
@@ -116,17 +117,14 @@ public class ParameterListWrappingRule :
     }
 
     private fun ASTNode.needToWrapParameterList() =
-        if (hasNoParameters() ||
-            isPartOfFunctionLiteralInNonKtlintOfficialCodeStyle() ||
-            isFunctionTypeWrappedInNullableType()
-        ) {
-            false
-        } else {
-            // each parameter should be on a separate line if
-            // - at least one of the parameters is
-            // - maxLineLength exceeded (and separating parameters with \n would actually help)
-            // in addition, "(" and ")" must be on separates line if any of the parameters are (otherwise on the same)
-            textContains('\n') || this.exceedsMaxLineLength()
+        when {
+            hasNoParameters() -> false
+            isPartOfFunctionLiteralInNonKtlintOfficialCodeStyle() -> false
+            isFunctionTypeWrappedInNullableType() -> false
+            textContains('\n') -> true
+            codeStyle == ktlint_official && containsAnnotatedParameter() -> true
+            exceedsMaxLineLength() -> true
+            else -> false
         }
 
     private fun ASTNode.hasNoParameters(): Boolean {
@@ -144,6 +142,19 @@ public class ParameterListWrappingRule :
         return treeParent.elementType == FUNCTION_TYPE && treeParent?.treeParent?.elementType == NULLABLE_TYPE
     }
 
+    private fun ASTNode.containsAnnotatedParameter(): Boolean {
+        require(elementType == VALUE_PARAMETER_LIST)
+        return this.children()
+            .filter { it.elementType == VALUE_PARAMETER }
+            .any { it.isAnnotated() }
+    }
+
+    private fun ASTNode.isAnnotated() =
+        findChildByType(ElementType.MODIFIER_LIST)
+            ?.children()
+            .orEmpty()
+            .any { it.elementType == ElementType.ANNOTATION_ENTRY }
+
     private fun wrapParameterList(
         node: ASTNode,
         emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
@@ -243,7 +254,7 @@ public class ParameterListWrappingRule :
         when (node.elementType) {
             LPAR -> """Unnecessary newline before "(""""
             VALUE_PARAMETER ->
-                "Parameter should be on a separate line (unless all parameters can fit a single line)"
+                "Parameter should start on a newline"
             RPAR -> """Missing newline before ")""""
             else -> throw UnsupportedOperationException()
         }