fun-keyword-spacing is redundant with keyword-spacing

[mgroth0]

Maybe fun-keyword-spacing should be removed, and parameters could be added to keyword-spacing to include or exclude specific keywords?

[paul-dingemans]

Those rules can be merged. Except that it is one rule less, there does not seem to be much other benefits. Also, there are more spacing rules than this.

So what would be the benefit from your perspective?

[mgroth0]

In my opinion, its just about the benefits of removing redundancy. I haven't yet taken a close look at most of the rules, but in general I think removing redundancy between rules is good? Removing redundancy might lead to performance improvements but more importantly I think it just makes the library easier and more organized for maintainers and users.

[paul-dingemans]

In my opinion, its just about the benefits of removing redundancy. I haven't yet taken a close look at most of the rules, but in general I think removing redundancy between rules is good?

Yes, and no. Given two rules that have no overlap. Combining those rules into one rule means less flexibility for users who want to disable only one of the rules, as they then can only disable the combined rules. Given two rules where one of the rules is a subset of the other rule it would also result in loss of flexibility as the user can no longer keep the first rule enabled while disabling the rule with which it overlaps.

Removing redundancy might lead to performance improvements

Of course it is more performant to have less rules. But one should never optimize before measuring that this is a performance bottleneck.

but more importantly I think it just makes the library easier and more organized for maintainers and users.

Yes and no. Less rules is of course more easy. But if the rules get more complex then the benefit is quickly lost.

In this particular example I see no gains (reuse of code) by combining fun-keyword-spacing and keyword-spacing. It would also not be a big deal to combine them as the complexity of the combined rule is still manageable. Combining them could cause some problems for the very unlikely case that some user has disabled the fun-keyword-spacing.

[mgroth0]

Combining those rules into one rule means less flexibility for users who want to disable only one of the rules, as they then can only disable the combined rules

I agree that we don't want to sacrifice configurability. Part of my original suggestion was to add one or more new rule-specific .editorconfig properties. For example, we could define ktlint_standard_keyword_spacing_exclude/include to be a comma delemited list of keywords that we want to exclude/include.

So instead of:

ktlint_standard_keyword-spacing = disabled
ktlint_standard_fun-keyword-spacing = enabled

We could have:

ktlint_standard_keyword-spacing = enabled
ktlint_standard_keyword_spacing_include = fun,var

The two designs above achieve the same goal, but the one below provides a higher degree of flexibility to the user because the user can decide exactly which keywords to include/exclude. It seems easier to learn and more flexible. I see there is also a rule for "Modifier list spacing" and this could also be part of the merge.

one should never optimize before measuring that this is a performance bottleneck.

This sounds like reasonable wisdom. We can ignore the performance argument, as this was just a secondary motivation.

Combining them could cause some problems for the very unlikely case that some user has disabled the fun-keyword-spacing

I propose this could just be converted to:

ktlint_standard_keyword-spacing = enabled
ktlint_standard_keyword_spacing_exclude = fun

[paul-dingemans]

This is a way how it could be solved, but it is not the way we should do it.

Adding configuration options is a kind of last resort in Ktlint. The more options ktlint has, the more it leads to bikeshedding. Also, it has been proven to be hard to maintain (e.g. more difficult to test), it is hard for users to understand what possible options are, and it is more difficult to document. User often request additional configuration options for their use case, and for this reason I have add this FAQ.

For this particular case, I can not think of any good reasons why a configuration option would be of value. Why should it be supported that user can choose for which keyword the standard spacing is used or not.

[mgroth0]

Got it. I think your reasoning for not merging them is valid. As long as Ktlint is extensible then there need not be any pressure to overcomplicate the standard ruleset.

But according to this logic, the existence of fun-keyword-spacing seems unjustified. Am I missing something? What reason would anyone have to want different spacings around fun but not similar keywords like val or var?

You mentioned above that removing flexibility for users is undesirable, but now you are saying that bikeshedding and overly complicated configuration is undersirable. It seems that these two design principles are conflicting. But if the conclusion is to just leave it as is, with a special spacing rule for fun but not for other keywords, that doesn't seem right either as it is arbitrarily giving special flexibility for one keyword.

I also am surprised that you don't see the potential to merge rules in a way that reuses code. I found this interesting so I tried to clone the library and investigate merging the rules myself but am having an issue setting up the project.

[paul-dingemans]

But according to this logic, the existence of fun-keyword-spacing seems unjustified. Am I missing something? What reason would anyone have to want different spacings around fun but not similar keywords like val or var?

You mentioned above that removing flexibility for users is undesirable, but now you are saying that bikeshedding and overly complicated configuration is undersirable. It seems that these two design principles are conflicting. But if the conclusion is to just leave it as is, with a special spacing rule for fun but not for other keywords, that doesn't seem right either as it is arbitrarily giving special flexibility for one keyword.

The fun-keyword-spacing was created while developing the function-signature rule. When that rule got more and more complicated, I extracted some logic into separate rules. When I created the fun-keyword-spacing rule, I did not notice that there was already a rule keyword-spacing. If I would have notice that, I probably would have extended that rule. But now the rule has been published, it has given users the flexibility to disable only one of the rules.

A a general design principle, more smaller rules give users more flexibility. But you're right that using that flexibility leads to bikeshedding. Enabling/disabling rules however is generic for all rules. Users do not need to lookup documentation once they know the principle. When introducing configuration parameters, users do need to read the documentation to become aware of them, and how to use them.

I also am surprised that you don't see the potential to merge rules in a way that reuses code.

I have investigated the merge of the fun-keyword-spacing and keyword-spacing based on your earlier remarks, and did not see how merging would lead to reuse of code.

I found this interesting so I tried to clone the library and investigate merging the rules myself but am having https://github.com/pinterest/ktlint/issues/2529.

Cool, I am interested in your findings.