java.lang.IllegalStateException: Skipping rule(s) which are depending on a rule which is not loaded

[jeremymailen]

Observed Behavior

If you disable a rule such as:

[*.{kt,kts}]
ktlint_standard_multiline-expression-wrapping = disabled

It will cause execution to fail due to dependent rules requiring it to be loaded.

lint worker execution error
java.lang.IllegalStateException: Skipping rule(s) which are depending on a rule which is not loaded. Please check if you need to add additional rule sets before creating an issue.
  - Rule with id 'RuleId(value=standard:string-template-indent)' requires rule with id 'RuleId(value=standard:multiline-expression-wrapping)' to be loaded
        at com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter.filter(RunAfterRuleFilter.kt:77)
        at com.pinterest.ktlint.rule.engine.internal.rulefilter.RuleFilterKt.applyRuleFilters(RuleFilter.kt:15)
        at com.pinterest.ktlint.rule.engine.internal.RuleExecutionContext$Companion.createRuleExecutionContext$ktlint_rule_engine(RuleExecutionContext.kt:192)

Expected Behavior

Disabling a rule should only prevent it from being evaluated, it shouldn't impact the executability of other rules which may share some behavior or logic.

Your Environment

• Version of ktlint used: 1.0.0
• Name and version (or code for custom task) of integration used: kotlinter-gradle 4.0.0

[paul-dingemans]

The idea is no repeat logic in multiple rules as those can get out of sync which can lead to conflicts between rules. Also, sharing of logic between rules by using additional classes goes against the ktlint architecture.

Restrictions like your example are only created when a rule (A) can only be executed if it is guarranteed that another rule (B) has been formatting a node beforehand. In this way rule B can ignore harmful situations which are already resolved by rule (A). If rule B could not assume this, it has to repeat logic from rule A to be able to format the code in a reasonable way.

Ktlint allows to define two different kind of relations between rules. The most strict relation is that a specific rule A must run before rule B (and as of that needs to be loaded and must be enabled) like in your example. The other relation states that rule A must run before rule B whenever rule A is enabled.

By throwing the exception, you user is forced to configure ktlint in a consistent way. Silently ignoring rule standard:string-template-indent because its entry conditions (rule standard:multiline-expression-wrapping is loaded) are not met will be very confusing in the user configured ktlint as follows:

[*.{kt,kts}]
ktlint_standard_multiline-expression-wrapping = disabled
ktlint_standard_string-template-indent = enabled

[jeremymailen]

Well, practically speaking I found it impossible to configure ktlint in a way that would be workable for my organization. I'm not sure we'll ever be able to upgrade to ktlint 1.0, which will be a bit of an odd life experience for me given that separately I also maintain a gradle plugin for ktlint.

Here's the problem, and the example you show there is a great example, a rule like multiline-expression-wrapping is quite opinionated and would ripple through a codebase that had been previously consistent with ktlint. It's a bit jarring and I doubt every organization would find a rule like that desirable. On the other hand string-template-indent seems very sensible and desirable and I want that rule. I can't think of a functional reason why string-template-indent would also require multiline-expression-wrapping?

[paul-dingemans]

Have you set ktlint_code_style? The default code style has been changed from intellij_idea in 0.x to ktlint_official in 1.0.

The string-template-indent is as opiniated as multiline-expression-wrapping as it does not comply with the default IntelliJ IDEA formatter. Its output is accepted by the default formatter, but it is never suggested by IDEA.

The multiline-expression-wrapping is required for string-template-indent to handle code like:

val foo = """
    some text
   """.trimIndent()

The string-template-indent has no idea how to calculate the identation of a multiline raw string for which the opening quotes are not aligned with the closing quotes.

[peter-evans]

I ran into this same issue, and I also came across another similar rule dependency like this which makes even less sense to me.

  - Rule with id 'RuleId(value=standard:if-else-wrapping)' requires rule with id 'RuleId(value=standard:discouraged-comment-location)' to be loaded

Why should comment locations have anything to do with actual code blocks. 😕 I only want to disable the discouraged-comment-location rule so we can put comments where we like. Forcing us to disable other reasonable code rules to achieve it doesn't feel great.

[paul-dingemans]

Why should comment locations have anything to do with actual code blocks. 😕 I only want to disable the discouraged-comment-location rule so we can put comments where we like. Forcing us to disable other reasonable code rules to achieve it doesn't feel great.

I totally get this. Comments (and especially end of line comments) are always a lot of work to get right. Every now and then I see that even IntelliJ IDEA gets confused in refactoring of code with comments. I do not always have time and energy to take comments into account in every rule. Comments can almost literally be between any two tokens in the code. On a lot of places this totally makes no sense and probably no one will place comments at certain locations. But to make a rule resilient against comments you have to take this into account. For example:

if (true) {
    doTrue()
} // comment
else {
    doFalse()
}

Does the comment in sample above apply to the block preceding or following it.

[peter-evans]

I appreciate it's a very difficult problem and compromises will need to be made somewhere. Thanks for the explanation.

[vanniktech]

I can only 100% agree what Peter wrote. This really feels like a step backwards and not like something I'd consider 1.x

[paul-dingemans]

discouraged-comment-location is refactored and no rule depends on it anymore. Some logic is moved inside existing rules. 4 new rules with a smaller scope are introduced to disallow comments in for example a value argument (list). The smaller rules at least make it more understandable why there is a relation between rules. For more context, see #2371.

[bcmedeiros]

In general, I agree with the points raised here, I had to disable (at least at first) quite a few rules to be able to migrate to 1.x without many changes.

Thanks for the adjustments around discouraged-comment-location, doing

ktlint_standard_value-parameter-comment = disabled
ktlint_standard_value-argument-comment = disabled

is much better than

ktlint_standard_value-discouraged-comment-location = disabled
ktlint_standard_value-if-else-wrapping = disabled

As general feedback, multiline-expression-wrapping is by far the most controversial rule I needed to disable, followed closely by function-signature. I'll try deep diving on those and propose something that would allow me to re-enable them, but no ideas on how to approach that so far.

[paul-dingemans]

@jeremymailen wrote:

Here's the problem, and the example you show there is a great example, a rule like multiline-expression-wrapping is quite opinionated and would ripple through a codebase that had been previously consistent with ktlint. It's a bit jarring and I doubt every organization would find a rule like that desirable. On the other hand string-template-indent seems very sensible and desirable and I want that rule. I can't think of a functional reason why string-template-indent would also require multiline-expression-wrapping?

On this I replied with:

The multiline-expression-wrapping is required for string-template-indent to handle code like:

val foo = """
    some text
   """.trimIndent()

The string-template-indent has no idea how to calculate the identation of a multiline raw string for which the opening quotes are not aligned with the closing quotes.

The dependency on the multiline-expression-wrapping ensures that the opening """ are wrapped to a newline. I now see that it would be more convenient to let the string-template-indent do the wrapping of the opening """ which would mean that the rule dependency can be removed. Wrapping the opening """ functionally would fit in the string-template-indent rule as well.

@jeremymailen Now the question is whether you meant that wrapping of the opening """ is something which is not acceptable to your organization or that you only meant the generic multiline-expression-wrapping rule?

[jeremymailen]

That sounds great. I only meant that adopting a different rule that affected all or many lines of code might become a sticking point. From a user point of view my opinion is that string template indent would ideally be independent of all other rules except the indent size. I totally admit this comes with a lot of implementation challenges and a rule needing some context input as to what formatting it's taking part in so that it's cooperative.

[paul-dingemans]

As far as I know, no further action is required at this moment. It the issue is not yet completely resolved, please add new information.