Skip to content

Commit

Permalink
Support partial formatting (range) in given code
Browse files Browse the repository at this point in the history
Allows API consumers like the 'ktlint-intellij-plugin' to format a block of code inside the given code (for example a file) without autocorrect the given code entirely.

The start offset of the node on which a rule can detect a lint violation should be inside the range which is to be formatted. This has some unexpected side effects for some rules. In most cases it is to be expected that the user won't notice those side effects. And if it happens, the most likely way the user responds is widening the range which is to be formatted, and try to format again.

For example, the given code might contain the when-statement below:
```
   // code with lint violations

    when(foobar) {
        FOO -> "Single line"
        BAR ->
            """
            Multi line
            """.trimIndent()
        else -> null
    }

   // more code with lint violations
```
The `blank-line-between-when-conditions` rule requires blank lines to be added between the conditions. If the when-keyword above is included in the range which is to be formatted then the blank lines before the conditions are added. If only the when-conditions itself are selected, but not the when-keyword, then the blank lines are not added.

This unexpected behavior is a side effect of the way the partial formatting is implemented currently. The side effects can be prevented by delaying the decision to autocorrect as late as possible and the exact offset of the error is known. This however would cause a breaking change, and needs to wait until Ktlint V2.x.

Closes #2629
  • Loading branch information
paul-dingemans committed Apr 23, 2024
1 parent c9a97af commit fd083b6
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 9 deletions.
Expand Up @@ -272,6 +272,47 @@ class KtLintRuleEngineTest {
""".trimIndent(),
)
}

@Test
fun `Given a kotlin code snippet that does contain multiple indentation errors then only format errors found in given range`() {
val ktLintRuleEngine =
KtLintRuleEngine(
ruleProviders =
setOf(
RuleProvider { IndentationRule() },
),
fileSystem = ktlintTestFileSystem.fileSystem,
)

val originalCode =
"""
fun main() {
println("Hello world!")
println("Hello world!")
println("Hello world!")
}
""".trimIndent()
val newlineIndexes =
Regex("\n")
.findAll(originalCode)
.map { it.range.first }
.toList()
val actual =
ktLintRuleEngine.format(
code = Code.fromSnippet(originalCode),
autoCorrectOffsetRange = IntRange(newlineIndexes[1], newlineIndexes[2]),
)

assertThat(actual).isEqualTo(
"""
fun main() {
println("Hello world!")
println("Hello world!")
println("Hello world!")
}
""".trimIndent(),
)
}
}

@Test
Expand Down
2 changes: 2 additions & 0 deletions ktlint-rule-engine/api/ktlint-rule-engine.api
Expand Up @@ -69,7 +69,9 @@ public final class com/pinterest/ktlint/rule/engine/api/KtLintRuleEngine {
public synthetic fun <init> (Ljava/util/Set;Lcom/pinterest/ktlint/rule/engine/api/EditorConfigDefaults;Lcom/pinterest/ktlint/rule/engine/api/EditorConfigOverride;ZLjava/nio/file/FileSystem;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun editorConfigFilePaths (Ljava/nio/file/Path;)Ljava/util/List;
public final fun format (Lcom/pinterest/ktlint/rule/engine/api/Code;Lkotlin/jvm/functions/Function2;)Ljava/lang/String;
public final fun format (Lcom/pinterest/ktlint/rule/engine/api/Code;Lkotlin/ranges/IntRange;Lkotlin/jvm/functions/Function2;)Ljava/lang/String;
public static synthetic fun format$default (Lcom/pinterest/ktlint/rule/engine/api/KtLintRuleEngine;Lcom/pinterest/ktlint/rule/engine/api/Code;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Ljava/lang/String;
public static synthetic fun format$default (Lcom/pinterest/ktlint/rule/engine/api/KtLintRuleEngine;Lcom/pinterest/ktlint/rule/engine/api/Code;Lkotlin/ranges/IntRange;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Ljava/lang/String;
public final fun generateKotlinEditorConfigSection (Ljava/nio/file/Path;)Ljava/lang/String;
public final fun getEditorConfigDefaults ()Lcom/pinterest/ktlint/rule/engine/api/EditorConfigDefaults;
public final fun getEditorConfigOverride ()Lcom/pinterest/ktlint/rule/engine/api/EditorConfigOverride;
Expand Down
Expand Up @@ -12,6 +12,10 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CodeStyleValue
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.END_OF_LINE_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.propertyTypes
import com.pinterest.ktlint.rule.engine.core.util.safeAs
import com.pinterest.ktlint.rule.engine.internal.AutoCorrectDisabledHandler
import com.pinterest.ktlint.rule.engine.internal.AutoCorrectEnabledHandler
import com.pinterest.ktlint.rule.engine.internal.AutoCorrectHandler
import com.pinterest.ktlint.rule.engine.internal.AutoCorrectOffsetRangeHandler
import com.pinterest.ktlint.rule.engine.internal.EditorConfigFinder
import com.pinterest.ktlint.rule.engine.internal.EditorConfigGenerator
import com.pinterest.ktlint.rule.engine.internal.EditorConfigLoader
Expand Down Expand Up @@ -97,7 +101,7 @@ public class KtLintRuleEngine(
VisitorProvider(ruleExecutionContext.ruleProviders)
.visitor()
.invoke { rule ->
ruleExecutionContext.executeRule(rule, false) { offset, errorMessage, canBeAutoCorrected ->
ruleExecutionContext.executeRule(rule, AutoCorrectDisabledHandler) { offset, errorMessage, canBeAutoCorrected ->
val (line, col) = ruleExecutionContext.positionInTextLocator(offset)
LintError(line, col, rule.ruleId, errorMessage, canBeAutoCorrected)
.let { lintError ->
Expand Down Expand Up @@ -127,6 +131,53 @@ public class KtLintRuleEngine(
public fun format(
code: Code,
callback: (LintError, Boolean) -> Unit = { _, _ -> },
): String = format(code, AutoCorrectEnabledHandler, callback)

/**
* Fix style violations in [code] for lint errors found in the [autoCorrectOffsetRange] when possible. If [code] is passed as file
* reference then the '.editorconfig' files on the path are taken into account. For each lint violation found, the [callback] is
* invoked.
*
* IMPORTANT: Partial formatting not always works as expected. The offset of the node which is triggering the violation does not
* necessarily to be close to the offset at which the violation is reported. Counter-intuitively the offset of the trigger node must be
* located inside the [autoCorrectOffsetRange] instead of the offset at which the violation is reported.
*
* For example, the given code might contain the when-statement below:
* ```
* // code with lint violations
*
* when(foobar) {
* FOO -> "Single line"
* BAR ->
* """
* Multi line
* """.trimIndent()
* else -> null
* }
*
* // more code with lint violations
* ```
* The `blank-line-between-when-conditions` rule requires blank lines to be added between the conditions. If the when-keyword above is
* included in the range which is to be formatted then the blank lines before the conditions are added. If only the when-conditions
* itself are selected, but not the when-keyword, then the blank lines are not added.
*
* This unexpected behavior is a side effect of the way the partial formatting is implemented currently. The side effects can be
* prevented by delaying the decision to autocorrect as late as possible and the exact offset of the error is known. This however would
* cause a breaking change, and needs to wait until Ktlint V2.x.
*
* @throws KtLintParseException if text is not a valid Kotlin code
* @throws KtLintRuleException in case of internal failure caused by a bug in rule implementation
*/
public fun format(
code: Code,
autoCorrectOffsetRange: IntRange,
callback: (LintError, Boolean) -> Unit = { _, _ -> },
): String = format(code, AutoCorrectOffsetRangeHandler(autoCorrectOffsetRange), callback)

private fun format(
code: Code,
autoCorrectHandler: AutoCorrectHandler,
callback: (LintError, Boolean) -> Unit = { _, _ -> },
): String {
LOGGER.debug { "Starting with formatting file '${code.fileNameOrStdin()}'" }

Expand All @@ -143,7 +194,7 @@ public class KtLintRuleEngine(
visitorProvider
.visitor()
.invoke { rule ->
ruleExecutionContext.executeRule(rule, true) { offset, errorMessage, canBeAutoCorrected ->
ruleExecutionContext.executeRule(rule, autoCorrectHandler) { offset, errorMessage, canBeAutoCorrected ->
if (canBeAutoCorrected) {
mutated = true
/*
Expand Down Expand Up @@ -178,7 +229,7 @@ public class KtLintRuleEngine(
.visitor()
.invoke { rule ->
if (!hasErrorsWhichCanBeAutocorrected) {
ruleExecutionContext.executeRule(rule, false) { _, _, canBeAutoCorrected ->
ruleExecutionContext.executeRule(rule, AutoCorrectDisabledHandler) { _, _, canBeAutoCorrected ->
if (canBeAutoCorrected) {
hasErrorsWhichCanBeAutocorrected = true
}
Expand Down
@@ -0,0 +1,24 @@
package com.pinterest.ktlint.rule.engine.internal

import org.jetbrains.kotlin.com.intellij.lang.ASTNode

/**
* Handler which determines whether autocorrect should be enabled or disabled for the given offset.
*/
internal sealed interface AutoCorrectHandler {
fun autoCorrect(node: ASTNode): Boolean
}

internal data object AutoCorrectDisabledHandler : AutoCorrectHandler {
override fun autoCorrect(node: ASTNode) = false
}

internal data object AutoCorrectEnabledHandler : AutoCorrectHandler {
override fun autoCorrect(node: ASTNode) = true
}

internal class AutoCorrectOffsetRangeHandler(
private val autoCorrectOffsetRange: IntRange,
) : AutoCorrectHandler {
override fun autoCorrect(node: ASTNode) = node.startOffset in autoCorrectOffsetRange
}
Expand Up @@ -46,7 +46,7 @@ internal class RuleExecutionContext private constructor(

fun executeRule(
rule: Rule,
autoCorrect: Boolean,
autoCorrectHandler: AutoCorrectHandler,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
try {
Expand All @@ -59,7 +59,7 @@ internal class RuleExecutionContext private constructor(
// EditorConfigProperty that is not explicitly defined.
editorConfig.filterBy(rule.usesEditorConfigProperties.plus(CODE_STYLE_PROPERTY)),
)
this.executeRuleOnNodeRecursively(rootNode, rule, autoCorrect, emit)
this.executeRuleOnNodeRecursively(rootNode, rule, autoCorrectHandler, emit)
rule.afterLastNode()
} catch (e: RuleExecutionException) {
throw KtLintRuleException(
Expand All @@ -80,17 +80,28 @@ internal class RuleExecutionContext private constructor(
private fun executeRuleOnNodeRecursively(
node: ASTNode,
rule: Rule,
autoCorrect: Boolean,
autoCorrectHandler: AutoCorrectHandler,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
// TODO: In Ktlint V2 the autocorrect handler should be passed down to the rules, so that the autocorrect handler can check the
// offset at which the violation is found is in the autocorrect range or not. Currently it is checked whether the offset of the
// node that is triggering the violation is in the range. This has following side effects:
// * if the offset of the node which triggers the violation is inside the range, but the offset of the violation itself it outside
// the autocorrect range than a change is made to code outside the selected range
// * if the offset of the node which triggers the violation is outside the range, but the offset of the violation itself it inside
// the autocorrect range than *not* change is made to code which is in the selected range while the user would have expected it
// to be changed.
// Passing down the autocorrectHandler to the rules is a breaking change as the Rule signature needs to be changed.
val autoCorrect = autoCorrectHandler.autoCorrect(node)

/**
* The [suppressionLocator] can be changed during each visit of node when running [KtLintRuleEngine.format]. So a new handler is to
* be built before visiting the nodes.
*/
val suppressHandler = SuppressHandler(suppressionLocator, autoCorrect, emit)
if (rule.shouldContinueTraversalOfAST()) {
try {
executeRuleOnNodeRecursively(node, rule, suppressHandler)
executeRuleOnNodeRecursively(node, rule, autoCorrectHandler, suppressHandler)
} catch (e: Exception) {
if (autoCorrect) {
// line/col cannot be reliably mapped as exception might originate from a node not present in the
Expand Down Expand Up @@ -119,6 +130,7 @@ internal class RuleExecutionContext private constructor(
private fun executeRuleOnNodeRecursively(
node: ASTNode,
rule: Rule,
autoCorrectHandler: AutoCorrectHandler,
suppressHandler: SuppressHandler,
) {
suppressHandler.handle(node, rule.ruleId) { autoCorrect, emit ->
Expand All @@ -128,11 +140,11 @@ internal class RuleExecutionContext private constructor(
node
.getChildren(null)
.forEach { childNode ->
suppressHandler.handle(childNode, rule.ruleId) { autoCorrect, emit ->
suppressHandler.handle(childNode, rule.ruleId) { _, emit ->
this.executeRuleOnNodeRecursively(
childNode,
rule,
autoCorrect,
autoCorrectHandler,
emit,
)
}
Expand Down

0 comments on commit fd083b6

Please sign in to comment.