fix indentation rule has problems with ignoring braces (#1646)

* fix indentation wrong if suppress single line contains braces
* 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.

Closes #1644

Co-authored-by: yuejunyu <yuejunyu.0@bytedance.com>
Co-authored-by: paul-dingemans <paul-dingemans@users.noreply.github.com>

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

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<LintError>()
 
  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<Pair<LintError, Boolean>>()
  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))

ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/PreparedCode.kt → 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,
  )
 }
 

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 = { _, _, _ -> }
+ }
+}

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,

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
  }