pinterest
Indentation change since 0.38.1
Expected Behavior
Upgrading from 0.38.1 to latest I see a change in indentation with parameter names in a call.
For example this was allowed in 0.38.1:
someFunction(
parameterName =
someValue
.someProperty
.someCall()
)
Observed Behavior
With 0.41.1 it now demands that it be indented like this:
someFunction(
parameterName =
someValue
.someProperty
.someCall()
)
Which is not very readable.
I can make it go away by moving someValue to same line as parameter name:
someFunction(
parameterName = someValue
.someProperty
.someCall()
)
But sometimes there is a longer expression than this example and it is more readable to move it to its own line.
It still hapens for me once I use the --experimental parameter on 0.43.2.
I double checked it once more on my site, but it works fine. Please provide me details about following:
- java version
- editorconfig
- full code sample in which you experiment the problem when it is not identical to the sample given in this issue
- full output when running with flags
--format --verbose --relative --experimental --debug
trying this:
$ java --version
openjdk 17.0.1 2021-10-19
OpenJDK Runtime Environment Temurin-17.0.1+12 (build 17.0.1+12)
OpenJDK 64-Bit Server VM Temurin-17.0.1+12 (build 17.0.1+12, mixed mode, sharing)
- no editorconfig, only single example file
private fun transactionOf() = PurchaseTransaction().also {
it.timestamp = timestamp
it.timestampDesk = timestamp
it.sale = testSale
it.count = count
it.amountCents = amountCents
it.reserved = reserved
it.dealSplit = mutableListOf(
PurchaseTransactionDeal().also {
it.sale = testSale
it.deal = testDeal
it.count = count
it.amountCents = amountCents
}
)
}
retina:~/Workspace/ktlint-test$ cat PurchaseInvoiceConsumerTest.kt
private fun transactionOf() = PurchaseTransaction().also {
it.timestamp = timestamp
it.timestampDesk = timestamp
it.sale = testSale
it.count = count
it.amountCents = amountCents
it.reserved = reserved
it.dealSplit = mutableListOf(
PurchaseTransactionDeal().also {
it.sale = testSale
it.deal = testDeal
it.count = count
it.amountCents = amountCents
}
)
}
retina:~/Workspace/ktlint-test$ ktlint
retina:~/Workspace/ktlint-test$ ktlint --experimental --format --verbose --relative --debug
[DEBUG] Discovered ruleset with "standard" id.
[DEBUG] Discovered ruleset with "experimental" id.
[DEBUG] Discovered reporter with "baseline" id.
[DEBUG] Discovered reporter with "checkstyle" id.
[DEBUG] Discovered reporter with "json" id.
[DEBUG] Discovered reporter with "html" id.
[DEBUG] Discovered reporter with "plain" id.
[DEBUG] Discovered reporter with "sarif" id.
[DEBUG] Initializing "plain" reporter with {verbose=true, color=false, color_name=DARK_GRAY}
[DEBUG] Checking PurchaseInvoiceConsumerTest.kt
Resolving .editorconfig files for /Users/knyttlwork/Workspace/ktlint-test/PurchaseInvoiceConsumerTest.kt file path
Loaded .editorconfig: []
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jetbrains.kotlin.com.intellij.util.ReflectionUtil (file:/usr/local/Cellar/ktlint/0.43.2/libexec/ktlint) to field java.lang.Throwable.backtrace
WARNING: Please consider reporting this to the maintainers of org.jetbrains.kotlin.com.intellij.util.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
PurchaseInvoiceConsumerTest.kt:9:9: Unexpected indentation (expected 4, actual 8) (experimental:argument-list-wrapping)
PurchaseInvoiceConsumerTest.kt:15:5: Unexpected indentation (expected 0, actual 4) (experimental:argument-list-wrapping)
[DEBUG] 1331ms / 1 file(s) / 2 error(s)
Another failing example is:
val foo = listOf(
Status.OK example MessageThreadResponse(
ListResponseMeta(0, true),
listOf(messageThreadDtoExample),
MessageThreadResponseIncluded(listOf(), listOf(), listOf()),
)
)
works correctly without --experimental flag.
@knyttl Your examples do not seem to be related to the original problem of @dalewking. Your problem is already addressed in https://github.com/pinterest/ktlint/pull/1284.
@dalewking This problem is indeed not solved. Due to the partial code sample, I misinterpreted it before. A complete code sample, reproducing the problem is:
fun someFunction(parameterName: String) =
parameterName.toUpperCase()
@Test
fun name() {
val actual = someFunction(
parameterName =
"The quick brown fox "
.plus("jumps ")
.plus("over the lazy dog")
)
assertThat(actual).isEqualTo("THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG")
}
I did some further investigation. The current behavior has been built-int on purpose as a result of https://github.com/pinterest/ktlint/issues/964. I have verified in IntelliJ Ultimate and the default formatting for the example above is:
fun someFunction(parameterName: String) =
parameterName.toUpperCase()
@Test
fun name() {
val actual = someFunction(
parameterName =
"The quick brown fox "
.plus("jumps ")
.plus("over the lazy dog")
)
assertThat(actual).isEqualTo("THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG")
}
Personally, I would prefer to change the formatting as has been requested. However, it conflicts with the (default) IntelliJ / Android Studio formatting. One of our code policies, is that code that is formatted by ktlint should be accepted by the default IntelliJ formatting without any conflicts. @romtsn Please confirm, whether you agree on this. If so, this issue should be closed.
The fact that IntelliJ / Android Studio format the code that way shouldn't matter, since it is incorrect according to Jetbrain's own style guide.
Unfortunately they don't have a rule specifically about named parameters, but we can infer from several very similar rules:
- https://kotlinlang.org/docs/coding-conventions.html#expression-bodies
- https://kotlinlang.org/docs/coding-conventions.html#properties
- https://kotlinlang.org/docs/coding-conventions.html#wrap-chained-calls
Considering the purpose of the style guide is for consistency to begin with, this formatting quirk in IntelliJ is a bug, not a feature to emulate.
While I agree with you that ideally IntelliJ should fix its formatter, I don't think it is realistic to ignore IntelliJ's integrated formatter. It would be bad for developer's user experience to turn off integrated IDE formatter and only rely on external ktlint formatter.
However, it conflicts with the (default) IntelliJ / Android Studio formatting.
Could we maybe have some flag for this, which could be disabled? I personally prefer to have a nice code than just follow a weird IDE behaviour.
While I agree with you that ideally IntelliJ should fix its formatter, I don't think it is realistic to ignore IntelliJ's integrated formatter. It would be bad for developer's user experience to turn off integrated IDE formatter and only rely on external ktlint formatter.
This issue first needs to be resolved in the IntelliJ formatter. Please file an issue at their tracker first.
Could we maybe have some flag for this, which could be disabled? I personally prefer to have a nice code than just follow a weird IDE behaviour.
Ktlint should not offer such a flag for a minor issue like this. Most important reason is that it would still conflict with IntelliJ formatting. If IntelliJ would offer such a flag, we could make ktlint compatible with it. Secondly because the increase of code complexity as well as explaining the configuration settings for such flags becomes bigger and bigger with each flag being added.
This seems backwards to what ktlint should be doing. Adhering to random and unexplained behavior in IntelliJ to the letter instead of examining the situation and choosing the better option ('better' being what everyone here is agreeing with) is not an admirable quality for a formatter like this.
I use ktlint to remove the ambiguity of IDE formatting, not the other way around.
I created an issue for IntelliJ which relates to this: https://youtrack.jetbrains.com/issue/IDEA-291053