Skip to content

Commit

Permalink
Merge pull request #1862 from pinterest/1456-miscellaneous-todo
Browse files Browse the repository at this point in the history
Cleanup miscellaneous todo's

Closes #1456
  • Loading branch information
paul-dingemans committed Mar 16, 2023
2 parents 601b84c + 77edb2a commit fac12ee
Show file tree
Hide file tree
Showing 39 changed files with 554 additions and 525 deletions.
10 changes: 4 additions & 6 deletions RELEASE_TESTING.MD
Original file line number Diff line number Diff line change
Expand Up @@ -80,22 +80,20 @@ Pre-requisites:

Formatting projects in which ktlint is not used may result in a huge amount of fixes. The main focus of this test is to see what the effects are when upgrading ktlint in a project already formatted with latest released ktlint version.

TODO: After release 0.48, update procedure below as the parameter "--experimental" does not need to be specified anymore when it is stored correctly in the `.editorconfig` file(s).

1. Format the sample projects with the *latest released* ktlint version:
```shell
ktlint-prev -F --experimental --relative # Do not call this command via the "./exec-in-each-project.sh" script.
ktlint-prev -F --relative # Do not call this command via the "./exec-in-each-project.sh" script.
```
Note: Ignore all output as this is the old version!
2. Commit changes:
```shell
./exec-in-each-project.sh "git add --all && git commit -m \"Format with ktlint (xx.yy.zz) -F --experimental\""
./exec-in-each-project.sh "git add --all && git commit -m \"Format with ktlint (xx.yy.zz) -F\""
```
Repeat step 1 and 2 until no files are changed anymore.
3. Create a new baseline file with the *latest released* ktlint version to ignore all lint violations that could not be autocorrected using the latest ktlint version:
```shell
rm baseline.xml
ktlint-prev --experimental --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want one combined baseline.xml file for all projects.
ktlint-prev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want one combined baseline.xml file for all projects.
```
4. Check that besides the `baseline.xml` no files are changed (in step 1 and 2 all violations which could be autocorrected have already been committed). Remaining violations which could not be autocorrected are saved in the `baseline.xml` which is stored outside the project directories.
```shell
Expand Down Expand Up @@ -152,7 +150,7 @@ TODO: After release 0.48, update procedure below as the parameter "--experimenta
```
10. Rerun lint with *latest development* version:
```shell
ktlint-dev --experimental --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
ktlint-dev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
```
No violations, except error that can not be autocorrected, should be expected.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ class KtLintRuleEngineTest {
)

val failedRules = errors.map { it.ruleId }
check(failedRules.contains(NoVarRule.NO_VAR_RULE_ID.value))
check(failedRules.contains(NoVarRule.NO_VAR_RULE_ID))
}

private class NoVarRule :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ internal class KtlintCommandLine {
val ktlintCliError = KtlintCliError(
line = lintError.line,
col = lintError.col,
ruleId = lintError.ruleId,
ruleId = lintError.ruleId.value,
detail = lintError
.detail
.applyIf(corrected) { "$this (cannot be auto-corrected)" },
Expand Down Expand Up @@ -499,7 +499,7 @@ internal class KtlintCommandLine {
val ktlintCliError = KtlintCliError(
line = lintError.line,
col = lintError.col,
ruleId = lintError.ruleId,
ruleId = lintError.ruleId.value,
detail = lintError.detail,
status = if (lintError.canBeAutoCorrected) {
LINT_CAN_BE_AUTOCORRECTED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package com.pinterest.ktlint.cli.api
import com.pinterest.ktlint.cli.CommandLineTestRunner
import com.pinterest.ktlint.cli.containsLineMatching
import com.pinterest.ktlint.cli.doesNotContainLineMatching
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.SoftAssertions
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import org.jetbrains.kotlin.com.intellij.lang.ASTNode
* The [Rule] contains the life cycle hooks which are needed for the KtLint rule engine.
*
* Implementation **doesn't** have to be thread-safe or stateless, provided that [RuleSetProviderV2] creates a new
* instance of the [Rule] on each call to [RuleProvider.createNewRuleInstance]. The KtLint engine never re-uses a [Rule]
* instance of the [Rule] on each call to [RuleProvider.getRuleInstance]. The KtLint engine never re-uses a [Rule]
* instance once is has been used for traversal of the AST of a file.
*/
@Deprecated("Deprecated since ktlint 0.49.0. Custom rulesets have to be migrated to RuleSetProviderV3. See changelog 0.49.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package com.pinterest.ktlint.core

/**
* Provides a [Rule] instance. Important: to ensure that a [Rule] can keep internal state and that processing of files
* is thread-safe, a *new* instance should be provided on each call of [createNewRuleInstance]. A custom [RuleProvider]
* is thread-safe, a *new* instance should be provided on each call of [getRuleInstance]. A custom [RuleProvider]
* does not need to take care of clearing internal state as KtLint calls this method any time the [Rule] instance has
* been used for processing a file and as of that might have an internal state set.
*/
Expand All @@ -13,5 +13,5 @@ public class RuleProvider(
*/
private val provider: () -> Rule,
) {
public fun createNewRuleInstance(): Rule = provider()
public fun getRuleInstance(): Rule = provider()
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ public fun ASTNode.parent(
return null
}

// TODO in ktlint 0.49 deprecate and replace with "ASTNode.parent(strict: Boolean = true, p: (ASTNode) -> Boolean): ASTNode?"
@Deprecated(
message = "Marked for removal in KtLint 0.50",
replaceWith = ReplaceWith("parent(strict, p)"),
)
public fun ASTNode.parent(
p: (ASTNode) -> Boolean,
strict: Boolean = true,
Expand All @@ -183,6 +186,20 @@ public fun ASTNode.parent(
return null
}

public fun ASTNode.parent(
strict: Boolean = true,
predicate: (ASTNode) -> Boolean,
): ASTNode? {
var n: ASTNode? = if (strict) this.treeParent else this
while (n != null) {
if (predicate(n)) {
return n
}
n = n.treeParent
}
return null
}

/**
* @param elementType [ElementType].*
*/
Expand All @@ -200,8 +217,12 @@ public fun ASTNode.isPartOf(klass: KClass<out PsiElement>): Boolean {
}

public fun ASTNode.isPartOfCompositeElementOfType(iElementType: IElementType): Boolean =
parent(findCompositeElementOfType(iElementType))?.elementType == iElementType
iElementType == findCompositeParentElementOfType(iElementType)?.elementType

public fun ASTNode.findCompositeParentElementOfType(iElementType: IElementType): ASTNode? =
parent { it.elementType == iElementType || it !is CompositeElement }

@Deprecated("Marked for removal in KtLint 0.50")
public fun findCompositeElementOfType(iElementType: IElementType): (ASTNode) -> Boolean =
{ it.elementType == iElementType || it !is CompositeElement }

Expand All @@ -222,7 +243,7 @@ public fun ASTNode.isLeaf(): Boolean = firstChildNode == null
*/
public fun ASTNode.isCodeLeaf(): Boolean = isLeaf() && !isWhiteSpace() && !isPartOfComment()

public fun ASTNode.isPartOfComment(): Boolean = parent({ it.psi is PsiComment }, strict = false) != null
public fun ASTNode.isPartOfComment(): Boolean = parent(strict = false) { it.psi is PsiComment } != null

public fun ASTNode.children(): Sequence<ASTNode> = generateSequence(firstChildNode) { node -> node.treeNext }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@ public value class RuleId(public val value: String) {
get() = RuleSetId(value.substringBefore(DELIMITER, ""))

public companion object {
private const val STANDARD_RULE_SET_ID = "standard"
private const val DELIMITER = ":"

// TODO: Remove in future version when backward compatibility of rule id references can be dropped.
// TODO: Remove in 1.0 version when backward compatibility of rule id references can be dropped.
public fun prefixWithStandardRuleSetIdWhenMissing(id: String): String =
if (id.contains(DELIMITER)) {
id
} else {
"$STANDARD_RULE_SET_ID$DELIMITER$id"
"${RuleSetId.STANDARD.value}$DELIMITER$id"
}
}
}
Expand All @@ -33,14 +32,22 @@ public value class RuleSetId(public val value: String) {
init {
IdNamingPolicy.enforceRuleSetIdNaming(value)
}

public companion object {
/**
* The `standard` rule set is reserved for rules published by the KtLint project only. Custom rules should be provided via a rule
* set using a custom id so that in case of problems, it can be more clearly communicated to users which project is responsible for
* maintenance of the rule (set).
*/
public val STANDARD: RuleSetId = RuleSetId("standard")
}
}

/**
* The [Rule] contains the life cycle hooks which are called by the KtLint rule engine to execute the rule.
*
* The implementation of a [Rule] **doesn't** have to be thread-safe or stateless provided that the [RuleProvider] creates a new instance of
* [Rule] on each call to [RuleProvider.createNewRuleInstance]. The KtLint Rule Engine never re-uses a [Rule] instance once is has been used
* for traversal of the AST of a file.
* [Rule] on each call to [RuleProvider.provider].
*
* When wrapping a rule from the ktlint project and modifying its behavior, please change the [ruleId] and [about] fields, so that it is
* clear to users whenever they used the original rule provided by KtLint versus a modified version which is not maintained by the KtLint
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,60 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfigProper
import org.ec4j.core.model.PropertyType

/**
* Provides a [Rule] instance. Important: to ensure that a [Rule] can keep internal state and that processing of files
* is thread-safe, a *new* instance should be provided on each call of [createNewRuleInstance]. A custom [RuleProvider]
* does not need to take care of clearing internal state as KtLint calls this method any time the [Rule] instance has
* been used for processing a file and as of that might have an internal state set.
* Provides a [Rule] instance. Important: to ensure that a [Rule] can keep internal state and that processing of files is thread-safe, a
* *new* instance should be provided on each call of the [provider] function.
*/
public class RuleProvider(
public class RuleProvider private constructor(
/**
* Lambda which creates a new instance of the rule.
*/
private val provider: () -> Rule,

/**
* The rule id of the [Rule] created by the provider.
*/
public val ruleId: RuleId,

/**
* Flag whether the [Rule] created by the provider has to run as late as possible.
*/
public val runAsLateAsPossible: Boolean,

/**
* The list of rules which have to run before the [Rule] created by the provider can be run.
*/
public val runAfterRules: List<Rule.VisitorModifier.RunAfterRule>,
) {
public fun createNewRuleInstance(): Rule = provider()
/**
* Creates a new [Rule] instance.
*/
public fun createNewRuleInstance(): Rule {
return provider()
}

/**
* Lambda which creates a new instance of the [Rule]. Important: to ensure that a [Rule] can keep internal state and that processing of
* files is thread-safe, a *new* instance should be provided on each call of the [provider] function.
*/
public companion object {
// Note that the KDOC is placed on the companion object to make it actually visually when the RuleProvider identifier is being
// hovered in IntelliJ IDEA
public operator fun invoke(provider: () -> Rule): RuleProvider =
provider()
.let { rule ->
RuleProvider(
provider = provider,
ruleId = rule.ruleId,
runAsLateAsPossible = rule
.visitorModifiers
.filterIsInstance<Rule.VisitorModifier.RunAsLateAsPossible>()
.any(),
runAfterRules = rule
.visitorModifiers
.filterIsInstance<Rule.VisitorModifier.RunAfterRule>(),
)
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import com.pinterest.ktlint.rule.engine.internal.EditorConfigLoader
import com.pinterest.ktlint.rule.engine.internal.EditorConfigLoaderEc4j
import com.pinterest.ktlint.rule.engine.internal.RuleExecutionContext
import com.pinterest.ktlint.rule.engine.internal.RuleExecutionContext.Companion.createRuleExecutionContext
import com.pinterest.ktlint.rule.engine.internal.RuleRunner
import com.pinterest.ktlint.rule.engine.internal.ThreadSafeEditorConfigCache.Companion.THREAD_SAFE_EDITOR_CONFIG_CACHE
import com.pinterest.ktlint.rule.engine.internal.VisitorProvider
import mu.KotlinLogging
Expand Down Expand Up @@ -98,12 +97,12 @@ public class KtLintRuleEngine(
val ruleExecutionContext = createRuleExecutionContext(this, code)
val errors = mutableListOf<LintError>()

VisitorProvider(ruleExecutionContext.ruleRunners)
VisitorProvider(ruleExecutionContext.ruleProviders)
.visitor()
.invoke { rule, fqRuleId ->
ruleExecutionContext.executeRule(rule, fqRuleId, false) { offset, errorMessage, canBeAutoCorrected ->
.invoke { rule ->
ruleExecutionContext.executeRule(rule, false) { offset, errorMessage, canBeAutoCorrected ->
val (line, col) = ruleExecutionContext.positionInTextLocator(offset)
errors.add(LintError(line, col, fqRuleId, errorMessage, canBeAutoCorrected))
errors.add(LintError(line, col, rule.ruleId, errorMessage, canBeAutoCorrected))
}
}

Expand Down Expand Up @@ -133,11 +132,11 @@ public class KtLintRuleEngine(
var tripped = false
var mutated = false
val errors = mutableSetOf<Pair<LintError, Boolean>>()
val visitorProvider = VisitorProvider(ruleExecutionContext.ruleRunners)
val visitorProvider = VisitorProvider(ruleExecutionContext.ruleProviders)
visitorProvider
.visitor()
.invoke { rule, fqRuleId ->
ruleExecutionContext.executeRule(rule, fqRuleId, true) { offset, errorMessage, canBeAutoCorrected ->
.invoke { rule ->
ruleExecutionContext.executeRule(rule, true) { offset, errorMessage, canBeAutoCorrected ->
tripped = true
if (canBeAutoCorrected) {
mutated = true
Expand All @@ -150,7 +149,7 @@ public class KtLintRuleEngine(
val (line, col) = ruleExecutionContext.positionInTextLocator(offset)
errors.add(
Pair(
LintError(line, col, fqRuleId, errorMessage, canBeAutoCorrected),
LintError(line, col, rule.ruleId, errorMessage, canBeAutoCorrected),
// It is assumed that a rule that emits that an error can be autocorrected, also
// does correct the error.
canBeAutoCorrected,
Expand All @@ -161,16 +160,15 @@ public class KtLintRuleEngine(
if (tripped) {
visitorProvider
.visitor()
.invoke { rule, fqRuleId ->
.invoke { rule ->
ruleExecutionContext.executeRule(
rule,
fqRuleId,
false,
) { offset, errorMessage, canBeAutoCorrected ->
val (line, col) = ruleExecutionContext.positionInTextLocator(offset)
errors.add(
Pair(
LintError(line, col, fqRuleId, errorMessage, canBeAutoCorrected),
LintError(line, col, rule.ruleId, 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 Down Expand Up @@ -228,11 +226,9 @@ public class KtLintRuleEngine(
?: CODE_STYLE_PROPERTY.defaultValue
val rules =
ruleProviders
.map { RuleRunner(it) }
.map { it.createNewRuleInstance() }
.distinctBy { it.ruleId }
.toSet()
.map { it.getRule() }
.toSet()
return EditorConfigGenerator(fileSystem, editorConfigLoaderEc4j)
.generateEditorconfig(
rules,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
package com.pinterest.ktlint.rule.engine.api

import com.pinterest.ktlint.rule.engine.core.api.RuleId

/**
* Lint error found by the [KtLintRuleEngine].
*
* [line]: line number (one-based)
* [col]: column number (one-based)
* [ruleId]: rule id (prepended with "&lt;ruleSetId&gt;:" in case of non-standard ruleset)
* [ruleId]: rule id
* [detail]: error message
* [canBeAutoCorrected]: flag indicating whether the error can be corrected by the rule if "format" is run
*/
public data class LintError(
val line: Int,
val col: Int,
val ruleId: String,
val ruleId: RuleId,
val detail: String,
val canBeAutoCorrected: Boolean,
)

0 comments on commit fac12ee

Please sign in to comment.