"Unexpected indentation" on wrapped function declarations

[xenomachina]

The following function appears in the Kotlin Reference:

fun reformat(str: String,
             normalizeCase: Boolean = true,
             upperCaseFirstLetter: Boolean = true,
             divideByCamelHumps: Boolean = false,
             wordSeparator: Char = ' ') {
...
}

If you copy this to a source file (and remove the ellipsis), ktlint will complain:

src/main/kotlin/Foo.kt:3:1: Unexpected indentation (13)
src/main/kotlin/Foo.kt:4:1: Unexpected indentation (13)
src/main/kotlin/Foo.kt:5:1: Unexpected indentation (13)
src/main/kotlin/Foo.kt:6:1: Unexpected indentation (13)

This style also appears numerous times within the Kotlin compiler source, and when asked to reformat the code, this is the indentation style used by IntelliJ, so I think this error is incorrect.

[shyiko]

Hi.

This is because Code Style for Kotlin hasn't been applied ("Code Style" -> "Kotlin" -> "Wrapping and Braces" -> "Method declaration parameters" -> "Align when multiline" is on while it should be off). Right now indentation logic is simple: 4 spaces everywhere (which makes rules about how to align method parameters, function call arguments, etc. consistent and easy to remember). The code above can be written (reformater can do it for you provided aforementioned flag is off) as

fun reformat(
    str: String,
    normalizeCase: Boolean = true,
    upperCaseFirstLetter: Boolean = true,
    divideByCamelHumps: Boolean = false,
    wordSeparator: Char = ' '
) {
    ...
}

Kotlin compiler source is quite inconsistent in term of code style (it was breaking pretty much every rule last time I tried ktlint on it) so it's not really the best place to look.

There is a relevant discussion on vertical alignment at Kotlin/kotlin-style-guide#23 (comment). So far, ktlint tried to stay away from any kind of vertical alignment just to keep things simple.

[xenomachina]

I don't think that discussion is talking about exactly the same thing, though they are related. They're talking more about alignment of elements within the line, like infix operators or parameters to calls of the same function, by adding multiple internal spaces.

I do sympathize with @yole's comment, re: "The problem with any kind of vertical alignment", and I think the problems he mentions do apply in this case (though perhaps to a lesser degree than in some others). Interestingly, in Kotlin/kotlin-style-guide#15 (comment) he indicates that he thinks it's important that parameters align, and doesn't like being forced to have a line break after the open paren.

Inserting the newline leads to some somewhat ugly cases in my code:

1. Using "=" instead of braces to define a function results in the function body masquerading as a parameter.

   fun <T> adding(
           vararg names: String,
           help: String,
           transform: String.() -> T) =
           adding(*names, initialValue = mutableListOf(), help = help, transform = transform)
   

2. When there's a superclass/interface after a somewhat long constructor declaration the superclass/interface looks almost like a constructor argument. (The capitalization convention at least helps this stand out a bit.)

   class ShowHelpException internal constructor(
           private val helpFormatter: HelpFormatter,
           private val delegates: List<ArgParser.ParsingDelegate<*>>) :
           SystemExitException("Help was requested", 0) {
   

Relying on parameter alignment to fix these does seem kind of ugly, though, so I'm inclined to agree with you that this sort of alignment isn't such a great idea. So now I'm reformatting my code like this:

fun <T> adding(
            vararg names: String,
            help: String,
            transform: String.() -> T
    ) = adding(*names, initialValue = mutableListOf(), help = help, transform = transform)

and this:

class ShowHelpException internal constructor(
        private val helpFormatter: HelpFormatter,
        private val delegates: List<ArgParser.ParsingDelegate<*>>
) : SystemExitException("Help was requested", 0) {

I also egrepped for '<fun>.*([^)]+$' in my code and found a few other instance of this that just happened to fall on multiples of 4. It looks the recommended indentation for functions hasn't really been nailed down yet, but if ktlint is going to complain about non-multiple-of-4 indents, then perhaps it should also complain about function (or constructor) declarations that have multi-line parameter lists with a parameter on the first line.

[shyiko]

Yeah, I didn't mean that discussion is about the exact same problem. Vertical alignment comes in many different forms and

fun reformat(str: String,
             normalizeCase: Boolean = true,
             upperCaseFirstLetter: Boolean = true,
             divideByCamelHumps: Boolean = false,
             wordSeparator: Char = ' ') {
...
}

is just one of them. Imagine that you decided to rename reformat to format. In this case you would have to update every single line containing parameters (polluting git diff with needless chatter) because otherwise you'll end up with

fun format(str: String,
             normalizeCase: Boolean = true,
             upperCaseFirstLetter: Boolean = true,
             divideByCamelHumps: Boolean = false,
             wordSeparator: Char = ' ') {
...
}

multiple-of-4 rule eliminates this kind of problem.

fun reformat(
    str: String,
    normalizeCase: Boolean = true,
    upperCaseFirstLetter: Boolean = true,
    divideByCamelHumps: Boolean = false,
    wordSeparator: Char = ' '
) {
    ...
}

stays

fun format(
    str: String,
    normalizeCase: Boolean = true,
    upperCaseFirstLetter: Boolean = true,
    divideByCamelHumps: Boolean = false,
    wordSeparator: Char = ' '
) {
    ...
}

Regarding a newline before first parameter in multi-line parameter declaration, personally I feel like it would be a good thing (standardjs for example has a similar rule for objects (object-property-newline) which states that "object properties must go on a new line if they aren't all on the same line") but I'm not convinced (yet) that this should be enforced by ktlint. It all depends on the outcome of Kotlin/kotlin-style-guide#15 (comment).

[shyiko]

@xenomachina

Starting from 0.5.0 ktlint accepts both

fun f(
  val a: Any,
  val b: Any,
  val c: Any
) {
}

and

fun f(val a: Any,
      val b: Any,
      val c: Any) {
}

as valid.
This is primarily to minimize the contention that this particular topic is creating. Once Kotlin/kotlin-style-guide#15 is resolved ktlint will swing one way or another.

🎈

[voghDev]

An interesting fact I just discovered about Android Studio is that it sets Continuation indent to 8 spaces by default, so you will get tons of errors like these:

 Unexpected indentation (expected 8, actual 12)
 Unexpected indentation (expected 4, actual 8)

The way to fix this is opening Settings > Editor > Code Style > Kotlin > Tabs and Indents and set Continuation indent to 4.

After doing this, when reformatting code, ktlint won't complain about indents :-)

(I'm using version 0.25.0 of ktlint and Android Studio 3.1.2)

[seunggabi]

.editorconfig 406e832

disabled_rules=experimental:argument-list-wrapping