Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ignore EOL comment that causes max_line_length to be exceeded, except in max-line-length rule #2516

Merged
merged 1 commit into from
Jan 28, 2024
Merged

Ignore EOL comment that causes max_line_length to be exceeded, except in max-line-length rule #2516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ktlint-rule-engine-core/api/ktlint-rule-engine-core.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ public final class com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtensionKt
public static final fun beforeCodeSibling (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;)Z
public static final fun betweenCodeSiblings (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;)Z
public static final fun children (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;)Lkotlin/sequences/Sequence;
public static final fun dropTrailingEolComment (Lkotlin/sequences/Sequence;)Lkotlin/sequences/Sequence;
public static final fun findCompositeParentElementOfType (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;)Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;
public static final fun firstChildLeafOrSelf (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;)Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;
public static final fun getColumn (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;)I
Expand All @@ -28,6 +29,8 @@ public final class com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtensionKt
public static final fun leavesIncludingSelf (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Z)Lkotlin/sequences/Sequence;
public static synthetic fun leavesIncludingSelf$default (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZILjava/lang/Object;)Lkotlin/sequences/Sequence;
public static final fun leavesOnLine (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;)Lkotlin/sequences/Sequence;
public static final fun lineLength (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Z)I
public static synthetic fun lineLength$default (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZILjava/lang/Object;)I
public static final fun lineLengthWithoutNewlinePrefix (Lkotlin/sequences/Sequence;)I
public static final fun lineLengthWithoutNewlinePrefix (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;)I
public static final fun logStructure (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;)Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;
Expand Down
33 changes: 32 additions & 1 deletion ...engine-core/src/main/kotlin/com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtension.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.pinterest.ktlint.rule.engine.core.api

import com.pinterest.ktlint.rule.engine.core.api.ElementType.EOL_COMMENT
import com.pinterest.ktlint.rule.engine.core.api.ElementType.REGULAR_STRING_PART
import com.pinterest.ktlint.rule.engine.core.api.ElementType.STRING_TEMPLATE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.VAL_KEYWORD
Expand Down Expand Up @@ -444,6 +445,16 @@ public fun ASTNode.leavesOnLine(): Sequence<ASTNode> {
.takeWhile { lastLeafOnLineOrNull == null || it.prevLeaf() != lastLeafOnLineOrNull }
}

/**
* Take all nodes preceding the whitespace before the EOL comment
*/
public fun Sequence<ASTNode>.dropTrailingEolComment(): Sequence<ASTNode> =
takeWhile {
!(it.isWhiteSpace() && it.nextLeaf()?.elementType == EOL_COMMENT) &&
// But if EOL-comment not preceded by whitespace than take all node before the EOL comment
it.elementType != EOL_COMMENT
}

internal fun ASTNode.getFirstLeafOnLineOrSelf() =
prevLeaf { it.textContains('\n') || it.prevLeaf() == null }
?: this
Expand All @@ -452,10 +463,30 @@ internal fun ASTNode.getLastLeafOnLineOrNull() = nextLeaf { it.textContains('\n'

/**
* Get the total length of all leaves on the same line as the given node including the whitespace indentation but excluding all leading
* newline characters in the whitespace indentation.
* newline characters in the whitespace indentation. Note that EOL-comments are included in the line length calculation. This can lead to
* rules modifying the code before the EOL-comment in case the EOL-comment causes the max_line_length to be exceeded. It should be
* preferred to prevent this by excluding the EOL-comment from the calculation (use [lineLength]), and let max-line-length rule report the
* violation.
*/
@Deprecated(
message =
"Marked for removal in Ktlint 2.x. Rules should not modify code in case the EOL comment causes the max_line_length to be exceeded.",
replaceWith = ReplaceWith("lineLength(excludeEolComment = false)"),
)
public fun ASTNode.lineLengthWithoutNewlinePrefix(): Int = leavesOnLine().lineLengthWithoutNewlinePrefix()

/**
* Get the total length of all leaves on the same line as the given node including the whitespace indentation but excluding all leading
* newline characters in the whitespace indentation. Use [excludeEolComment] to exclude the EOL-comment (and preceding whitespace) from this
* calculation. Note that rules should not modify code in case the EOL comment causes the max_line_length to be exceeded. Instead, let the
* max-line-length rule report this violation so that the developer can choose whether the comment can be shortened or that it can be placed
* on a separate line.
*/
public fun ASTNode.lineLength(excludeEolComment: Boolean = false): Int =
leavesOnLine()
.applyIf(excludeEolComment) { dropTrailingEolComment() }
.lineLengthWithoutNewlinePrefix()

/**
* Get the total length of all leaves in the sequence including the whitespace indentation but excluding all leading newline characters in
* the whitespace indentation. The first leaf node in the sequence must be a white space starting with at least one newline.
Expand Down
223 changes: 173 additions & 50 deletions ...ne-core/src/test/kotlin/com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtensionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,66 +665,189 @@ class ASTNodeExtensionTest {
)
}

@Test
fun `Given some lines containing identifiers at different indentation levels then get line length exclusive the leading newline characters`() {
val code =
"""
class Foo1 {
val foo2 = "foo2"
@Suppress("DEPRECATION")
@Nested
inner class LineLengthWithoutNewlinePrefix {
@Test
fun `Given some lines containing identifiers at different indentation levels then get line length exclusive the leading newline characters`() {
val code =
"""
class Foo1 {
val foo2 = "foo2"

fun foo3() {
val foo4 = "foo4"
fun foo3() {
val foo4 = "foo4"
}
}
}
""".trimIndent()
""".trimIndent()

val actual =
transformCodeToAST(code)
.firstChildLeafOrSelf()
.leaves()
.filter { it.elementType == IDENTIFIER }
.map { identifier -> identifier.lineLengthWithoutNewlinePrefix() }
.toList()
val actual =
transformCodeToAST(code)
.firstChildLeafOrSelf()
.leaves()
.filter { it.elementType == IDENTIFIER }
.map { identifier -> identifier.lineLengthWithoutNewlinePrefix() }
.toList()

assertThat(actual).contains(
"class Foo1 {".length,
" val foo2 = \"foo2\"".length,
" fun foo3() {".length,
" val foo4 = \"foo4\"".length,
)
}

assertThat(actual).contains(
"class Foo1 {".length,
" val foo2 = \"foo2\"".length,
" fun foo3() {".length,
" val foo4 = \"foo4\"".length,
)
@Test
fun `Given some lines containing identifiers and EOL comment then get line length exclusive the leading newline characters and exclusive EOL comment`() {
val code =
"""
class Foo1 {
val foo2 = "foo2" // some comment

fun foo3() {
val foo4 = "foo4" // some comment
}
}
""".trimIndent()

val actual =
transformCodeToAST(code)
.firstChildLeafOrSelf()
.leaves()
.filter { it.elementType == IDENTIFIER }
.map { identifier -> identifier.lineLengthWithoutNewlinePrefix() }
.toList()

assertThat(actual).contains(
"class Foo1 {".length,
" val foo2 = \"foo2\" // some comment".length,
" fun foo3() {".length,
" val foo4 = \"foo4\" // some comment".length,
)
}

@Test
fun `Given some lines containing identifiers at different indentation levels then get line length exclusive the leading newline characters until and including the identifier`() {
val code =
"""
class Foo1 {
val foo2 = "foo2"

fun foo3() {
val foo4 = "foo4"
}
}
""".trimIndent()

val actual =
transformCodeToAST(code)
.firstChildLeafOrSelf()
.leaves()
.filter { it.elementType == IDENTIFIER }
.map { identifier ->
identifier
.leavesOnLine()
.takeWhile { it.prevLeaf() != identifier }
.lineLengthWithoutNewlinePrefix()
}.toList()

assertThat(actual).contains(
"class Foo1".length,
" val foo2".length,
" fun foo3".length,
" val foo4".length,
)
}
}

@Test
fun `Given some lines containing identifiers at different indentation levels then get line length exclusive the leading newline characters until and including the identifier`() {
val code =
"""
class Foo1 {
val foo2 = "foo2"
@Nested
inner class LineLength {
@Test
fun `Given some lines containing identifiers at different indentation levels then get line length exclusive the leading newline characters`() {
val code =
"""
class Foo1 {
val foo2 = "foo2"

fun foo3() {
val foo4 = "foo4"
fun foo3() {
val foo4 = "foo4"
}
}
}
""".trimIndent()
""".trimIndent()

val actual =
transformCodeToAST(code)
.firstChildLeafOrSelf()
.leaves()
.filter { it.elementType == IDENTIFIER }
.map { identifier ->
identifier
.leavesOnLine()
.takeWhile { it.prevLeaf() != identifier }
.lineLengthWithoutNewlinePrefix()
}.toList()
val actual =
transformCodeToAST(code)
.firstChildLeafOrSelf()
.leaves()
.filter { it.elementType == IDENTIFIER }
.map { identifier -> identifier.lineLength() }
.toList()

assertThat(actual).contains(
"class Foo1 {".length,
" val foo2 = \"foo2\"".length,
" fun foo3() {".length,
" val foo4 = \"foo4\"".length,
)
}

assertThat(actual).contains(
"class Foo1".length,
" val foo2".length,
" fun foo3".length,
" val foo4".length,
)
@Test
fun `Given some lines containing identifiers and EOL comment then get line length exclusive the leading newline characters`() {
val code =
"""
class Foo1 {
val foo2 = "foo2" // some comment

fun foo3() {
val foo4 = "foo4" // some comment
}
}
""".trimIndent()

val actual =
transformCodeToAST(code)
.firstChildLeafOrSelf()
.leaves()
.filter { it.elementType == IDENTIFIER }
.map { identifier -> identifier.lineLength() }
.toList()

assertThat(actual).contains(
"class Foo1 {".length,
" val foo2 = \"foo2\" // some comment".length,
" fun foo3() {".length,
" val foo4 = \"foo4\" // some comment".length,
)
}

@Test
fun `Given some lines containing identifiers and EOL comment then get line length exclusive the leading newline characters and exclusive EOL comment`() {
val code =
"""
class Foo1 {
val foo2 = "foo2" // some comment

fun foo3() {
val foo4 = "foo4" // some comment
}
}
""".trimIndent()

val actual =
transformCodeToAST(code)
.firstChildLeafOrSelf()
.leaves()
.filter { it.elementType == IDENTIFIER }
.map { identifier -> identifier.lineLength(excludeEolComment = true) }
.toList()

assertThat(actual).contains(
"class Foo1 {".length,
" val foo2 = \"foo2\"".length,
" fun foo3() {".length,
" val foo4 = \"foo4\"".length,
)
}
}

@ParameterizedTest(name = "Text between FUN_KEYWORD and IDENTIFIER: {0}")
Expand Down
4 changes: 2 additions & 2 deletions ...d/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import com.pinterest.ktlint.rule.engine.core.api.indent
import com.pinterest.ktlint.rule.engine.core.api.isPartOfComment
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.lineLengthWithoutNewlinePrefix
import com.pinterest.ktlint.rule.engine.core.api.lineLength
import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
import com.pinterest.ktlint.ruleset.standard.StandardRule
import org.ec4j.core.model.PropertyType
Expand Down Expand Up @@ -128,7 +128,7 @@ public class ArgumentListWrappingRule :
false
}

private fun ASTNode.exceedsMaxLineLength() = lineLengthWithoutNewlinePrefix() > maxLineLength && !textContains('\n')
private fun ASTNode.exceedsMaxLineLength() = lineLength(excludeEolComment = true) > maxLineLength && !textContains('\n')

private fun intendedIndent(child: ASTNode): String =
when {
Expand Down
15 changes: 8 additions & 7 deletions ...c/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BinaryExpressionWrappingRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ 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.EXPERIMENTAL
import com.pinterest.ktlint.rule.engine.core.api.children
import com.pinterest.ktlint.rule.engine.core.api.dropTrailingEolComment
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
Expand All @@ -31,6 +32,7 @@ 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.leavesOnLine
import com.pinterest.ktlint.rule.engine.core.api.lineLengthWithoutNewlinePrefix
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.noNewLineInClosedRange
Expand Down Expand Up @@ -246,19 +248,18 @@ public class BinaryExpressionWrappingRule :
}
}

private fun ASTNode.isOnLineExceedingMaxLineLength() = leavesOnLine().lengthWithoutNewlinePrefix() > maxLineLength
private fun ASTNode.isOnLineExceedingMaxLineLength() =
maxLineLength <
leavesOnLine()
.dropTrailingEolComment()
.lineLengthWithoutNewlinePrefix()

private fun ASTNode.causesMaxLineLengthToBeExceeded() =
lastChildLeafOrSelf().let { lastChildLeaf ->
leavesOnLine()
.takeWhile { it.prevLeaf() != lastChildLeaf }
.lengthWithoutNewlinePrefix()
.lineLengthWithoutNewlinePrefix()
} > maxLineLength

private fun Sequence<ASTNode>.lengthWithoutNewlinePrefix() =
joinToString(separator = "") { it.text }
.dropWhile { it == '\n' }
.length
}

private fun IElementType.anyOf(vararg elementType: IElementType): Boolean = elementType.contains(this)
Expand Down
2 changes: 0 additions & 2 deletions ...src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionExpressionBodyRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,9 @@ public class FunctionExpressionBodyRule :
Rule.Experimental {
private var codeStyle = CODE_STYLE_PROPERTY.defaultValue
private var indentConfig = IndentConfig.DEFAULT_INDENT_CONFIG
private var maxLineLength = MAX_LINE_LENGTH_PROPERTY.defaultValue

override fun beforeFirstNode(editorConfig: EditorConfig) {
codeStyle = editorConfig[CODE_STYLE_PROPERTY]
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
indentConfig =
IndentConfig(
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
Expand Down
Loading
Loading