diff --git a/CHANGELOG.md b/CHANGELOG.md index 0358f50e32..30ba8b6fc8 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 ([#1644](https://github.com/pinterest/ktlint/issue/1644)) + +### Changed + + ## [0.47.1] - 2022-09-07 ### Fixed 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..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,31 +196,39 @@ 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 { - if (!suppressedRegionLocator(node.startOffset, fqRuleId, node === rootNode)) { + 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) - if (!suppressedRegionLocator(childNode.startOffset, fqRuleId, childNode === rootNode)) { - this.executeRuleOnNodeRecursively(childNode, rule, fqRuleId, autoCorrect, emit) + suppressHandler.handle(childNode, fqRuleId) { autoCorrect, emit -> + this.executeRuleOnNodeRecursively( + childNode, + rule, + fqRuleId, + autoCorrect, + emit, + ) } } } - if (!suppressedRegionLocator(node.startOffset, fqRuleId, node === rootNode)) { + suppressHandler.handle(node, fqRuleId) { autoCorrect, emit -> rule.afterVisitChildNodes(node, autoCorrect, emit) } } catch (e: Exception) { @@ -244,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), @@ -279,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), @@ -303,7 +308,7 @@ public object KtLint { return params.text } - val code = preparedCode + val code = ruleExecutionContext .rootNode .text .replace("\n", determineLineSeparator(params.text, params.userData)) 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 f77ab7ace5..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 @@ -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 fooBar() { + fun foo() { + // some code + } // ktlint-disable indent + + fun bar() { + // some code + } + } + """.trimIndent() + indentationRuleAssertThat(code).hasNoLintViolations() + } + private companion object { val INDENT_STYLE_TAB = indentStyleProperty to PropertyType.IndentStyleValue.tab }