Skip to content

Commit

Permalink
Call callback on all errors (#1499)
Browse files Browse the repository at this point in the history 
Closes #1491
  • Loading branch information
@paul-dingemans
paul-dingemans committed Jul 14, 2022
1 parent dbc7453 commit 0949748
Show file tree
Hide file tree
Showing 3 changed files with 177 additions and 7 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Expand Up @@ -53,12 +53,16 @@ If your project did not run with the `experimental` ruleset enabled before, you

### API Changes & RuleSet providers

If you are not an API user nor a RuleSet provider, then you can safely skip this section. Otherwise, please read below carefully and upgrade your usage of ktlint. In this and coming releases, we are changing and adapting important parts of our API in order to increase maintainability and flexibility for future changes. Please avoid skipping a releases as that will make it harder to migrate.
If you are not an API consumer nor a RuleSet provider, then you can safely skip this section. Otherwise, please read below carefully and upgrade your usage of ktlint. In this and coming releases, we are changing and adapting important parts of our API in order to increase maintainability and flexibility for future changes. Please avoid skipping a releases as that will make it harder to migrate.

#### Lint and formatting functions

The lint and formatting changes no longer accept parameters of type `Params` but only `ExperimentalParams`. Also, the VisitorProvider parameter has been removed. Because of this, your integration with KtLint breaks. Based on feedback with ktlint 0.45.x, we now prefer to break at compile time instead of trying to keep the interface backwards compatible. Please raise an issue, in case you help to convert to the new API.

#### Format callback

The callback function provided as parameter to the format function is now called for all errors regardless whether the error has been autocorrected. Existing consumers of the format function should now explicitly check the `autocorrected` flag in the callback result and handle it appropriately (in most case this will be ignoring the callback results for which `autocorrected` has value `true`).

#### Use of ".editorconfig" properties & userData

The interface `UsesEditorConfigProperties` provides method `getEditorConfigValue` to retrieve a named `.editorconfig` property for a given ASTNode. When implementing this interface, the value `editorConfigProperties` needs to be overridden. Previously it was not checked whether a retrieved property was actually recorded in this list. Now, retrieval of unregistered properties results in an exception.
Expand Down Expand Up @@ -121,6 +125,7 @@ An AssertJ style API for testing KtLint rules ([#1444](https://github.com/pinter
- Fix indentation of property getter/setter when the property has an initializer on a separate line `indent` ([#1335](https://github.com/pinterest/ktlint/issues/1335))
- When `.editorconfig` setting `indentSize` is set to value `tab` then return the default tab width as value for `indentSize` ([#1485](https://github.com/pinterest/ktlint/issues/1485))
- Allow suppressing all rules or a list of specific rules in the entire file with `@file:Suppress(...)` ([#1029](https://github.com/pinterest/ktlint/issues/1029))
- Invoke callback on `format` function for all errors including errors that are autocorrected ([#1491](https://github.com/pinterest/ktlint/issues/1491))


### Changed
Expand Down
23 changes: 17 additions & 6 deletions ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt
View file
Expand Up @@ -236,6 +236,7 @@ public object KtLint {

var tripped = false
var mutated = false
val errors = mutableSetOf<Pair<LintError, Boolean>>()
val visitorProvider = VisitorProvider(params = params)
visitorProvider
.visitor(
Expand All @@ -248,7 +249,7 @@ public object KtLint {
!preparedCode.suppressedRegionLocator(node.startOffset, fqRuleId, node === preparedCode.rootNode)
) {
try {
rule.visit(node, true) { _, _, canBeAutoCorrected ->
rule.visit(node, true) { offset, errorMessage, canBeAutoCorrected ->
tripped = true
if (canBeAutoCorrected) {
mutated = true
Expand All @@ -259,6 +260,15 @@ public object KtLint {
)
}
}
val (line, col) = preparedCode.positionInTextLocator(offset)
errors.add(
Pair(
LintError(line, col, fqRuleId, errorMessage, canBeAutoCorrected),
// It is assumed that a rule that emits that an error can be autocorrected, also
// does correct the error.
canBeAutoCorrected
)
)
}
} catch (e: Exception) {
// line/col cannot be reliably mapped as exception might originate from a node not present
Expand All @@ -268,7 +278,6 @@ public object KtLint {
}
}
if (tripped) {
val errors = mutableListOf<Pair<LintError, Boolean>>()
visitorProvider
.visitor(preparedCode.rootNode)
.invoke { node, rule, fqRuleId ->
Expand Down Expand Up @@ -298,6 +307,8 @@ public object KtLint {
errors.add(
Pair(
LintError(line, col, fqRuleId, errorMessage, canBeAutoCorrected),
// It is assumed that a rule only corrects an error after it has emitted an
// error and indicating that it actually can be autocorrected.
false
)
)
Expand All @@ -309,12 +320,12 @@ public object KtLint {
}
}
}

errors
.sortedWith { (l), (r) -> if (l.line != r.line) l.line - r.line else l.col - r.col }
.forEach { (e, corrected) -> params.cb(e, corrected) }
}

errors
.sortedWith { (l), (r) -> if (l.line != r.line) l.line - r.line else l.col - r.col }
.forEach { (e, corrected) -> params.cb(e, corrected) }

if (!mutated) {
return params.text
}
Expand Down
154 changes: 154 additions & 0 deletions ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/KtLintTest.kt
View file
@@ -1,12 +1,15 @@
package com.pinterest.ktlint.core

import com.pinterest.ktlint.core.AutoCorrectErrorRule.Companion.STRING_VALUE_AFTER_AUTOCORRECT
import com.pinterest.ktlint.core.DummyRuleWithCustomEditorConfigProperty.Companion.SOME_CUSTOM_RULE_PROPERTY
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
import com.pinterest.ktlint.core.ast.ElementType.REGULAR_STRING_PART
import com.pinterest.ktlint.core.ast.isRoot
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatThrownBy
import org.ec4j.core.model.PropertyType
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafElement
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test

Expand Down Expand Up @@ -145,6 +148,58 @@ class KtLintTest {
"differs from the actual value in the '.editorconfig' file."
)
}

@Test
fun `Given a rule returning an errors which can and can not be autocorrected than that state of the error can be retrieved in the callback`() {
val code =
"""
val foo = "${AutoCorrectErrorRule.STRING_VALUE_NOT_TO_BE_CORRECTED}"
val bar = "${AutoCorrectErrorRule.STRING_VALUE_TO_BE_AUTOCORRECTED}"
""".trimIndent()
val callbacks = mutableListOf<CallbackResult>()
KtLint.lint(
KtLint.ExperimentalParams(
text = code,
ruleSets = listOf(
RuleSet("standard", AutoCorrectErrorRule())
),
userData = emptyMap(),
cb = { e, corrected ->
callbacks.add(
CallbackResult(
line = e.line,
col = e.col,
ruleId = e.ruleId,
detail = e.detail,
canBeAutoCorrected = e.canBeAutoCorrected,
corrected = corrected
)
)
},
script = false,
editorConfigPath = null,
debug = false
)
)
assertThat(callbacks).containsExactly(
CallbackResult(
line = 1,
col = 12,
ruleId = "auto-correct",
detail = AutoCorrectErrorRule.ERROR_MESSAGE_CAN_NOT_BE_AUTOCORRECTED,
canBeAutoCorrected = false,
corrected = false
),
CallbackResult(
line = 2,
col = 12,
ruleId = "auto-correct",
detail = AutoCorrectErrorRule.ERROR_MESSAGE_CAN_BE_AUTOCORRECTED,
canBeAutoCorrected = true,
corrected = false
)
)
}
}

@Nested
Expand Down Expand Up @@ -276,6 +331,64 @@ class KtLintTest {
)
}
}

@Test
fun `Given a rule returning an errors which can and can not be autocorrected than that state of the error can be retrieved in the callback`() {
val code =
"""
val foo = "${AutoCorrectErrorRule.STRING_VALUE_NOT_TO_BE_CORRECTED}"
val bar = "${AutoCorrectErrorRule.STRING_VALUE_TO_BE_AUTOCORRECTED}"
""".trimIndent()
val formattedCode =
"""
val foo = "${AutoCorrectErrorRule.STRING_VALUE_NOT_TO_BE_CORRECTED}"
val bar = "$STRING_VALUE_AFTER_AUTOCORRECT"
""".trimIndent()
val callbacks = mutableListOf<CallbackResult>()
val actualFormattedCode = KtLint.format(
KtLint.ExperimentalParams(
text = code,
ruleSets = listOf(
RuleSet("standard", AutoCorrectErrorRule())
),
userData = emptyMap(),
cb = { e, corrected ->
callbacks.add(
CallbackResult(
line = e.line,
col = e.col,
ruleId = e.ruleId,
detail = e.detail,
canBeAutoCorrected = e.canBeAutoCorrected,
corrected = corrected
)
)
},
script = false,
editorConfigPath = null,
debug = false
)
)
assertThat(actualFormattedCode).isEqualTo(formattedCode)
assertThat(callbacks).containsExactly(
CallbackResult(
line = 1,
col = 12,
ruleId = "auto-correct",
detail = AutoCorrectErrorRule.ERROR_MESSAGE_CAN_NOT_BE_AUTOCORRECTED,
canBeAutoCorrected = false,
corrected = false
),
CallbackResult(
line = 2,
col = 12,
ruleId = "auto-correct",
detail = AutoCorrectErrorRule.ERROR_MESSAGE_CAN_BE_AUTOCORRECTED,
canBeAutoCorrected = true,
corrected = true
)
)
}
}

@Test
Expand Down Expand Up @@ -404,7 +517,48 @@ private open class DummyRule(
}
}

/**
* A dummy rule for testing
*/
private class AutoCorrectErrorRule : Rule("auto-correct") {
override fun visit(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
if (node.elementType == REGULAR_STRING_PART) {
when (node.text) {
STRING_VALUE_TO_BE_AUTOCORRECTED -> {
emit(node.startOffset, ERROR_MESSAGE_CAN_BE_AUTOCORRECTED, true)
if (autoCorrect) {
(node as LeafElement).replaceWithText(STRING_VALUE_AFTER_AUTOCORRECT)
}
}
STRING_VALUE_NOT_TO_BE_CORRECTED ->
emit(node.startOffset, ERROR_MESSAGE_CAN_NOT_BE_AUTOCORRECTED, false)
}
}
}

companion object {
const val STRING_VALUE_TO_BE_AUTOCORRECTED = "string-value-to-be-autocorrected"
const val STRING_VALUE_NOT_TO_BE_CORRECTED = "string-value-not-to-be-corrected"
const val STRING_VALUE_AFTER_AUTOCORRECT = "string-value-after-autocorrect"
const val ERROR_MESSAGE_CAN_BE_AUTOCORRECTED = "This string value is not allowed and can be autocorrected"
const val ERROR_MESSAGE_CAN_NOT_BE_AUTOCORRECTED = "This string value is not allowed but can not be autocorrected"
}
}

private fun getResourceAsText(path: String) =
(ClassLoader.getSystemClassLoader().getResourceAsStream(path) ?: throw RuntimeException("$path not found"))
.bufferedReader()
.readText()

private data class CallbackResult(
val line: Int,
val col: Int,
val ruleId: String,
val detail: String,
val canBeAutoCorrected: Boolean,
val corrected: Boolean
)

0 comments on commit 0949748

Please sign in to comment.