Skip to content

Commit

Permalink
Ignore tokens between backticks in rule max line length (#1007) (#1008)
Browse files Browse the repository at this point in the history
* Ignore tokens between backticks in rule max line length (#1007)

Tokens starting and ending with a backtick should be ignored entirely
when validating whether a line exceeds the maximum length of a line.
Such tokens can not be spit into multiple lines. For example when
writing JUnit test it is a common pattern to write the function names
between back ticks instead of using the DisplayName annotation.
  • Loading branch information
paul-dingemans authored Mar 7, 2021
1 parent 37453ec commit 4306d76
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased
### Added
- New `ktlint_ignore_back_ticked_identifier` EditorConfig option for `max-line-length` rule to ignore long method names inside backticks
(primarily used in tests) ([#1007](https://github.com/pinterest/ktlint/issues/1007))

### Fixed
- Incorrect indentation with multiple interfaces ([#1003](https://github.com/pinterest/ktlint/issues/1003))
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,12 @@ disabled_rules=no-wildcard-imports,experimental:annotation,my-custom-ruleset:my-
ij_kotlin_imports_layout=* # alphabetical with capital letters before lower case letters (e.g. Z before a), no blank lines
ij_kotlin_imports_layout=*,java.**,javax.**,kotlin.**,^ # default IntelliJ IDEA style, same as alphabetical, but with "java", "javax", "kotlin" and alias imports in the end of the imports list
ij_kotlin_imports_layout=android.**,|,^org.junit.**,kotlin.io.Closeable.*,|,*,^ # custom imports layout

# According to https://kotlinlang.org/docs/reference/coding-conventions.html#names-for-test-methods it is acceptable to write method names
# in natural language. When using natural language, the description tends to be longer. Allow lines containing an identifier between
# backticks to be longer than the maximum line length. (Since 0.41.0)
[**/test/**.kt]
ktlint_ignore_back_ticked_identifier=true
```

### Overriding Editorconfig properties for specific directories
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface EditorConfig {
val indentSize: Int
val tabWidth: Int
val maxLineLength: Int

@Deprecated(
message = "Not used anymore by rules, please use 'insert_final_newline' directly via get()"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,33 @@ package com.pinterest.ktlint.ruleset.standard

import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.api.EditorConfigProperties
import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.isPartOf
import com.pinterest.ktlint.core.ast.isRoot
import com.pinterest.ktlint.core.ast.nextLeaf
import com.pinterest.ktlint.core.ast.parent
import com.pinterest.ktlint.core.ast.prevCodeSibling
import org.ec4j.core.model.PropertyType
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.kdoc.psi.api.KDoc
import org.jetbrains.kotlin.psi.KtImportDirective
import org.jetbrains.kotlin.psi.KtPackageDirective

class MaxLineLengthRule : Rule("max-line-length"), Rule.Modifier.Last {
@OptIn(FeatureInAlphaState::class)
class MaxLineLengthRule :
Rule("max-line-length"),
Rule.Modifier.Last,
UsesEditorConfigProperties {

override val editorConfigProperties: List<UsesEditorConfigProperties.EditorConfigProperty<*>> = listOf(
ignoreBackTickedIdentifierProperty
)

private var maxLineLength: Int = -1
private var rangeTree = RangeTree()
Expand All @@ -26,17 +40,19 @@ class MaxLineLengthRule : Rule("max-line-length"), Rule.Modifier.Last {
) {
if (node.isRoot()) {
val editorConfig = node.getUserData(KtLint.EDITOR_CONFIG_USER_DATA_KEY)!!
val editorConfigProperties: EditorConfigProperties =
node.getUserData(KtLint.EDITOR_CONFIG_PROPERTIES_USER_DATA_KEY)!!
val ignoreBackTickedIdentifier = editorConfigProperties.getEditorConfigValue(ignoreBackTickedIdentifierProperty)
maxLineLength = editorConfig.maxLineLength
if (maxLineLength <= 0) {
return
}
val errorOffset = arrayListOf<Int>()
val text = node.text
val lines = text.split("\n")
var offset = 0
for (line in lines) {
if (line.length > maxLineLength) {
val el = node.psi.findElementAt(offset + line.length - 1)!!.node
node
.getElementsPerLine()
.filter { it.lineLength(ignoreBackTickedIdentifier) > maxLineLength }
.forEach { parsedLine ->
val el = parsedLine.elements.last()
if (!el.isPartOf(KDoc::class) && !el.isPartOfRawMultiLineString()) {
if (!el.isPartOf(PsiComment::class)) {
if (!el.isPartOf(KtPackageDirective::class) && !el.isPartOf(KtImportDirective::class)) {
Expand All @@ -45,27 +61,25 @@ class MaxLineLengthRule : Rule("max-line-length"), Rule.Modifier.Last {
// node spanning the same offset is 'visit'ed
// (for ktlint-disable directive to have effect (when applied))
// this will be rectified in the upcoming release(s)
errorOffset.add(offset)
errorOffset.add(parsedLine.offset)
}
} else {
// Allow ktlint-disable comments to exceed max line length
if (!el.text.startsWith("// ktlint-disable")) {
// if comment is the only thing on the line - fine, otherwise emit an error
val prevLeaf = el.prevCodeSibling()
if (prevLeaf != null && prevLeaf.startOffset >= offset) {
if (prevLeaf != null && prevLeaf.startOffset >= parsedLine.offset) {
// fixme:
// normally we would emit here but due to API limitations we need to hold off until
// node spanning the same offset is 'visit'ed
// (for ktlint-disable directive to have effect (when applied))
// this will be rectified in the upcoming release(s)
errorOffset.add(offset)
errorOffset.add(parsedLine.offset)
}
}
}
}
}
offset += line.length + 1
}
rangeTree = RangeTree(errorOffset)
} else if (!rangeTree.isEmpty() && node.psi is LeafPsiElement) {
rangeTree
Expand All @@ -79,6 +93,63 @@ class MaxLineLengthRule : Rule("max-line-length"), Rule.Modifier.Last {
private fun ASTNode.isPartOfRawMultiLineString() =
parent(ElementType.STRING_TEMPLATE, strict = false)
?.let { it.firstChildNode.text == "\"\"\"" && it.textContains('\n') } == true

public companion object {
internal const val KTLINT_IGNORE_BACKTICKED_IDENTIFIER_NAME = "ktlint_ignore_back_ticked_identifier"
private const val PROPERTY_DESCRIPTION = "Defines whether the backticked identifier (``) should be ignored"

public val ignoreBackTickedIdentifierProperty: UsesEditorConfigProperties.EditorConfigProperty<Boolean> =
UsesEditorConfigProperties.EditorConfigProperty(
type = PropertyType(
/* name = */ KTLINT_IGNORE_BACKTICKED_IDENTIFIER_NAME,
/* description = */ PROPERTY_DESCRIPTION,
/* parser = */ PropertyType.PropertyValueParser.BOOLEAN_VALUE_PARSER
),
defaultValue = false
)
}
}

private fun ASTNode.getElementsPerLine(): List<ParsedLine> {
val parsedLines = mutableListOf<ParsedLine>()
val lines = text.split("\n")
var offset = 0
for (line in lines) {
val elements = mutableListOf<ASTNode>()
var el = psi.findElementAt(offset)?.node
while (el != null && el.startOffset < offset + line.length) {
elements.add(el)
el = el.nextLeaf()
}
parsedLines.add(ParsedLine(line, offset, elements))
offset += line.length + 1 // +1 for the newline which is stripped due to the splitting of the lines
}
return parsedLines
}

private data class ParsedLine(
val line: String,
val offset: Int,
val elements: List<ASTNode>
) {
fun lineLength(ignoreBackTickedIdentifier: Boolean): Int {
return if (ignoreBackTickedIdentifier) {
line.length - totalLengthBacktickedElements()
} else {
line.length
}
}

private fun totalLengthBacktickedElements(): Int {
return elements
.filterIsInstance(PsiElement::class.java)
.filter { it.text.matches(isValueBetweenBackticks) }
.sumBy(PsiElement::getTextLength)
}

private companion object {
val isValueBetweenBackticks = Regex("`.*`")
}
}

class RangeTree(seq: List<Int> = emptyList()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
package com.pinterest.ktlint.ruleset.standard

import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.test.EditorConfigTestRule
import com.pinterest.ktlint.test.diffFileLint
import com.pinterest.ktlint.test.lint
import java.io.File
import org.assertj.core.api.Assertions.assertThat
import org.junit.Rule
import org.junit.Test

@OptIn(FeatureInAlphaState::class)
class MaxLineLengthRuleTest {

@get:Rule
val editorConfigTestRule = EditorConfigTestRule()

@Test
fun testLint() {
assertThat(
Expand Down Expand Up @@ -38,6 +46,50 @@ class MaxLineLengthRuleTest {
)
}

@Test
fun testErrorSuppressionOnTokensBetweenBackticks() {
val testFile = ignoreBacktickedIdentifier()

assertThat(
MaxLineLengthRule().lint(
testFile.absolutePath,
"""
@Test
fun `Some too long test description between backticks`() {
println("teeeeeeeeeeeeeeeeeeeeeeeext")
}
""".trimIndent(),
userData = mapOf(
"max_line_length" to "40"
)
)
).isEqualTo(
listOf(
// Note that no error was generated on line 2 with the long fun name but on another line
LintError(3, 1, "max-line-length", "Exceeded max line length (40)")
)
)
}

@Test
fun testReportLongLinesAfterExcludingTokensBetweenBackticks() {
assertThat(
MaxLineLengthRule().lint(
"""
@ParameterizedTest
fun `Some too long test description between backticks`(looooooooongParameterName: String) {
println("teeeeeeeeext")
}
""".trimIndent(),
userData = mapOf("max_line_length" to "40")
)
).isEqualTo(
listOf(
LintError(2, 1, "max-line-length", "Exceeded max line length (40)")
)
)
}

@Test
fun testLintOff() {
assertThat(
Expand All @@ -61,4 +113,9 @@ class MaxLineLengthRuleTest {
assertThat(RangeTree((5 until 10).asSequence().toList()).query(10, 15).toString()).isEqualTo("[]")
assertThat(RangeTree(listOf(1, 5, 10)).query(3, 4).toString()).isEqualTo("[]")
}

private fun ignoreBacktickedIdentifier(): File = editorConfigTestRule
.writeToEditorConfig(
mapOf(MaxLineLengthRule.ignoreBackTickedIdentifierProperty.type to true.toString())
)
}

0 comments on commit 4306d76

Please sign in to comment.