Block ktlint-disable directive not working

[JoseAlcerreca]

Version: 0.39.0## Expected Behavior

/* ktlint-disable indent */
should disable the indent rule from that point on.

Observed Behavior

Still getting the ktlint errors that the directive is supposed to disable

Steps to Reproduce

ktlint is used in AndroidX and we want to disable some checks (they're snippets in documentation). For example:
https://android-review.googlesource.com/c/platform/frameworks/support/+/1497420/6/compose/integration-tests/docs-snippets/src/main/java/androidx/compose/ui/docs/tutorial/Tutorial.kt

The ktlint config can be found in
https://cs.android.com/androidx/platform/frameworks/support/+/androidx-master-dev:buildSrc/src/main/kotlin/androidx/build/Ktlint.kt?q=ktlint.kt&ss=androidx

However, code after /* ktlint-disable indent */ is still breaking the build:

/* ktlint-disable indent */
...
            Text("Text", Modifier.constrainAs(text) {
                top.linkTo(button.bottom, margin = 16.dp)
            })

shows

docs-snippets/src/main/java/androidx/compose/ui/docs/layout/Layout.kt:174:13: Missing newline before ")" (indent)

My workaround is to add // disable-ktlint at the top of the file and, for some reason, I also need to add /* ktlint-disable no-unused-imports */, otherwise every import triggers an error. It looks like a different, unrelated problem.

[romtsn]

yea, that's unfortunately not possible at the moment as the indent rule runs all or nothing:

ktlint/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRule.kt

Lines 96 to 99 in b4f0844

	* Current limitations:
	* - "all or nothing" (currently, rule can only be disabled for an entire file)
	*/
	class IndentationRule : Rule("indent"), Rule.Modifier.RestrictToRootLast {

we are planning to change this in the upcoming release and make the rule more granular so you can disable node-specific rules.

the 2nd issue sounds like a bug indeed, but let's see how it behaves after the split.
(btw, you could update your ktlint config as the paren-spacing and import-ordering rules were already fixed)

[JoseAlcerreca]

@liutikas for the ktlint config.

Thanks Roman, I've added the "all" directive to the affected files for now, pointing to this issue. Is there a better issue to point to?
https://android-review.googlesource.com/c/platform/frameworks/support/+/1497420/7/compose/integration-tests/docs-snippets/src/main/java/androidx/compose/integration/docs/tutorial/Tutorial.kt#1

I don't think the file scope for disable directives is documented, is it? In any case, putting it on the first line can cause problems with tools checking for copyright notices, so I suggest adding something more flexible :)

[romtsn]

yeah, this issue should be fine to point to. let's hope that you would not need to disable rules for the entire file anymore, but I agree we should think of something more flexible and straightforward ;)

[aablsk]

Hi @romtsn,

I just wanted to add that this feature would be quite valuable for the teams at my company as well. Therefore I wanted to ask, - whether it is already planned to implement this feature or if work is already being done?

• how complex you expect the implementation to be?

Depending on your responses I might be able to invest some time in order to support this feature request.

Also if adding this comment to this existing issue is inappropriate, please let me know.

Thanks in advance! 🙏

[paul-dingemans]

I just wanted to add that this feature would be quite valuable for the teams at my company as well. Therefore I wanted to ask, - whether it is already planned to implement this feature or if work is already being done?

• how complex you expect the implementation to be?

Depending on your responses I might be able to invest some time in order to support this feature request.

The indent-rule is one of the most complex rules in ktlint. A first step would be to split this rule into separate rules regarding line wrapping and indent fixing. For this, we have to await #1230. The next step after that would be cleaning up code and merging the expectedIndex variable into the IndentContext. After that it might be more clear what follow up steps have to be taken.

@aablsk Help is appreciated. It might be an idea to first get a bit acquainted with this rule by fixing other indentation problems which are reported in the issue tracker.

[paul-dingemans]

Fixed by #1547