ktlint: ktlint 1.1.0 doesn't respect disabled discouraged-comment-location rule in .editorconfig

Looks like a new rule was added for this - value-argument-comment

Unfortunately disabling it results in:

Rule with id ‘RuleId(value=standard:argument-list-wrapping)’ requires rule with id ‘RuleId(value=standard:value-argument-comment)’ to be loaded

At this rate I’ll have most ktlint rules disabled by 2025 😞

@Spikhalskiy I see, that you failed to understand. You referring again to “technically from a language perspective” and this is exactly what is wrong - technical limitations should not be an argument when we are taking care about readability. Readability is for human and if technical limitations hinder human needs, then technical limitations should be changed, not the human needs. The ultimate goal of linter is to make it easier for human to work with the code, not for computer or compiler. And now instead of moving to this goal linter becoming a tool with which I have to fight (of suppress) to have readable code.

@paul-dingemans Explanation why these rules are introduced seems reasonable, but it really sad that technical limitation and incorrect behavior of some tools leads to decreasing of code readability. I think linter is suppose to make code more comprehensible and not vice versa.

I think the main problem with this chain disabling from the user perspective is the following thinking process:

1. We want to have comments for the arguments after the commas. Which is, indeed, strictly speaking, grammatically incorrect. Such comments do belong to the next argument from the parser’s perspective.
2. We disable Rule 1 for the whole project hoping to just leave such comments be.
3. But that forces disabling a higher-level Rule 2 for the whole project.
4. Ok, it’s understandable that Rule 2 can’t work correctly with invocations with such comments without applying Rule 1.
5. But what about all the places that are already compliant with Rule 1 and we still want Rule 2 to be applied to them? Rule 2 doesn’t have to be disabled project-wise, it can just skip occurrences non-compliant with its dependencies. We still generally want Rule 2.

I don’t want to disable argument-list-wrapping for the whole project for all function/method invocations just because some minority of these calls are not compliant with value-argument-comment.

I think this matches with what I suggested:

Next best thing that I can think of, is that the rules that currently have a dependency on any of the comment rules just simply ignore elements containing unexpected comments.

I vehemently agree with this report. At first, I thought I’d just move all the comments in our codebase, but there are hundreds, and they are much more readable on the same line. I really want to disable value-argument/parameter-comment, but I want argument-list-wrapping and function-signature.

Nice things sometimes come at cost. But I do hear your argument that refactoring hundreds of lines of code is not an option for you.

I do not think these things should be coupled.

It is technical complex and most likely it will results in lots of other problems when trying to fix this. You are free to submit a PR which handles comments in value argument lists correctly.

I don’t think anyone on my team has ever moved a parameter using that UI, and I’d rather eat the cost of manually moving a comment in that case.

This is not a relevant argument. Even if ktlint could handle comments correctly, the problem inside IntelliJ IDEA still exists. The IntelliJ IDEA example was simple to illustrate that even in Jetbrains products this kind of code can lead to unpredictable results.

Next best thing that I can think of, is that the rules that currently have a dependency on any of the comment rules just simply ignore elements containing unexpected comments. But this will also lead to unpredictable results as devs will not understand why

        val object = Object(
            someParam1 = 10000, someParam2 = 100, // comment here about why
            sameParam3 = emptyList(),
        )

is not being wrapped, while it does work for:

        val object = Object(
            someParam1 = 10000, someParam2 = 100,
            sameParam3 = emptyList(),
        )

Sorry for not clearly communicating that the discouraged-comment-location has been splitted into separate rules:

• type-argument-comment
• type-parameter-comment
• value-argument-comment
• value-parameter-comment

Reason for this is to provide better insights why certain rules are blocked.

With ktlint 1.0.1, disabling discouraged-comment-location was only possible when also disabling:

• chain-method-continuation
• class-signature
• if-else-wrapping From a user perspective it makes what the relation is between those rules.

With ktlint 1.1.0, disabling value-argument-comment is only possible when also disabling:

• argument-list-wrapping
• chain-method-continuation The chain-method-continuation should not have been blocked. From a functional perspective, and from the unit test perspective, there seems no reason to block it.

Intuitively the relation between argument-list-wrapping and value-argument-comment is not too bad even in case you might not agree with disallowing comments in and after value arguments. It is a common pattern to use. But syntaxtically it is not correct as the comment is place after the semi colon while it refers to the element before the comment. Not only ktlint has troubles to process that correctly but also IntelliJ IDEA does not deal with it correctly as can be seen in screenshots below:

EDIT As documented in a comment below the same happens when the comments are on separate lines as well.

We start with this code:

Open ‘Change signature’ and select ‘sameParam3’:

Now move ‘sameParam3’ one position up and press ‘Refactor’:

The resulting code now looks as follows:

Please note that the comments are no longer related to the original parameter.

EDIT As documented in a comment below the same happens when the comments are on separate lines as well.

It would be nice to see the mention of the new value-argument-comment and value-parameter-comment rules

in docs: https://pinterest.github.io/ktlint/1.1.0/rules/standard/ in release notes: https://github.com/pinterest/ktlint/releases/tag/1.1.0, ideally including some kind of migration guide?

Yes indeed, this should have been documented more clearly. I will fix it.