diff --git a/CHANGELOG.md b/CHANGELOG.md index e86c1d9467..857ee04385 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -330,6 +330,8 @@ if (node.isRoot()) { * Update Kotlin development version to `1.8.20` and Kotlin version to `1.8.20`. * Revert to matrix build to speed up build, especially for the Windows related build ([#1787](https://github.com/pinterest/ktlint/pull/1787)) * For the new code style `ktlint_official`, do not allow wildcard imports `java.util` and `kotlinx.android.synthetic` by default. Important: `.editorconfig` property `ij_kotlin_packages_to_use_import_on_demand` needs to be set to value `unset` in order to enforce IntelliJ IDEA default formatter to not generate wildcard imports `no-wildcard-imports` ([#1797](https://github.com/pinterest/ktlint/issues/1797)) +* Convert a single line block comment to an EOL comment if not preceded or followed by another code element on the same line `comment-wrapping` ([#1941](https://github.com/pinterest/ktlint/issues/1941)) +* Ignore a block comment inside a single line block `comment-wrapping` ([#1942](https://github.com/pinterest/ktlint/issues/1942)) ## [0.48.2] - 2023-01-21 diff --git a/docs/rules/standard.md b/docs/rules/standard.md index d0653e8e63..12ff505919 100644 --- a/docs/rules/standard.md +++ b/docs/rules/standard.md @@ -875,6 +875,7 @@ A block comment should start and end on a line that does not contain any other e /* Some comment 1 */ val foo1 = "foo1" val foo2 = "foo" // Some comment + val foo3 = { /* no-op */ } ``` === "[:material-heart-off-outline:](#) Disallowed" diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRule.kt index 49da940c45..15a8257645 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRule.kt @@ -2,12 +2,17 @@ package com.pinterest.ktlint.ruleset.standard.rules import com.pinterest.ktlint.rule.engine.core.api.ElementType.BLOCK_COMMENT import com.pinterest.ktlint.rule.engine.core.api.ElementType.EOL_COMMENT -import com.pinterest.ktlint.rule.engine.core.api.ElementType.WHITE_SPACE +import com.pinterest.ktlint.rule.engine.core.api.ElementType.LBRACE +import com.pinterest.ktlint.rule.engine.core.api.ElementType.RBRACE import com.pinterest.ktlint.rule.engine.core.api.RuleId 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.hasNewLineInClosedRange import com.pinterest.ktlint.rule.engine.core.api.indent +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.nextLeaf import com.pinterest.ktlint.rule.engine.core.api.prevLeaf import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe @@ -15,6 +20,7 @@ import com.pinterest.ktlint.ruleset.standard.StandardRule import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiCommentImpl +import org.jetbrains.kotlin.psi.psiUtil.leaves /** * Checks external wrapping of block comments. Wrapping inside the comment is not altered. A block comment following another element on the @@ -35,19 +41,33 @@ public class CommentWrappingRule : emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, ) { if (node.elementType == BLOCK_COMMENT) { - val nonIndentLeafOnSameLinePrecedingBlockComment = + val beforeBlockComment = node - .prevLeaf() - ?.takeIf { isNonIndentLeafOnSameLine(it) } - val nonIndentLeafOnSameLineFollowingBlockComment = + .leaves(false) + .takeWhile { it.isWhiteSpace() && !it.textContains('\n') } + .firstOrNull() + ?: node.firstChildLeafOrSelf() + val afterBlockComment = node - .nextLeaf() - ?.takeIf { isNonIndentLeafOnSameLine(it) } + .leaves() + .takeWhile { it.isWhiteSpace() && !it.textContains('\n') } + .firstOrNull() + ?: node.lastChildLeafOrSelf() - if (nonIndentLeafOnSameLinePrecedingBlockComment != null && - nonIndentLeafOnSameLineFollowingBlockComment != null + if (!node.textContains('\n') && + beforeBlockComment.prevLeaf().isWhitespaceWithNewlineOrNull() && + afterBlockComment.nextLeaf().isWhitespaceWithNewlineOrNull() ) { - if (hasNewLineInClosedRange(nonIndentLeafOnSameLinePrecedingBlockComment, nonIndentLeafOnSameLineFollowingBlockComment)) { + emit(node.startOffset, "A single line block comment must be replaced with an EOL comment", true) + if (autoCorrect) { + node.replaceWithEndOfLineComment() + } + } + + if (!beforeBlockComment.prevLeaf().isWhitespaceWithNewlineOrNull() && + !afterBlockComment.nextLeaf().isWhitespaceWithNewlineOrNull() + ) { + if (hasNewLineInClosedRange(beforeBlockComment, afterBlockComment)) { // Do not try to fix constructs like below: // val foo = "foo" /* // some comment @@ -58,48 +78,57 @@ public class CommentWrappingRule : "disallowed", false, ) + } else if (beforeBlockComment.prevLeaf()?.elementType == LBRACE && + afterBlockComment.nextLeaf()?.elementType == RBRACE + ) { + // Allow single line blocks containing a block comment + // val foo = { /* no-op */ } + return } else { // Do not try to fix constructs like below: // val foo /* some comment */ = "foo" - emit( - node.startOffset, - "A block comment in between other elements on the same line is disallowed", - false, - ) + emit(node.startOffset, "A block comment in between other elements on the same line is disallowed", false) } return } - nonIndentLeafOnSameLinePrecedingBlockComment - ?.precedesBlockCommentOnSameLine(node, emit, autoCorrect) - - nonIndentLeafOnSameLineFollowingBlockComment - ?.followsBlockCommentOnSameLine(node, emit, autoCorrect) - } - } + beforeBlockComment + .prevLeaf() + .takeIf { !it.isWhitespaceWithNewlineOrNull() } + ?.let { + if (node.textContains('\n')) { + // It can not be autocorrected as it might depend on the situation and code style what is preferred. + emit( + node.startOffset, + "A block comment after any other element on the same line must be separated by a new line", + false, + ) + } else { + emit( + node.startOffset, + "A single line block comment after a code element on the same line must be replaced with an EOL comment", + true, + ) + if (autoCorrect) { + node.upsertWhitespaceBeforeMe(" ") + node.replaceWithEndOfLineComment() + } + } + } - private fun ASTNode.precedesBlockCommentOnSameLine( - blockCommentNode: ASTNode, - emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, - autoCorrect: Boolean, - ) { - val leafAfterBlockComment = blockCommentNode.nextLeaf() - if (!blockCommentNode.textContains('\n') && leafAfterBlockComment.isLastElementOnLine()) { - emit( - startOffset, - "A single line block comment after a code element on the same line must be replaced with an EOL comment", - true, - ) - if (autoCorrect) { - blockCommentNode.replaceWithEndOfLineComment() - } - } else { - // It can not be autocorrected as it might depend on the situation and code style what is preferred. - emit( - blockCommentNode.startOffset, - "A block comment after any other element on the same line must be separated by a new line", - false, - ) + afterBlockComment + .nextLeaf() + .takeIf { !it.isWhitespaceWithNewlineOrNull() } + ?.let { nextLeaf -> + emit( + nextLeaf.startOffset, + "A block comment may not be followed by any other element on that same line", + true + ) + if (autoCorrect) { + nextLeaf.upsertWhitespaceBeforeMe(node.indent()) + } + } } } @@ -110,20 +139,8 @@ public class CommentWrappingRule : rawRemove() } - private fun ASTNode.followsBlockCommentOnSameLine( - blockCommentNode: ASTNode, - emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, - autoCorrect: Boolean, - ) { - emit(startOffset, "A block comment may not be followed by any other element on that same line", true) - if (autoCorrect) { - this.upsertWhitespaceBeforeMe(blockCommentNode.indent()) - } - } - - private fun isNonIndentLeafOnSameLine(it: ASTNode) = it.elementType != WHITE_SPACE || !it.textContains('\n') - - private fun ASTNode?.isLastElementOnLine() = this == null || (elementType == WHITE_SPACE && textContains('\n')) + private fun ASTNode?.isWhitespaceWithNewlineOrNull() = + this == null || this.isWhiteSpaceWithNewline() } public val COMMENT_WRAPPING_RULE_ID: RuleId = CommentWrappingRule().ruleId diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRuleTest.kt index c89e598371..8600160b8a 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRuleTest.kt @@ -1,19 +1,34 @@ package com.pinterest.ktlint.ruleset.standard.rules +import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CODE_STYLE_PROPERTY +import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CodeStyleValue +import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CodeStyleValue.ktlint_official import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRule import com.pinterest.ktlint.test.LintViolation +import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test class CommentWrappingRuleTest { private val commentWrappingRuleAssertThat = assertThatRule { CommentWrappingRule() } @Test - fun `Given a single line block comment that start starts and end on a separate line then do not reformat`() { + fun `Given a single line block comment then replace it with an EOL comment`() { val code = """ - /* Some comment */ + fun bar() { + /* Some comment */ + } """.trimIndent() - commentWrappingRuleAssertThat(code).hasNoLintViolations() + val formattedCode = + """ + fun bar() { + // Some comment + } + """.trimIndent() + commentWrappingRuleAssertThat(code) + .withEditorConfigOverride(CODE_STYLE_PROPERTY to ktlint_official) + .hasLintViolation(2, 5, "A single line block comment must be replaced with an EOL comment") + .isFormattedAs(formattedCode) } @Test @@ -27,33 +42,138 @@ class CommentWrappingRuleTest { commentWrappingRuleAssertThat(code).hasNoLintViolations() } - @Test - fun `Given a block comment followed by a code element on the same line as the block comment ended then split the elements with a new line`() { - val code = - """ - /* Some comment 1 */ val foo1 = "foo1" - /* Some comment 2 */val foo2 = "foo2" - /* Some comment 3 */ fun foo3() = "foo3" - /* Some comment 4 */fun foo4() = "foo4" - """.trimIndent() - val formattedCode = - """ - /* Some comment 1 */ - val foo1 = "foo1" - /* Some comment 2 */ - val foo2 = "foo2" - /* Some comment 3 */ - fun foo3() = "foo3" - /* Some comment 4 */ - fun foo4() = "foo4" - """.trimIndent() - commentWrappingRuleAssertThat(code) - .hasLintViolations( - LintViolation(1, 21, "A block comment may not be followed by any other element on that same line"), - LintViolation(2, 21, "A block comment may not be followed by any other element on that same line"), - LintViolation(3, 21, "A block comment may not be followed by any other element on that same line"), - LintViolation(4, 21, "A block comment may not be followed by any other element on that same line"), - ).isFormattedAs(formattedCode) + @Nested + inner class `Given a block comment followed by a code element on the same line` { + @Test + fun `Given a comment followed by a property and separated with space`() { + val code = + """ + /* Some comment */ val foo = "foo" + """.trimIndent() + val formattedCode = + """ + /* Some comment */ + val foo = "foo" + """.trimIndent() + commentWrappingRuleAssertThat(code) + .hasLintViolation(1, 20, "A block comment may not be followed by any other element on that same line") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given a comment followed by a property but not separated with space`() { + val code = + """ + /* Some comment */val foo = "foo" + """.trimIndent() + val formattedCode = + """ + /* Some comment */ + val foo = "foo" + """.trimIndent() + commentWrappingRuleAssertThat(code) + .hasLintViolation(1, 19, "A block comment may not be followed by any other element on that same line") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given a comment followed by a function and separated with space`() { + val code = + """ + /* Some comment */ fun foo() = "foo" + """.trimIndent() + val formattedCode = + """ + /* Some comment */ + fun foo() = "foo" + """.trimIndent() + commentWrappingRuleAssertThat(code) + .hasLintViolation(1, 20, "A block comment may not be followed by any other element on that same line") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given a comment followed by a function but not separated with space`() { + val code = + """ + /* Some comment */fun foo() = "foo" + """.trimIndent() + val formattedCode = + """ + /* Some comment */ + fun foo() = "foo" + """.trimIndent() + commentWrappingRuleAssertThat(code) + .hasLintViolation(1, 19, "A block comment may not be followed by any other element on that same line") + .isFormattedAs(formattedCode) + } + } + + @Nested + inner class `Given some code code followed by a block comment on the same line` { + @Test + fun `Given a comment followed by a property and separated with space`() { + val code = + """ + val foo = "foo" /* Some comment */ + """.trimIndent() + val formattedCode = + """ + val foo = "foo" // Some comment + """.trimIndent() + @Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length") + commentWrappingRuleAssertThat(code) + .hasLintViolation(1, 17, "A single line block comment after a code element on the same line must be replaced with an EOL comment") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given a comment followed by a property but not separated with space`() { + val code = + """ + val foo = "foo"/* Some comment */ + """.trimIndent() + val formattedCode = + """ + val foo = "foo" // Some comment + """.trimIndent() + @Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length") + commentWrappingRuleAssertThat(code) + .hasLintViolation(1, 16, "A single line block comment after a code element on the same line must be replaced with an EOL comment") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given a comment followed by a function and separated with space`() { + val code = + """ + fun foo() = "foo" /* Some comment */ + """.trimIndent() + val formattedCode = + """ + fun foo() = "foo" // Some comment + """.trimIndent() + @Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length") + commentWrappingRuleAssertThat(code) + .hasLintViolation(1, 19, "A single line block comment after a code element on the same line must be replaced with an EOL comment") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given a comment followed by a function but not separated with space`() { + val code = + """ + fun foo() = "foo"/* Some comment */ + """.trimIndent() + val formattedCode = + """ + fun foo() = "foo" // Some comment + """.trimIndent() + @Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length") + commentWrappingRuleAssertThat(code) + .hasLintViolation(1, 18, "A single line block comment after a code element on the same line must be replaced with an EOL comment") + .isFormattedAs(formattedCode) + } } @Test @@ -64,12 +184,9 @@ class CommentWrappingRuleTest { * with a newline */ """.trimIndent() + @Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length") commentWrappingRuleAssertThat(code) - .hasLintViolationWithoutAutoCorrect( - 1, - 17, - "A block comment after any other element on the same line must be separated by a new line", - ) + .hasLintViolationWithoutAutoCorrect(1, 17, "A block comment after any other element on the same line must be separated by a new line") } @Test @@ -84,7 +201,7 @@ class CommentWrappingRuleTest { """.trimIndent() @Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length") commentWrappingRuleAssertThat(code) - .hasLintViolation(1, 16, "A single line block comment after a code element on the same line must be replaced with an EOL comment") + .hasLintViolation(1, 17, "A single line block comment after a code element on the same line must be replaced with an EOL comment") .isFormattedAs(formattedCode) } @@ -94,12 +211,9 @@ class CommentWrappingRuleTest { """ val foo /* some comment */ = "foo" """.trimIndent() + @Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length") commentWrappingRuleAssertThat(code) - .hasLintViolationWithoutAutoCorrect( - 1, - 9, - "A block comment in between other elements on the same line is disallowed", - ) + .hasLintViolationWithoutAutoCorrect(1, 9, "A block comment in between other elements on the same line is disallowed") } @Test @@ -110,12 +224,9 @@ class CommentWrappingRuleTest { some comment */ = "foo" """.trimIndent() + @Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length") commentWrappingRuleAssertThat(code) - .hasLintViolationWithoutAutoCorrect( - 1, - 9, - "A block comment starting on same line as another element and ending on another line before another element is disallowed", - ) + .hasLintViolationWithoutAutoCorrect(1, 9, "A block comment starting on same line as another element and ending on another line before another element is disallowed") } @Test @@ -134,7 +245,18 @@ class CommentWrappingRuleTest { } """.trimIndent() commentWrappingRuleAssertThat(code) - .hasLintViolation(2, 23, "A block comment may not be followed by any other element on that same line") + .hasLintViolation(2, 24, "A block comment may not be followed by any other element on that same line") .isFormattedAs(formattedCode) } + + @Test + fun `Given a single line block containing a block comment then do not reformat`() { + val code = + """ + val foo = { /* no-op */ } + """.trimIndent() + commentWrappingRuleAssertThat(code) + .withEditorConfigOverride(CODE_STYLE_PROPERTY to ktlint_official) + .hasNoLintViolations() + } }