Skip to content

Commit

Permalink
Extract rule no-single-line-block-comment from comment-wrapping r…
Browse files Browse the repository at this point in the history
…ule (#2000)

Closes #1980
  • Loading branch information
paul-dingemans committed May 9, 2023
1 parent 9256b8e commit 930d263
Show file tree
Hide file tree
Showing 8 changed files with 291 additions and 164 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
* Prevent nullpointer exception (NPE) if or operator at start of line is followed by dot qualified expression `indent` ([#1993](https://github.com/pinterest/ktlint/issues/1993))
* Fix indentation of multiline parameter list in function literal `indent` ([#1976](https://github.com/pinterest/ktlint/issues/1976))
* Restrict indentation of closing quotes to `ktlint_official` code style to keep formatting of other code styles consistent with `0.48.x` and before `indent` ([#1971](https://github.com/pinterest/ktlint/issues/1971))
* Extract rule `no-single-line-block-comment` from `comment-wrapping` rule. The `no-single-line-block-comment` rule is added as experimental rule to the `ktlint_official` code style, but it can be enabled explicitly for the other code styles as well. ([#1980](https://github.com/pinterest/ktlint/issues/1980))
* Clean-up unwanted logging dependencies ([#1998](https://github.com/pinterest/ktlint/issues/1998))
* Fix directory traversal for patterns referring to paths outside of current working directory or any of it child directories ([#2002](https://github.com/pinterest/ktlint/issues/2002))

Expand Down
26 changes: 26 additions & 0 deletions docs/rules/experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,32 @@ This rule can also be suppressed with the IntelliJ IDEA inspection suppression `

Rule id: `property-naming`

## No single line block comments

A single line block comment should be replaced with an EOL comment when possible.

=== "[:material-heart:](#) Ktlint"

```kotlin
/*
* Some comment
*/
val foo = "foo" // Some comment
val foo = { /* no-op */ }

/* ktlint-disable foo-rule-id bar-rule-id */
val foo = "foo"
/* ktlint-enable foo-rule-id bar-rule-id */
```
=== "[:material-heart-off-outline:](#) Disallowed"

```kotlin
/* Some comment */
val foo = "foo" /* Some comment */
```

Rule id: `no-single-line-block-comment`

## Spacing

### No blank lines in list
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/standard.md
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ Rule id: `wrapping`

### Comment wrapping

A block comment should start and end on a line that does not contain any other element. A block comment should not be used as end of line comment.
A block comment should start and end on a line that does not contain any other element.

=== "[:material-heart:](#) Ktlint"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import com.pinterest.ktlint.ruleset.standard.rules.NoLineBreakAfterElseRule
import com.pinterest.ktlint.ruleset.standard.rules.NoLineBreakBeforeAssignmentRule
import com.pinterest.ktlint.ruleset.standard.rules.NoMultipleSpacesRule
import com.pinterest.ktlint.ruleset.standard.rules.NoSemicolonsRule
import com.pinterest.ktlint.ruleset.standard.rules.NoSingleLineBlockCommentRule
import com.pinterest.ktlint.ruleset.standard.rules.NoTrailingSpacesRule
import com.pinterest.ktlint.ruleset.standard.rules.NoUnitReturnRule
import com.pinterest.ktlint.ruleset.standard.rules.NoUnusedImportsRule
Expand Down Expand Up @@ -126,6 +127,7 @@ public class StandardRuleSetProvider :
RuleProvider { NoLineBreakBeforeAssignmentRule() },
RuleProvider { NoMultipleSpacesRule() },
RuleProvider { NoSemicolonsRule() },
RuleProvider { NoSingleLineBlockCommentRule() },
RuleProvider { NoTrailingSpacesRule() },
RuleProvider { NoUnitReturnRule() },
RuleProvider { NoUnusedImportsRule() },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
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.LBRACE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.RBRACE
import com.pinterest.ktlint.rule.engine.core.api.RuleId
Expand All @@ -18,13 +17,10 @@ import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
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.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
* same line is replaced with an EOL comment, if possible.
* Checks external wrapping of block comments. Wrapping inside the comment is not altered.
*/
public class CommentWrappingRule :
StandardRule(
Expand Down Expand Up @@ -54,17 +50,6 @@ public class CommentWrappingRule :
.firstOrNull()
?: node.lastChildLeafOrSelf()

if (!node.textContains('\n') &&
!node.isKtlintSuppressionDirective() &&
beforeBlockComment.prevLeaf().isWhitespaceWithNewlineOrNull() &&
afterBlockComment.nextLeaf().isWhitespaceWithNewlineOrNull()
) {
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()
) {
Expand Down Expand Up @@ -112,7 +97,6 @@ public class CommentWrappingRule :
)
if (autoCorrect) {
node.upsertWhitespaceBeforeMe(" ")
node.replaceWithEndOfLineComment()
}
}
}
Expand All @@ -133,24 +117,7 @@ public class CommentWrappingRule :
}
}

private fun ASTNode.replaceWithEndOfLineComment() {
val content = text.removeSurrounding("/*", "*/").trim()
val eolComment = PsiCommentImpl(EOL_COMMENT, "// $content")
(this as LeafPsiElement).rawInsertBeforeMe(eolComment)
rawRemove()
}

private fun ASTNode?.isWhitespaceWithNewlineOrNull() = this == null || this.isWhiteSpaceWithNewline()

// TODO: Remove when ktlint suppression directive in comments are no longer supported
private fun ASTNode?.isKtlintSuppressionDirective() =
this
?.text
?.removePrefix("/*")
?.removeSuffix("*/")
?.trim()
?.let { it.startsWith("ktlint-enable") || it.startsWith("ktlint-disable") }
?: false
}

public val COMMENT_WRAPPING_RULE_ID: RuleId = CommentWrappingRule().ruleId
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
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.Rule
import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule
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.editorconfig.INDENT_SIZE_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPERTY
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.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

/**
* A block comment following another element on the same line is replaced with an EOL comment, if possible.
*/
public class NoSingleLineBlockCommentRule :
StandardRule(
id = "no-single-line-block-comment",
usesEditorConfigProperties =
setOf(
INDENT_SIZE_PROPERTY,
INDENT_STYLE_PROPERTY,
),
visitorModifiers = setOf(RunAfterRule(COMMENT_WRAPPING_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED)),
),
Rule.Experimental,
Rule.OfficialCodeStyle {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
if (node.elementType == BLOCK_COMMENT) {
val afterBlockComment =
node
.leaves()
.takeWhile { it.isWhiteSpace() && !it.textContains('\n') }
.firstOrNull()
?: node.lastChildLeafOrSelf()

if (!node.textContains('\n') &&
!node.isKtlintSuppressionDirective() &&
afterBlockComment.nextLeaf().isWhitespaceWithNewlineOrNull()
) {
emit(node.startOffset, "Replace the block comment with an EOL comment", true)
if (autoCorrect) {
node.replaceWithEndOfLineComment()
}
}
}
}

private fun ASTNode.replaceWithEndOfLineComment() {
val content = text.removeSurrounding("/*", "*/").trim()
val eolComment = PsiCommentImpl(EOL_COMMENT, "// $content")
(this as LeafPsiElement).rawInsertBeforeMe(eolComment)
rawRemove()
}

private fun ASTNode?.isWhitespaceWithNewlineOrNull() = this == null || this.isWhiteSpaceWithNewline()

// TODO: Remove when ktlint suppression directive in comments are no longer supported
private fun ASTNode?.isKtlintSuppressionDirective() =
this
?.text
?.removePrefix("/*")
?.removeSuffix("*/")
?.trim()
?.let { it.startsWith("ktlint-enable") || it.startsWith("ktlint-disable") }
?: false
}

public val NO_SINGLE_LINE_BLOCK_COMMENT_RULE_ID: RuleId = NoSingleLineBlockCommentRule().ruleId
Original file line number Diff line number Diff line change
@@ -1,34 +1,12 @@
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.ktlint_official
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRule
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 then replace it with an EOL comment`() {
val code =
"""
fun bar() {
/* Some comment */
}
""".trimIndent()
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
fun `Given a multi line block comment that start starts and end on a separate line then do not reformat`() {
val code =
Expand Down Expand Up @@ -107,73 +85,6 @@ class CommentWrappingRuleTest {
}
}

@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
fun `Given a block comment containing a newline which is preceded by another element on the same line then raise lint error but do not autocorrect`() {
val code =
Expand All @@ -187,22 +98,6 @@ class CommentWrappingRuleTest {
.hasLintViolationWithoutAutoCorrect(1, 17, "A block comment after any other element on the same line must be separated by a new line")
}

@Test
fun `Given a block comment that does not contain a newline and which is after some code om the same line is changed to an EOL comment`() {
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 block comment in between code elements on the same line then raise lint error but do not autocorrect`() {
val code =
Expand Down Expand Up @@ -246,28 +141,4 @@ class CommentWrappingRuleTest {
.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()
}

@Test
fun `Given single line block comments to disable or enable ktlint then do not reformat`() {
val code =
"""
/* ktlint-disable foo-rule-id bar-rule-id */
val foo = "foo"
/* ktlint-enable foo-rule-id bar-rule-id */
""".trimIndent()
commentWrappingRuleAssertThat(code)
.withEditorConfigOverride(CODE_STYLE_PROPERTY to ktlint_official)
.hasNoLintViolations()
}
}
Loading

0 comments on commit 930d263

Please sign in to comment.