From 33c7a8536d5e41db4b46899a789e949642b6804d Mon Sep 17 00:00:00 2001 From: yuejunyu Date: Sun, 11 Sep 2022 16:30:22 +0800 Subject: [PATCH 1/4] fix indentation wrong if suppress single line contains braces --- .../com/pinterest/ktlint/core/KtLint.kt | 21 +++++++++++-------- .../ruleset/standard/IndentationRuleTest.kt | 17 +++++++++++++++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt index a552f91035..f9199000f1 100644 --- a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt @@ -205,9 +205,8 @@ public object KtLint { ) { if (rule.shouldContinueTraversalOfAST()) { try { - if (!suppressedRegionLocator(node.startOffset, fqRuleId, node === rootNode)) { - rule.beforeVisitChildNodes(node, autoCorrect, emit) - } + val suppressedBefore = suppressedRegionLocator(node.startOffset, fqRuleId, node === rootNode) + rule.beforeVisitChildNodes(node, autoCorrect && !suppressedBefore, if (suppressedBefore) ignoredEmit else emit) if (!rule.runsOnRootNodeOnly() && rule.shouldContinueTraversalOfAST()) { node .getChildren(null) @@ -215,14 +214,16 @@ public object KtLint { // https://github.com/shyiko/ktlint/issues/158#issuecomment-462728189 // fixme: enforcing suppression based on node.startOffset is wrong (not just because not all nodes are leaves // but because rules are free to emit (and fix!) errors at any position) - if (!suppressedRegionLocator(childNode.startOffset, fqRuleId, childNode === rootNode)) { - this.executeRuleOnNodeRecursively(childNode, rule, fqRuleId, autoCorrect, emit) - } + val suppressChild = suppressedRegionLocator(childNode.startOffset, fqRuleId, childNode === rootNode) + this.executeRuleOnNodeRecursively( + childNode, rule, fqRuleId, + autoCorrect && !suppressChild, + if (suppressChild) ignoredEmit else emit, + ) } } - if (!suppressedRegionLocator(node.startOffset, fqRuleId, node === rootNode)) { - rule.afterVisitChildNodes(node, autoCorrect, emit) - } + val suppressedAfter = suppressedRegionLocator(node.startOffset, fqRuleId, node === rootNode) + rule.afterVisitChildNodes(node, autoCorrect && !suppressedAfter, if (suppressedAfter) ignoredEmit else emit) } catch (e: Exception) { if (autoCorrect) { // line/col cannot be reliably mapped as exception might originate from a node not present in the @@ -448,3 +449,5 @@ internal class RuleRunner(private val provider: RuleProvider) { @Deprecated(message = "Remove when 'ruleSets' are removed") private fun createStaticRuleProvider(rule: Rule) = RuleProvider { rule } + +private val ignoredEmit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit = { _, _, _ -> } diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt index f77ab7ace5..ec607110e8 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt @@ -3950,6 +3950,23 @@ internal class IndentationRuleTest { } } + @Test + fun `Issue 1644 - Given multiple nested brackets and some of them have ktlint-disable`(){ + val code = + """ + fun main() { + fun innerFun0() { + // some code + } // ktlint-disable indent + + fun innerFunc1() { + // some code + } + } + """.trimIndent() + indentationRuleAssertThat(code).hasNoLintViolations() + } + private companion object { val INDENT_STYLE_TAB = indentStyleProperty to PropertyType.IndentStyleValue.tab } From 001eaa2d103a56b36ddb77e7989dd132e7fdc962 Mon Sep 17 00:00:00 2001 From: paul-dingemans Date: Wed, 14 Sep 2022 18:52:56 +0200 Subject: [PATCH 2/4] Refactor Rename PreparedCode to RuleExecutionContext as changing the suppressionLocator in a Context feels less strange than in PreparedCode which sounds more immutable. Extract SuppressHandler so that the logic that alters the autoCorrect and emit variables is in one place. --- .../com/pinterest/ktlint/core/KtLint.kt | 82 ++++++++++--------- ...reparedCode.kt => RuleExecutionContext.kt} | 23 ++++-- .../ktlint/core/internal/SuppressHandler.kt | 34 ++++++++ .../ktlint/core/ast/PackageKtTest.kt | 4 +- .../ruleset/standard/IndentationRuleTest.kt | 20 ++--- 5 files changed, 103 insertions(+), 60 deletions(-) rename ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/{PreparedCode.kt => RuleExecutionContext.kt} (85%) create mode 100644 ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/SuppressHandler.kt diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt index f9199000f1..f1c349e1c5 100644 --- a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt @@ -10,11 +10,11 @@ import com.pinterest.ktlint.core.api.EditorConfigProperties import com.pinterest.ktlint.core.api.UsesEditorConfigProperties import com.pinterest.ktlint.core.internal.EditorConfigGenerator import com.pinterest.ktlint.core.internal.EditorConfigLoader -import com.pinterest.ktlint.core.internal.PreparedCode -import com.pinterest.ktlint.core.internal.SuppressionLocatorBuilder +import com.pinterest.ktlint.core.internal.RuleExecutionContext +import com.pinterest.ktlint.core.internal.SuppressHandler import com.pinterest.ktlint.core.internal.ThreadSafeEditorConfigCache.Companion.threadSafeEditorConfigCache import com.pinterest.ktlint.core.internal.VisitorProvider -import com.pinterest.ktlint.core.internal.prepareCodeForLinting +import com.pinterest.ktlint.core.internal.createRuleExecutionContext import com.pinterest.ktlint.core.internal.toQualifiedRuleId import java.nio.charset.StandardCharsets import java.nio.file.FileSystems @@ -167,14 +167,14 @@ public object KtLint { * @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation */ public fun lint(params: ExperimentalParams) { - val preparedCode = prepareCodeForLinting(params) + val ruleExecutionContext = createRuleExecutionContext(params) val errors = mutableListOf() VisitorProvider(params) - .visitor(preparedCode.editorConfigProperties) + .visitor(ruleExecutionContext.editorConfigProperties) .invoke { rule, fqRuleId -> - preparedCode.executeRule(rule, fqRuleId, false) { offset, errorMessage, canBeAutoCorrected -> - val (line, col) = preparedCode.positionInTextLocator(offset) + ruleExecutionContext.executeRule(rule, fqRuleId, false) { offset, errorMessage, canBeAutoCorrected -> + val (line, col) = ruleExecutionContext.positionInTextLocator(offset) errors.add(LintError(line, col, fqRuleId, errorMessage, canBeAutoCorrected)) } } @@ -184,7 +184,7 @@ public object KtLint { .forEach { e -> params.cb(e, false) } } - private fun PreparedCode.executeRule( + private fun RuleExecutionContext.executeRule( rule: Rule, fqRuleId: String, autoCorrect: Boolean, @@ -196,34 +196,41 @@ public object KtLint { rule.afterLastNode() } - private fun PreparedCode.executeRuleOnNodeRecursively( + private fun RuleExecutionContext.executeRuleOnNodeRecursively( node: ASTNode, rule: Rule, fqRuleId: String, autoCorrect: Boolean, emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, ) { + /** + * The [RuleExecutionContext.suppressionLocator] can be changed during each visit of node when running + * [KtLint.format]. So a new handler is to be built before visiting the nodes. + */ + val suppressHandler = SuppressHandler(suppressionLocator, rootNode, autoCorrect, emit) if (rule.shouldContinueTraversalOfAST()) { try { - val suppressedBefore = suppressedRegionLocator(node.startOffset, fqRuleId, node === rootNode) - rule.beforeVisitChildNodes(node, autoCorrect && !suppressedBefore, if (suppressedBefore) ignoredEmit else emit) + suppressHandler.handle(node, fqRuleId) { autoCorrect, emit -> + rule.beforeVisitChildNodes(node, autoCorrect, emit) + } if (!rule.runsOnRootNodeOnly() && rule.shouldContinueTraversalOfAST()) { node .getChildren(null) .forEach { childNode -> - // https://github.com/shyiko/ktlint/issues/158#issuecomment-462728189 - // fixme: enforcing suppression based on node.startOffset is wrong (not just because not all nodes are leaves - // but because rules are free to emit (and fix!) errors at any position) - val suppressChild = suppressedRegionLocator(childNode.startOffset, fqRuleId, childNode === rootNode) - this.executeRuleOnNodeRecursively( - childNode, rule, fqRuleId, - autoCorrect && !suppressChild, - if (suppressChild) ignoredEmit else emit, - ) + suppressHandler.handle(childNode, fqRuleId) { autoCorrect, emit -> + this.executeRuleOnNodeRecursively( + childNode, + rule, + fqRuleId, + autoCorrect, + emit, + ) + } } } - val suppressedAfter = suppressedRegionLocator(node.startOffset, fqRuleId, node === rootNode) - rule.afterVisitChildNodes(node, autoCorrect && !suppressedAfter, if (suppressedAfter) ignoredEmit else emit) + suppressHandler.handle(node, fqRuleId) { autoCorrect, emit -> + rule.afterVisitChildNodes(node, autoCorrect, emit) + } } catch (e: Exception) { if (autoCorrect) { // line/col cannot be reliably mapped as exception might originate from a node not present in the @@ -245,29 +252,26 @@ public object KtLint { */ public fun format(params: ExperimentalParams): String { val hasUTF8BOM = params.text.startsWith(UTF8_BOM) - val preparedCode = prepareCodeForLinting(params) + val ruleExecutionContext = createRuleExecutionContext(params) var tripped = false var mutated = false val errors = mutableSetOf>() val visitorProvider = VisitorProvider(params = params) visitorProvider - .visitor(preparedCode.editorConfigProperties) + .visitor(ruleExecutionContext.editorConfigProperties) .invoke { rule, fqRuleId -> - preparedCode.executeRule(rule, fqRuleId, true) { offset, errorMessage, canBeAutoCorrected -> + ruleExecutionContext.executeRule(rule, fqRuleId, true) { offset, errorMessage, canBeAutoCorrected -> tripped = true if (canBeAutoCorrected) { mutated = true - if (preparedCode.suppressedRegionLocator !== SuppressionLocatorBuilder.noSuppression) { - // Offsets of start and end positions of suppressed regions might have changed due to - // updating the code - preparedCode.suppressedRegionLocator = - SuppressionLocatorBuilder.buildSuppressedRegionsLocator( - preparedCode.rootNode, - ) - } + /** + * Rebuild the suppression locator after each change in the AST as the offsets of the + * suppression hints might have changed. + */ + ruleExecutionContext.rebuildSuppressionLocator() } - val (line, col) = preparedCode.positionInTextLocator(offset) + val (line, col) = ruleExecutionContext.positionInTextLocator(offset) errors.add( Pair( LintError(line, col, fqRuleId, errorMessage, canBeAutoCorrected), @@ -280,10 +284,10 @@ public object KtLint { } if (tripped) { visitorProvider - .visitor(preparedCode.editorConfigProperties) + .visitor(ruleExecutionContext.editorConfigProperties) .invoke { rule, fqRuleId -> - preparedCode.executeRule(rule, fqRuleId, false) { offset, errorMessage, canBeAutoCorrected -> - val (line, col) = preparedCode.positionInTextLocator(offset) + ruleExecutionContext.executeRule(rule, fqRuleId, false) { offset, errorMessage, canBeAutoCorrected -> + val (line, col) = ruleExecutionContext.positionInTextLocator(offset) errors.add( Pair( LintError(line, col, fqRuleId, errorMessage, canBeAutoCorrected), @@ -304,7 +308,7 @@ public object KtLint { return params.text } - val code = preparedCode + val code = ruleExecutionContext .rootNode .text .replace("\n", determineLineSeparator(params.text, params.userData)) @@ -449,5 +453,3 @@ internal class RuleRunner(private val provider: RuleProvider) { @Deprecated(message = "Remove when 'ruleSets' are removed") private fun createStaticRuleProvider(rule: Rule) = RuleProvider { rule } - -private val ignoredEmit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit = { _, _, _ -> } diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/PreparedCode.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/RuleExecutionContext.kt similarity index 85% rename from ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/PreparedCode.kt rename to ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/RuleExecutionContext.kt index 4810e6ab8e..c179a93824 100644 --- a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/PreparedCode.kt +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/RuleExecutionContext.kt @@ -11,14 +11,24 @@ import org.jetbrains.kotlin.psi.KtFile private val kotlinPsiFileFactoryProvider = KotlinPsiFileFactoryProvider() -internal class PreparedCode( +internal class RuleExecutionContext( val rootNode: FileASTNode, val editorConfigProperties: EditorConfigProperties, val positionInTextLocator: (offset: Int) -> LineAndColumn, - var suppressedRegionLocator: SuppressionLocator, -) +) { + lateinit var suppressionLocator: SuppressionLocator + private set -internal fun prepareCodeForLinting(params: KtLint.ExperimentalParams): PreparedCode { + init { + rebuildSuppressionLocator() + } + + fun rebuildSuppressionLocator() { + suppressionLocator = SuppressionLocatorBuilder.buildSuppressedRegionsLocator(rootNode) + } +} + +internal fun createRuleExecutionContext(params: KtLint.ExperimentalParams): RuleExecutionContext { val psiFileFactory = kotlinPsiFileFactoryProvider.getKotlinPsiFileFactory(params.isInvokedFromCli) val normalizedText = normalizeText(params.text) val positionInTextLocator = buildPositionInTextLocator(normalizedText) @@ -57,13 +67,10 @@ internal fun prepareCodeForLinting(params: KtLint.ExperimentalParams): PreparedC // is removed rootNode.putUserData(KtLint.EDITOR_CONFIG_PROPERTIES_USER_DATA_KEY, editorConfigProperties) - val suppressedRegionLocator = SuppressionLocatorBuilder.buildSuppressedRegionsLocator(rootNode) - - return PreparedCode( + return RuleExecutionContext( rootNode, editorConfigProperties, positionInTextLocator, - suppressedRegionLocator, ) } diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/SuppressHandler.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/SuppressHandler.kt new file mode 100644 index 0000000000..883b01c06a --- /dev/null +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/SuppressHandler.kt @@ -0,0 +1,34 @@ +package com.pinterest.ktlint.core.internal + +import org.jetbrains.kotlin.com.intellij.lang.ASTNode + +internal class SuppressHandler( + private val suppressionLocator: SuppressionLocator, + private val rootNode: ASTNode, + private val autoCorrect: Boolean, + private val emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, +) { + fun handle( + node: ASTNode, + ruleId: String, + function: (Boolean, (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) -> Unit, + ) { + val suppress = suppressionLocator( + node.startOffset, + ruleId, + node == rootNode, + ) + val autoCorrect = this.autoCorrect && !suppress + val emit = if (suppress) { + suppressEmit + } else { + this.emit + } + function(autoCorrect, emit) + } + + private companion object { + // Swallow violation so that it is not emitted + val suppressEmit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit = { _, _, _ -> } + } +} diff --git a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/ast/PackageKtTest.kt b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/ast/PackageKtTest.kt index 069c41bce3..c4e8da7b9e 100644 --- a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/ast/PackageKtTest.kt +++ b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/ast/PackageKtTest.kt @@ -6,7 +6,7 @@ import com.pinterest.ktlint.core.RuleProvider import com.pinterest.ktlint.core.ast.ElementType.CLASS import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY import com.pinterest.ktlint.core.ast.ElementType.ENUM_ENTRY -import com.pinterest.ktlint.core.internal.prepareCodeForLinting +import com.pinterest.ktlint.core.internal.createRuleExecutionContext import org.assertj.core.api.Assertions.assertThat import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.lang.FileASTNode @@ -231,7 +231,7 @@ class PackageKtTest { } private fun transformCodeToAST(code: String) = - prepareCodeForLinting( + createRuleExecutionContext( KtLint.ExperimentalParams( text = code, diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt index ec607110e8..21580ebe6b 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt @@ -3951,19 +3951,19 @@ internal class IndentationRuleTest { } @Test - fun `Issue 1644 - Given multiple nested brackets and some of them have ktlint-disable`(){ + fun `Issue 1644 - Given multiple nested brackets and some of them have ktlint-disable`() { val code = - """ - fun main() { - fun innerFun0() { - // some code - } // ktlint-disable indent + """ + fun fooBar() { + fun foo() { + // some code + } // ktlint-disable indent - fun innerFunc1() { - // some code - } + fun bar() { + // some code } - """.trimIndent() + } + """.trimIndent() indentationRuleAssertThat(code).hasNoLintViolations() } From e73485082028e4e05dfab5846867e798df266372 Mon Sep 17 00:00:00 2001 From: paul-dingemans Date: Wed, 14 Sep 2022 21:36:43 +0200 Subject: [PATCH 3/4] Update changelog Closes #1645 --- CHANGELOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0358f50e32..d873adfc6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/). +## Unreleased + +### API Changes & RuleSet providers + +### Added + +### Fixed + +* Let a rule process all nodes even in case the rule is suppressed for a node so that the rule can update the internal state ([#1645](https://github.com/pinterest/ktlint/issue/1645)) + +### Changed + + ## [0.47.1] - 2022-09-07 ### Fixed From a45f516b0542115216a99316e291c2399917223a Mon Sep 17 00:00:00 2001 From: paul-dingemans Date: Fri, 16 Sep 2022 20:13:10 +0200 Subject: [PATCH 4/4] Fix changelog Closes #1644 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d873adfc6e..30ba8b6fc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* Let a rule process all nodes even in case the rule is suppressed for a node so that the rule can update the internal state ([#1645](https://github.com/pinterest/ktlint/issue/1645)) +* Let a rule process all nodes even in case the rule is suppressed for a node so that the rule can update the internal state ([#1644](https://github.com/pinterest/ktlint/issue/1644)) ### Changed