From 395bfd306c02525e8207c1dacd6e7be69967d6fa Mon Sep 17 00:00:00 2001 From: paul-dingemans <paul-dingemans@users.noreply.github.com> Date: Fri, 24 Nov 2023 20:19:25 +0100 Subject: [PATCH 1/2] Move curly brace before all consecutive comments preceding that curly brace Closes #2355 --- .../standard/rules/SpacingAroundCurlyRule.kt | 68 ++++++++++--- .../rules/SpacingAroundCurlyRuleTest.kt | 98 +++++++++++++++++++ 2 files changed, 150 insertions(+), 16 deletions(-) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule.kt index 3d01f1c888..c666a8c563 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule.kt @@ -5,7 +5,6 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS_BODY import com.pinterest.ktlint.rule.engine.core.api.ElementType.COLONCOLON import com.pinterest.ktlint.rule.engine.core.api.ElementType.COMMA import com.pinterest.ktlint.rule.engine.core.api.ElementType.DOT -import com.pinterest.ktlint.rule.engine.core.api.ElementType.EOL_COMMENT import com.pinterest.ktlint.rule.engine.core.api.ElementType.EXCLEXCL import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN import com.pinterest.ktlint.rule.engine.core.api.ElementType.LAMBDA_EXPRESSION @@ -17,26 +16,56 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.RBRACKET import com.pinterest.ktlint.rule.engine.core.api.ElementType.RPAR import com.pinterest.ktlint.rule.engine.core.api.ElementType.SAFE_ACCESS import com.pinterest.ktlint.rule.engine.core.api.ElementType.SEMICOLON +import com.pinterest.ktlint.rule.engine.core.api.IndentConfig import com.pinterest.ktlint.rule.engine.core.api.RuleId import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.STABLE +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.isLeaf +import com.pinterest.ktlint.rule.engine.core.api.isPartOfComment import com.pinterest.ktlint.rule.engine.core.api.isPartOfString +import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace +import com.pinterest.ktlint.rule.engine.core.api.leavesIncludingSelf import com.pinterest.ktlint.rule.engine.core.api.nextLeaf 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.ruleset.standard.StandardRule import org.jetbrains.kotlin.com.intellij.lang.ASTNode -import org.jetbrains.kotlin.com.intellij.psi.PsiComment 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.TreeElement import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtLambdaExpression @SinceKtlint("0.1", STABLE) -public class SpacingAroundCurlyRule : StandardRule("curly-spacing") { +public class SpacingAroundCurlyRule : + StandardRule( + id = "curly-spacing", + usesEditorConfigProperties = + setOf( + CODE_STYLE_PROPERTY, + INDENT_SIZE_PROPERTY, + INDENT_STYLE_PROPERTY, + ), + ) { + 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], + ) + if (indentConfig.disabled) { + stopTraversalOfAST() + } + } + override fun beforeVisitChildNodes( node: ASTNode, autoCorrect: Boolean, @@ -76,19 +105,21 @@ public class SpacingAroundCurlyRule : StandardRule("curly-spacing") { ) { emit(node.startOffset, "Unexpected newline before \"${node.text}\"", true) if (autoCorrect) { - val eolCommentExists = + if (prevLeaf.isPrecededByEolComment()) { + // All consecutive whitespaces and comments preceding the curly have to be moved after the curly brace prevLeaf - .prevLeaf() - ?.let { it is PsiComment && it.elementType == EOL_COMMENT } - ?: false - if (eolCommentExists) { - val commentLeaf = prevLeaf.prevLeaf()!! - if (commentLeaf.prevLeaf() is PsiWhiteSpace) { - (commentLeaf.prevLeaf() as LeafPsiElement).rawRemove() - } - (node.treeParent.treeParent as TreeElement).removeChild(commentLeaf) - (node.treeParent as TreeElement).addChild(commentLeaf, node.treeNext) - node.upsertWhitespaceAfterMe(" ") + .leavesIncludingSelf(forward = false) + .takeWhile { it.isWhiteSpace() || it.isPartOfComment() } + .toList() + .reversed() + .takeIf { it.isNotEmpty() } + ?.let { leavesToMoveAfterCurly -> + node.treeParent.addChildren( + leavesToMoveAfterCurly.first(), + leavesToMoveAfterCurly.last(), + node.treeNext, + ) + } } (prevLeaf as LeafPsiElement).rawReplaceWithText(" ") } @@ -131,6 +162,11 @@ public class SpacingAroundCurlyRule : StandardRule("curly-spacing") { } } + private fun ASTNode.isPrecededByEolComment() = + prevLeaf() + ?.isPartOfComment() + ?: false + private fun shouldNotToBeSeparatedBySpace(leaf: ASTNode?): Boolean { val nextElementType = leaf?.elementType return ( diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRuleTest.kt index 348f2c5154..b47f7118d8 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRuleTest.kt @@ -464,4 +464,102 @@ class SpacingAroundCurlyRuleTest { LintViolation(5, 1, "Unexpected newline before \"{\""), ).isFormattedAs(formattedCode) } + + @Test + fun `Given a block comment before opening curly brace of function`() { + val code = + """ + class Foo1()/* a comment (originally) not preceded by a space */ + { + } + class Foo2() /* a comment (originally) preceded by a space */ + { + } + """.trimIndent() + val formattedCode = + """ + class Foo1() { /* a comment (originally) not preceded by a space */ + } + class Foo2() { /* a comment (originally) preceded by a space */ + } + """.trimIndent() + spacingAroundCurlyRuleAssertThat(code) + .hasLintViolations( + LintViolation(2, 1, "Unexpected newline before \"{\""), + LintViolation(5, 1, "Unexpected newline before \"{\""), + ).isFormattedAs(formattedCode) + } + + @Nested + inner class `Issue 2355 - Given a function signature with multiple comments at an unexpected location` { + @Test + fun `Multiple EOL comments at an unexpected location`() { + val code = + """ + fun foo(): Float + // comment1 + // comment2 + { + return 0.0f + } + """.trimIndent() + val formattedCode = + """ + fun foo(): Float { + // comment1 + // comment2 + return 0.0f + } + """.trimIndent() + spacingAroundCurlyRuleAssertThat(code) + .hasLintViolation(4, 1, "Unexpected newline before \"{\"") + .isFormattedAs(formattedCode) + } + + @Test + fun `Multiple block comments at an unexpected location`() { + val code = + """ + fun foo(): Float + /* comment1 */ + /* comment2 */ + { + return 0.0f + } + """.trimIndent() + val formattedCode = + """ + fun foo(): Float { + /* comment1 */ + /* comment2 */ + return 0.0f + } + """.trimIndent() + spacingAroundCurlyRuleAssertThat(code) + .hasLintViolation(4, 1, "Unexpected newline before \"{\"") + .isFormattedAs(formattedCode) + } + + @Test + fun `Multiple block and EOL comments at an unexpected location`() { + val code = + """ + fun foo(): Float // comment1 + /* comment2 */ + { + return 0.0f + } + """.trimIndent() + val formattedCode = + """ + fun foo(): Float { // comment1 + /* comment2 */ + return 0.0f + } + """.trimIndent() + spacingAroundCurlyRuleAssertThat(code) + .hasLintViolation(3, 1, "Unexpected newline before \"{\"") + .isFormattedAs(formattedCode) + } + } } From d9d1e01a42c5a11daceeacee39bd676bb670aa50 Mon Sep 17 00:00:00 2001 From: paul-dingemans <paul-dingemans@users.noreply.github.com> Date: Sat, 25 Nov 2023 12:24:47 +0100 Subject: [PATCH 2/2] Fix public api --- ktlint-ruleset-standard/api/ktlint-ruleset-standard.api | 1 + 1 file changed, 1 insertion(+) diff --git a/ktlint-ruleset-standard/api/ktlint-ruleset-standard.api b/ktlint-ruleset-standard/api/ktlint-ruleset-standard.api index 9e028878fc..eef3a3e4ca 100644 --- a/ktlint-ruleset-standard/api/ktlint-ruleset-standard.api +++ b/ktlint-ruleset-standard/api/ktlint-ruleset-standard.api @@ -683,6 +683,7 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundComm public final class com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule : com/pinterest/ktlint/ruleset/standard/StandardRule { public fun <init> ()V + public fun beforeFirstNode (Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfig;)V public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZLkotlin/jvm/functions/Function3;)V }