Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix loading custom rule sets #1752

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
* Add API so that KtLint API consumer is able to process a Kotlin script snippet without having to specify a file path ([#1738](https://github.com/pinterest/ktlint/issues/1738))
* Disable the `standard:filename` rule whenever Ktlint CLI is run with option `--stdin` ([#1742](https://github.com/pinterest/ktlint/issues/1742))
* Fix initialization of the logger when `--log-level` is specified. Throw exception when an invalid value is passed. ([#1749](https://github.com/pinterest/ktlint/issues/1749))
* Fix loading of custom rule set JARs

### Changed

Expand Down
Expand Up @@ -65,13 +65,13 @@ public abstract class RuleSetProviderV2(
require(description == null || description.length <= 400) {
"Length of description should be 400 characters or less"
}
require(license == null || license.length <= 80) {
"Length of license should be 80 characters or less"
require(license == null || license.length <= 120) {
"Length of license url should be 80 characters or less"
}
require(repositoryUrl == null || repositoryUrl.length <= 80) {
require(repositoryUrl == null || repositoryUrl.length <= 120) {
"Length of repository url should be 80 characters or less"
}
require(issueTrackerUrl == null || issueTrackerUrl.length <= 80) {
require(issueTrackerUrl == null || issueTrackerUrl.length <= 120) {
"Length of repository url should be 80 characters or less"
}
}
Expand Down
Expand Up @@ -51,6 +51,6 @@ internal class RuleRunner(private val provider: RuleProvider) {
require(!rule.isUsedForTraversalOfAST()) {
"RunAfterRules can not be removed when RuleRunner has already been used for traversal of the AST"
}
this.runAfterRules - runAfterRules
this.runAfterRules = this.runAfterRules - runAfterRules
}
}
Expand Up @@ -93,8 +93,10 @@ internal class RuleRunnerSorter {
do {
if (newRuleRunnersAdded) {
newRuleRunnersAdded = false
// All rule runners which were (previously) blocked can now be checked again
ruleRunnersIterator =
blockedRuleRunners
.canRunWith(newRuleRunners)
.toSet()
.iterator()
blockedRuleRunners.clear()
Expand Down Expand Up @@ -157,13 +159,6 @@ internal class RuleRunnerSorter {
// already added to the new list of rule which will be loaded before the current rule.
newRuleRunners.add(currentRuleRunner)
newRuleRunnersAdded = true
// All rule runners which were (recursively) blocked because they need to be run after the newly added rule
// runner can now be added to the new list of rule runners as well.
val ruleReferencesToUnblock = blockedRuleRunners.canRunWith(newRuleRunners)
if (ruleReferencesToUnblock.isNotEmpty()) {
newRuleRunners.addAll(ruleReferencesToUnblock)
blockedRuleRunners.removeAll(ruleReferencesToUnblock.toSet())
}
}

BLOCK_UNTIL_RUN_AFTER_RULE_IS_LOADED -> {
Expand Down Expand Up @@ -200,9 +195,9 @@ internal class RuleRunnerSorter {
.sorted()
val prefix =
if (customRuleSetIds.isEmpty()) {
"Found cyclic dependencies between rules that should run after another rule:"
"Found cyclic dependencies between required rules that should run after another rule:"
} else {
"Found cyclic dependencies between rules that should run after another rule. Please contact " +
"Found cyclic dependencies between required rules that should run after another rule. Please contact " +
"the maintainer(s) of the custom rule set(s) [${customRuleSetIds.joinToString()}] before " +
"creating an issue in the KtLint project. Dependencies:"
}
Expand All @@ -217,13 +212,6 @@ internal class RuleRunnerSorter {
return newRuleRunners
}

private fun Set<RuleRunner>.findRuleRunnersBlockedBy(qualifiedRuleId: String): List<RuleRunner> {
return this
.filter { it.runAfterRules.any { it.ruleId == qualifiedRuleId } }
.map { listOf(it) + this.findRuleRunnersBlockedBy(it.qualifiedRuleId) }
.flatten()
}

private fun Set<RuleRunner>.canRunWith(loadedRuleRunners: List<RuleRunner>): List<RuleRunner> =
canRunWithRuleIds(loadedRuleRunners.map { it.qualifiedRuleId })

Expand Down
Expand Up @@ -222,10 +222,11 @@ class RuleRunnerSorterTest {
assertThat(actual).containsExactly(
"$STANDARD:$RULE_B",
"$STANDARD:$RULE_C",
"$STANDARD:$RULE_A",
// Although RULE_D like RULE_C depends on RULE_B it still comes after RULE_A because that rules according to
// the default sort order comes before rule D
"$STANDARD:$RULE_D",
// RULE_D is ordered before RULE_A because rules are evaluated in order of the initial sorting (A, B, C, D). In the first
// iteration of the rules, RULE_A is blocked because rule C is not yet added. RULE_B, RULE_C and RULE_D can be added during the
// first iteration as the rules are not blocked when they are evaluated. In the second iteration, RULE_A can be added as well.
"$STANDARD:$RULE_A",
)
}

Expand Down Expand Up @@ -492,7 +493,7 @@ class RuleRunnerSorterTest {
)
}.withMessage(
"""
Found cyclic dependencies between rules that should run after another rule:
Found cyclic dependencies between required rules that should run after another rule:
- Rule with id '$STANDARD:$RULE_A' should run after rule(s) with id '$STANDARD:$RULE_B'
- Rule with id '$STANDARD:$RULE_B' should run after rule(s) with id '$EXPERIMENTAL:$RULE_C'
- Rule with id '$EXPERIMENTAL:$RULE_C' should run after rule(s) with id '$STANDARD:$RULE_A'
Expand All @@ -509,29 +510,29 @@ class RuleRunnerSorterTest {
object : R(
id = RULE_A,
visitorModifiers = setOf(
VisitorModifier.RunAfterRule("$EXPERIMENTAL:$RULE_B"),
VisitorModifier.RunAfterRule("$CUSTOM_RULE_SET_A:$RULE_B"),
VisitorModifier.RunAfterRule(RULE_B),
),
) {},
object : R(
id = RULE_B,
visitorModifiers = setOf(
VisitorModifier.RunAfterRule(RULE_C),
VisitorModifier.RunAfterRule("$EXPERIMENTAL:$RULE_C"),
VisitorModifier.RunAfterRule("$CUSTOM_RULE_SET_A:$RULE_C"),
),
) {},
object : R(
id = "$EXPERIMENTAL:$RULE_C",
id = "$CUSTOM_RULE_SET_A:$RULE_C",
visitorModifier = VisitorModifier.RunAfterRule(RULE_A),
) {},
),
)
}.withMessage(
"""
Found cyclic dependencies between rules that should run after another rule:
- Rule with id '$STANDARD:$RULE_A' should run after rule(s) with id '$EXPERIMENTAL:$RULE_B, $STANDARD:$RULE_B'
- Rule with id '$STANDARD:$RULE_B' should run after rule(s) with id '$STANDARD:$RULE_C, $EXPERIMENTAL:$RULE_C'
- Rule with id '$EXPERIMENTAL:$RULE_C' should run after rule(s) with id '$STANDARD:$RULE_A'
Found cyclic dependencies between required rules that should run after another rule. Please contact the maintainer(s) of the custom rule set(s) [custom-rule-set-a] before creating an issue in the KtLint project. Dependencies:
- Rule with id '$STANDARD:$RULE_A' should run after rule(s) with id '$STANDARD:$RULE_B'
- Rule with id '$STANDARD:$RULE_B' should run after rule(s) with id '$CUSTOM_RULE_SET_A:$RULE_C'
- Rule with id '$CUSTOM_RULE_SET_A:$RULE_C' should run after rule(s) with id '$STANDARD:$RULE_A'
""".trimIndent(),
)
}
Expand Down Expand Up @@ -561,7 +562,7 @@ class RuleRunnerSorterTest {
)
}.withMessage(
"""
Found cyclic dependencies between rules that should run after another rule. Please contact the maintainer(s) of the custom rule set(s) [$CUSTOM_RULE_SET_A, $CUSTOM_RULE_SET_B] before creating an issue in the KtLint project. Dependencies:
Found cyclic dependencies between required rules that should run after another rule. Please contact the maintainer(s) of the custom rule set(s) [$CUSTOM_RULE_SET_A, $CUSTOM_RULE_SET_B] before creating an issue in the KtLint project. Dependencies:
- Rule with id '$STANDARD:$RULE_C' should run after rule(s) with id '$CUSTOM_RULE_SET_B:$RULE_B'
- Rule with id '$CUSTOM_RULE_SET_A:$RULE_A' should run after rule(s) with id '$STANDARD:$RULE_C'
- Rule with id '$CUSTOM_RULE_SET_B:$RULE_B' should run after rule(s) with id '$CUSTOM_RULE_SET_A:$RULE_A'
Expand Down Expand Up @@ -597,10 +598,10 @@ class RuleRunnerSorterTest {
)
}.withMessage(
"""
Found cyclic dependencies between rules that should run after another rule. Please contact the maintainer(s) of the custom rule set(s) [$CUSTOM_RULE_SET_A, $CUSTOM_RULE_SET_B] before creating an issue in the KtLint project. Dependencies:
- Rule with id '$STANDARD:$RULE_C' should run after rule(s) with id '$CUSTOM_RULE_SET_B:$RULE_B, $STANDARD:$RULE_B'
Found cyclic dependencies between required rules that should run after another rule. Please contact the maintainer(s) of the custom rule set(s) [$CUSTOM_RULE_SET_A, $CUSTOM_RULE_SET_B] before creating an issue in the KtLint project. Dependencies:
- Rule with id '$STANDARD:$RULE_C' should run after rule(s) with id '$CUSTOM_RULE_SET_B:$RULE_B'
- Rule with id '$CUSTOM_RULE_SET_A:$RULE_A' should run after rule(s) with id '$STANDARD:$RULE_C'
- Rule with id '$CUSTOM_RULE_SET_B:$RULE_B' should run after rule(s) with id '$STANDARD:$RULE_A, $CUSTOM_RULE_SET_A:$RULE_A'
- Rule with id '$CUSTOM_RULE_SET_B:$RULE_B' should run after rule(s) with id '$CUSTOM_RULE_SET_A:$RULE_A'
""".trimIndent(),
)
}
Expand Down
Expand Up @@ -29,8 +29,15 @@ internal class GenerateEditorConfigSubCommand : Runnable {
override fun run() {
commandSpec.commandLine().printCommandLineHelpOrVersionUsage()

val ruleProviders =
ktlintCommand
.ruleProvidersByRuleSetId()
.values
.flatten()
.toSet()

val ktLintRuleEngine = KtLintRuleEngine(
ruleProviders = ktlintCommand.ruleProviders(),
ruleProviders = ruleProviders,
editorConfigOverride = EditorConfigOverride.from(CODE_STYLE_PROPERTY to codeStyle()),
isInvokedFromCli = true,
)
Expand Down
Expand Up @@ -6,6 +6,7 @@ import com.pinterest.ktlint.core.KtLintRuleEngine
import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.core.Reporter
import com.pinterest.ktlint.core.ReporterProvider
import com.pinterest.ktlint.core.RuleProvider
import com.pinterest.ktlint.core.api.Baseline.Status.INVALID
import com.pinterest.ktlint.core.api.Baseline.Status.NOT_FOUND
import com.pinterest.ktlint.core.api.EditorConfigDefaults
Expand Down Expand Up @@ -251,20 +252,6 @@ internal class KtlintCommandLine {
?.let { path -> Paths.get(path) },
)

private val editorConfigOverride: EditorConfigOverride
get() =
EditorConfigOverride
.EMPTY_EDITOR_CONFIG_OVERRIDE
.applyIf(experimental) {
plus(createRuleSetExecutionEditorConfigProperty("experimental:all") to RuleExecution.enabled)
}.applyIf(disabledRules.isNotBlank()) {
plus(*disabledRulesEditorConfigOverrides())
}.applyIf(android) {
plus(CODE_STYLE_PROPERTY to CodeStyleValue.android)
}.applyIf(stdin) {
plus(createRuleExecutionEditorConfigProperty("standard:filename") to RuleExecution.disabled)
}

private fun disabledRulesEditorConfigOverrides() =
disabledRules
.split(",")
Expand All @@ -283,6 +270,37 @@ internal class KtlintCommandLine {
exitKtLintProcess(1)
}

val ruleProvidersByRuleSetId = ruleProvidersByRuleSetId()
val customRuleSetIds =
ruleProvidersByRuleSetId
.filterKeys {
// Exclude the standard and experimental rule sets from Ktlint itself
it != "standard" && it != "experimental"
}.map { it.key }

val editorConfigOverride = EditorConfigOverride
.EMPTY_EDITOR_CONFIG_OVERRIDE
.applyIf(experimental) {
logger.debug { "Add editor config override to allow the experimental rule set" }
plus(createRuleSetExecutionEditorConfigProperty("experimental:all") to RuleExecution.enabled)
}.applyIf(disabledRules.isNotBlank()) {
logger.debug { "Add editor config override to disable rules: '$disabledRules'" }
plus(*disabledRulesEditorConfigOverrides())
}.applyIf(android) {
logger.debug { "Add editor config override to set code style to 'android'" }
plus(CODE_STYLE_PROPERTY to CodeStyleValue.android)
}.applyIf(stdin) {
logger.debug { "Add editor config override to disable 'filename' rule which can not be used in combination with reading from <stdin>" }
plus(createRuleExecutionEditorConfigProperty("standard:filename") to RuleExecution.disabled)
}.applyIf(customRuleSetIds.isNotEmpty()) {
logger.debug { "Add editor config override to enable rule set(s) '$customRuleSetIds' from custom rule set JAR('s): '$rulesetJarPaths'" }
val ruleSetExecutionEditorConfigProperties =
customRuleSetIds
.map { createRuleSetExecutionEditorConfigProperty("$it:all") to RuleExecution.enabled }
.toTypedArray()
plus(*ruleSetExecutionEditorConfigProperties)
}

assertStdinAndPatternsFromStdinOptionsMutuallyExclusive()

val stdinPatterns: Set<String> = readPatternsFromStdin()
Expand All @@ -299,8 +317,14 @@ internal class KtlintCommandLine {

var reporter = loadReporter()

val ruleProviders =
ruleProvidersByRuleSetId
.values
.flatten()
.toSet()

val ktLintRuleEngine = KtLintRuleEngine(
ruleProviders = ruleProviders(),
ruleProviders = ruleProviders,
editorConfigDefaults = editorConfigDefaults,
editorConfigOverride = editorConfigOverride,
isInvokedFromCli = true,
Expand Down Expand Up @@ -347,11 +371,13 @@ internal class KtlintCommandLine {
}
}

internal fun ruleProviders() =
// Do not convert to "val" as the function depends on PicoCli options which are not fully instantiated until the "run" method is started
internal fun ruleProvidersByRuleSetId(): Map<String, Set<RuleProvider>> =
rulesetJarPaths
.toFilesURIList()
.loadRuleProviders(debug)
.loadRuleProvidersByRuleSetId(debug)

// Do not convert to "val" as the function depends on PicoCli options which are not fully instantiated until the "run" method is started
private fun List<String>.toFilesURIList() =
map {
val jarFile = File(it.expandTildeToFullPath())
Expand All @@ -362,6 +388,7 @@ internal class KtlintCommandLine {
jarFile.toURI().toURL()
}

// Do not convert to "val" as the function depends on PicoCli options which are not fully instantiated until the "run" method is started
internal fun configureLogger() {
logger = KotlinLogging
.logger {}
Expand Down