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.

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

Support partial formatting #2631

Merged
merged 4 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
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