From fd083b67a1ecb2a2821a3e65a236f912ccd0eca6 Mon Sep 17 00:00:00 2001 From: Paul Dingemans Date: Tue, 23 Apr 2024 11:55:52 +0200 Subject: [PATCH] Support partial formatting (range) in given code 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 --- .../api/consumer/KtLintRuleEngineTest.kt | 41 +++++++++++++ ktlint-rule-engine/api/ktlint-rule-engine.api | 2 + .../rule/engine/api/KtLintRuleEngine.kt | 57 ++++++++++++++++++- .../engine/internal/AutoCorrectHandler.kt | 24 ++++++++ .../engine/internal/RuleExecutionContext.kt | 24 ++++++-- 5 files changed, 139 insertions(+), 9 deletions(-) create mode 100644 ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/AutoCorrectHandler.kt diff --git a/ktlint-api-consumer/src/test/kotlin/com/pinterest/ktlint/api/consumer/KtLintRuleEngineTest.kt b/ktlint-api-consumer/src/test/kotlin/com/pinterest/ktlint/api/consumer/KtLintRuleEngineTest.kt index 82fb79d761..902043080e 100644 --- a/ktlint-api-consumer/src/test/kotlin/com/pinterest/ktlint/api/consumer/KtLintRuleEngineTest.kt +++ b/ktlint-api-consumer/src/test/kotlin/com/pinterest/ktlint/api/consumer/KtLintRuleEngineTest.kt @@ -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 diff --git a/ktlint-rule-engine/api/ktlint-rule-engine.api b/ktlint-rule-engine/api/ktlint-rule-engine.api index 8cb2f0e5b6..85560f94f4 100644 --- a/ktlint-rule-engine/api/ktlint-rule-engine.api +++ b/ktlint-rule-engine/api/ktlint-rule-engine.api @@ -69,7 +69,9 @@ public final class com/pinterest/ktlint/rule/engine/api/KtLintRuleEngine { public synthetic fun (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; diff --git a/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/api/KtLintRuleEngine.kt b/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/api/KtLintRuleEngine.kt index 82af41d9d8..2d32c5f0e4 100644 --- a/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/api/KtLintRuleEngine.kt +++ b/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/api/KtLintRuleEngine.kt @@ -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 @@ -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 -> @@ -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()}'" } @@ -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 /* @@ -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 } diff --git a/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/AutoCorrectHandler.kt b/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/AutoCorrectHandler.kt new file mode 100644 index 0000000000..66e99fa95a --- /dev/null +++ b/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/AutoCorrectHandler.kt @@ -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 +} diff --git a/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/RuleExecutionContext.kt b/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/RuleExecutionContext.kt index 3d8c733fda..802b9c03e6 100644 --- a/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/RuleExecutionContext.kt +++ b/ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/RuleExecutionContext.kt @@ -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 { @@ -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( @@ -80,9 +80,20 @@ 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. @@ -90,7 +101,7 @@ internal class RuleExecutionContext private constructor( 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 @@ -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 -> @@ -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, ) }