From 7c6c4fe3f1918bc823c50238f314f089b90d6e31 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Fri, 15 Jan 2021 10:54:06 +0900 Subject: [PATCH 1/2] Fix formatting with comments (`colon-spacing`) --- CHANGELOG.md | 1 + .../standard/SpacingAroundColonRule.kt | 34 +++++++++------ .../standard/SpacingAroundColonRuleTest.kt | 42 +++++++++++++++++-- 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f079a387c2..a391b0e7a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Don't remove the equals sign for a default argument (`no-line-break-before-assignment`) ([#1039](https://github.com/pinterest/ktlint/issues/1039)) - Fix internal error in `no-unused-imports` ([#1040](https://github.com/pinterest/ktlint/issues/1040)) - Fix false positives when declaration has tail comments (`spacing-between-declarations-with-comments`) ([#1053](https://github.com/pinterest/ktlint/issues/1053)) +- Fix formatting with comments (`colon-spacing`) ([#1057](https://github.com/pinterest/ktlint/issues/1057)) ### Changed - Update Gradle shadow plugin to `6.1.0` version diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt index 84f7b7ae88..aa8e841155 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt @@ -6,7 +6,9 @@ import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION_ENTRY import com.pinterest.ktlint.core.ast.isPartOf import com.pinterest.ktlint.core.ast.isPartOfComment import com.pinterest.ktlint.core.ast.isPartOfString +import com.pinterest.ktlint.core.ast.isWhiteSpace import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline +import com.pinterest.ktlint.core.ast.nextLeaf import com.pinterest.ktlint.core.ast.prevLeaf import com.pinterest.ktlint.core.ast.upsertWhitespaceAfterMe import com.pinterest.ktlint.core.ast.upsertWhitespaceBeforeMe @@ -17,6 +19,7 @@ import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtConstructor import org.jetbrains.kotlin.psi.KtTypeConstraint import org.jetbrains.kotlin.psi.KtTypeParameterList +import org.jetbrains.kotlin.psi.psiUtil.prevLeafs class SpacingAroundColonRule : Rule("colon-spacing") { @@ -36,24 +39,31 @@ class SpacingAroundColonRule : Rule("colon-spacing") { node.parent !is KtTypeConstraint && // where T : S node.parent?.parent !is KtTypeParameterList val prevLeaf = node.prevLeaf() - if (prevLeaf != null && - prevLeaf.isWhiteSpaceWithNewline() && - // FIXME: relocate : so that it would be in front of comment - // (see SpacingAroundColonRuleTest.testFormatEOF & ChainWrappingRule) - prevLeaf.prevLeaf()?.isPartOfComment() != true - ) { + if (prevLeaf != null && prevLeaf.isWhiteSpaceWithNewline()) { emit(prevLeaf.startOffset, "Unexpected newline before \":\"", true) - val text = prevLeaf.text if (autoCorrect) { - if (removeSpacingBefore) { - prevLeaf.treeParent.removeChild(prevLeaf) + if (prevLeaf.prevLeaf()?.isPartOfComment() == true) { + val nextLeaf = node.nextLeaf() + val prevNonCodeLeaves = + node.prevLeafs.takeWhile { it.node.isWhiteSpace() || it.node.isPartOfComment() }.toList() + prevNonCodeLeaves.reversed().forEach { + node.treeParent.addChild(it.node, nextLeaf) + } + if (nextLeaf != null && nextLeaf.isWhiteSpace()) { + node.treeParent.removeChild(nextLeaf) + } } else { - (prevLeaf as LeafPsiElement).rawReplaceWithText(" ") + val text = prevLeaf.text + if (removeSpacingBefore) { + prevLeaf.treeParent.removeChild(prevLeaf) + } else { + (prevLeaf as LeafPsiElement).rawReplaceWithText(" ") + } + node.upsertWhitespaceAfterMe(text) } - node.upsertWhitespaceAfterMe(text) } } - if (node.prevSibling is PsiWhiteSpace && removeSpacingBefore) { + if (node.prevSibling is PsiWhiteSpace && removeSpacingBefore && !prevLeaf.isWhiteSpaceWithNewline()) { emit(node.startOffset, "Unexpected spacing before \":\"", true) if (autoCorrect) { node.prevSibling.node.treeParent.removeChild(node.prevSibling.node) diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt index 3961a2a348..921f270604 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt @@ -248,15 +248,49 @@ class SpacingAroundColonRuleTest { class X : Y, Z - class A // comment - : B - class A /* + class A : // comment + B + class A : /* */ - : B + B val xmlFormatter: String = "" """.trimIndent() ) } + + // https://github.com/pinterest/ktlint/issues/1057 + @Test + fun testLintWithEolComment() { + assertThat( + SpacingAroundColonRule().lint( + """ + val x // comment + : Int = 1 + """.trimIndent() + ) + ).isEqualTo( + listOf( + LintError(1, 17, "colon-spacing", "Unexpected newline before \":\""), + ) + ) + } + + @Test + fun testFormatWithEolComment() { + assertThat( + SpacingAroundColonRule().format( + """ + val x // comment + : Int = 1 + """.trimIndent() + ) + ).isEqualTo( + """ + val x: // comment + Int = 1 + """.trimIndent() + ) + } } From e36aebf8ba14c0ad35d7f3c0d8f66e145314692a Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Sat, 16 Jan 2021 09:33:02 +0900 Subject: [PATCH 2/2] Place type on the same line with declaration name --- .../standard/SpacingAroundColonRule.kt | 57 +++++--- .../standard/SpacingAroundColonRuleTest.kt | 132 ++++++++++++++++-- 2 files changed, 162 insertions(+), 27 deletions(-) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt index aa8e841155..2a8510da4d 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRule.kt @@ -3,6 +3,7 @@ package com.pinterest.ktlint.ruleset.standard import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION_ENTRY +import com.pinterest.ktlint.core.ast.ElementType.EQ import com.pinterest.ktlint.core.ast.isPartOf import com.pinterest.ktlint.core.ast.isPartOfComment import com.pinterest.ktlint.core.ast.isPartOfString @@ -15,11 +16,15 @@ import com.pinterest.ktlint.core.ast.upsertWhitespaceBeforeMe 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.psi.KtBlockExpression import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtConstructor +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtProperty import org.jetbrains.kotlin.psi.KtTypeConstraint import org.jetbrains.kotlin.psi.KtTypeParameterList -import org.jetbrains.kotlin.psi.psiUtil.prevLeafs +import org.jetbrains.kotlin.psi.psiUtil.siblings +import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull class SpacingAroundColonRule : Rule("colon-spacing") { @@ -42,24 +47,44 @@ class SpacingAroundColonRule : Rule("colon-spacing") { if (prevLeaf != null && prevLeaf.isWhiteSpaceWithNewline()) { emit(prevLeaf.startOffset, "Unexpected newline before \":\"", true) if (autoCorrect) { - if (prevLeaf.prevLeaf()?.isPartOfComment() == true) { - val nextLeaf = node.nextLeaf() - val prevNonCodeLeaves = - node.prevLeafs.takeWhile { it.node.isWhiteSpace() || it.node.isPartOfComment() }.toList() - prevNonCodeLeaves.reversed().forEach { - node.treeParent.addChild(it.node, nextLeaf) + val parent = node.parent + val prevNonCodeElements = node.siblings(forward = false, withItself = false) + .takeWhile { it.node.isWhiteSpace() || it.node.isPartOfComment() }.toList() + when { + parent is KtProperty || parent is KtNamedFunction -> { + val equalsSignElement = node.siblings(forward = true, withItself = false) + .firstOrNull { it.node.elementType == EQ } + if (equalsSignElement != null) { + equalsSignElement.nextSibling?.takeIf { it.node.isWhiteSpace() }?.delete() + prevNonCodeElements.forEach { parent.addAfter(it, equalsSignElement) } + } + val blockElement = node.siblings(forward = true, withItself = false) + .firstIsInstanceOrNull() + if (blockElement != null) { + prevNonCodeElements + .let { if (it.first().node.isWhiteSpace()) it.drop(1) else it } + .forEach { blockElement.addAfter(it, blockElement.lBrace) } + } + parent.deleteChildRange(prevNonCodeElements.last(), prevNonCodeElements.first()) } - if (nextLeaf != null && nextLeaf.isWhiteSpace()) { - node.treeParent.removeChild(nextLeaf) + prevLeaf.prevLeaf()?.isPartOfComment() == true -> { + val nextLeaf = node.nextLeaf() + prevNonCodeElements.reversed().forEach { + node.treeParent.addChild(it.node, nextLeaf) + } + if (nextLeaf != null && nextLeaf.isWhiteSpace()) { + node.treeParent.removeChild(nextLeaf) + } } - } else { - val text = prevLeaf.text - if (removeSpacingBefore) { - prevLeaf.treeParent.removeChild(prevLeaf) - } else { - (prevLeaf as LeafPsiElement).rawReplaceWithText(" ") + else -> { + val text = prevLeaf.text + if (removeSpacingBefore) { + prevLeaf.treeParent.removeChild(prevLeaf) + } else { + (prevLeaf as LeafPsiElement).rawReplaceWithText(" ") + } + node.upsertWhitespaceAfterMe(text) } - node.upsertWhitespaceAfterMe(text) } } } diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt index 921f270604..f7aed928bc 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/SpacingAroundColonRuleTest.kt @@ -254,42 +254,152 @@ class SpacingAroundColonRuleTest { */ B - val xmlFormatter: - String = "" + val xmlFormatter: String = + "" """.trimIndent() ) } // https://github.com/pinterest/ktlint/issues/1057 @Test - fun testLintWithEolComment() { + fun testLintNewLineBeforeColon() { assertThat( SpacingAroundColonRule().lint( """ - val x // comment - : Int = 1 + fun test() { + val v1 + : Int = 1 + + val v2 // comment + : Int = 1 + + val v3 + // comment + : Int = 1 + + fun f1() + : Int = 1 + + fun f2() // comment + : Int = 1 + + fun f3() + // comment + : Int = 1 + + fun g1() + : Int { + return 1 + } + + fun g2() // comment + : Int { + return 1 + } + + fun g3() + // comment + : Int { + return 1 + } + } """.trimIndent() ) ).isEqualTo( listOf( - LintError(1, 17, "colon-spacing", "Unexpected newline before \":\""), + LintError(2, 11, "colon-spacing", "Unexpected newline before \":\""), + LintError(5, 22, "colon-spacing", "Unexpected newline before \":\""), + LintError(9, 19, "colon-spacing", "Unexpected newline before \":\""), + LintError(12, 13, "colon-spacing", "Unexpected newline before \":\""), + LintError(15, 24, "colon-spacing", "Unexpected newline before \":\""), + LintError(19, 19, "colon-spacing", "Unexpected newline before \":\""), + LintError(22, 13, "colon-spacing", "Unexpected newline before \":\""), + LintError(27, 24, "colon-spacing", "Unexpected newline before \":\""), + LintError(33, 19, "colon-spacing", "Unexpected newline before \":\""), ) ) } @Test - fun testFormatWithEolComment() { + fun testFormatNewLineBeforeColon() { assertThat( SpacingAroundColonRule().format( """ - val x // comment - : Int = 1 + fun test() { + val v1 + : Int = 1 + + val v2 // comment + : Int = 1 + + val v3 + // comment + : Int = 1 + + fun f1() + : Int = 1 + + fun f2() // comment + : Int = 1 + + fun f3() + // comment + : Int = 1 + + fun g1() + : Int { + return 1 + } + + fun g2() // comment + : Int { + return 1 + } + + fun g3() + // comment + : Int { + return 1 + } + } """.trimIndent() ) ).isEqualTo( """ - val x: // comment - Int = 1 + fun test() { + val v1: Int = + 1 + + val v2: Int = // comment + 1 + + val v3: Int = + // comment + 1 + + fun f1(): Int = + 1 + + fun f2(): Int = // comment + 1 + + fun f3(): Int = + // comment + 1 + + fun g1(): Int { + return 1 + } + + fun g2(): Int { // comment + return 1 + } + + fun g3(): Int { + // comment + return 1 + } + } """.trimIndent() ) }