Wrapping inside long statements is weird

[fejesjoco]

I frequently have to generate very big function bodies where some individual statements are huge. And it's actually pretty difficult to generate reasonably looking output. The fix for this might be as simple as providing examples in the documentation.

Let me describe the problem first.

Generating code:

  val lambda = CodeBlock.of(
    """
      { foo -> println(foo + %S + %S) }
    """.trimIndent(),
    "A".repeat(50),
    "B".repeat(50),
  )
  val cb1 = buildCodeBlock {
    addStatement("if (foo) { longFunctionCall(%S, %L, %S) }", "X".repeat(50), lambda, "Y".repeat(50))
  }
  val cb2 = buildCodeBlock {
    addStatement(
      """
        if (foo) {
          longFunctionCall(
            %S,
            %L,
            %S
          )
        }
      """.trimIndent(),
      "X".repeat(50), lambda, "Y".repeat(50))
  }
  val cb3 = buildCodeBlock {
    add(
      """
        if (foo) {
          longFunctionCall(
            %S,
            ⇥%L⇤,
            %S
          )
        }
      """.trimIndent(),
      "X".repeat(50), lambda, "Y".repeat(50))
  }

  FileSpec.builder("com.test", "HelloWorld")
    .addType(
      TypeSpec.classBuilder("HelloWorld")
        .addFunction(FunSpec.builder("hello1").addCode(cb1).build())
        .addFunction(FunSpec.builder("hello2").addCode(cb2).build())
        .addFunction(FunSpec.builder("hello3").addCode(cb3).build())
        .build()
    )
    .build()
    .writeTo(System.out)

It prints:

package com.test

public class HelloWorld {
  public fun hello1() {
    if (foo) { longFunctionCall("XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", { foo ->
        println(foo + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
        "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB") },
        "YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY") }
  }

  public fun hello2() {
    if (foo) {
          longFunctionCall(
            "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX",
            { foo -> println(foo + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
            "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB") },
            "YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY"
          )
        }
  }

  public fun hello3() {
    if (foo) {
      longFunctionCall(
        "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX",
        { foo -> println(foo + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
          "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB") },
        "YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY"
      )
    }
  }
}

The first function looks quite horrible, because the wrapping has no idea about semantics, it just breaks in the middle of the deepest level of nesting if it feels like it. My naive attempt at fixing that is the second code block where I do the wrapping by hand, using Kotlin's lovely triple-quoted string. But it still looks bad because of addStatement's behavior. It indents all lines after the first, but in Kotlin the last line of a block should be indented the same as the first line. My third attempt is to use add instead of addStatement and also using the indentation magic characters. The output of that looks very good.

The proper fix for addStatement's bad wrapping would be to count opening and closing braces and parentheses but I understand it could be difficult. So we could just say that addStatement has this limitation and people should live with it and use add instead.

The problem with add is that when I add my own indentation, it's not compatible with KotlinPoet's indentation. But I figured that if I use indentation magic characters around composition points, it works really great. One fix would be to detect the current indentation when embedding a nested CodeBlock into another nested CodeBlock, but again it could be difficult. So instead it could be just added to the documentation how people should deal with the indentation.

[JakeWharton]

So there's multiple statements here not just one which is why it gets weird when you try to use addStatement which was designed to emit a single statement.

Traditionally, constructs like if are meant to be created with beginControlFlow/nextControlFlow/endControlFlow. This automatically emits the correct indentation changes as well as handles the curly braces. It's nothing you can't do yourself, but it is meant to simplify things because KotlinPoet is not a compiler or lexer/parser and tries really hard to know nothing about what's going on in a statement (aside from where to break it when long).

[Egorand]

KotlinPoet prioritizes correctness, which we believe is more important in generated code than getting the formatting 100% right (though the library makes a decent effort at producing reasonably formatted code). If formatting is important, we usually recommend post-processing KotlinPoet's output with a formatter tool, such as ktlint.

[fejesjoco]

The example may be a bit wrong, since I work on proprietary code I have to make stuff up. My problems usually start when I have to call a statement, and within that I have to embed a lot of stuff, like other calls or lambdas. We can't embed that kind of stuff within an addStatement. And simply having too many parameters alone is a reason to avoid addStatement because of its wrapping behavior. But I agree that I prefer to use addStatement/beginControlFlow wherever possible.

At our company we have a massive amount of builds, and we have many code generators in the build pipeline (Bazel genrule), and we were asked to not format code as part of the builds because the total resource usage and effect on build time can be big if we add it all up. So we put some effort (not too much) into making the generated code look reasonable enough by default.

I think in this case using constructs like ⇥%L⇤ really makes a difference in the output. So I mainly wanted to suggest mentioning it in the official documentation. Because I'm not even sure if it's part of the public API. On top of that it would be great if addStatement wrapped at commas instead of wrapping at the column limit. Other than these two suggestions I accept that this is pretty open ended and there are no big benefits to be had here.

[Egorand]

I think in this case using constructs like ⇥%L⇤ really makes a difference in the output. So I mainly wanted to suggest mentioning it in the official documentation. Because I'm not even sure if it's part of the public API.

It is part of the public API, see docs for CodeBlock!

On top of that it would be great if addStatement wrapped at commas instead of wrapping at the column limit.

I don't think it'll be easy. We don't want to simply always wrap after a comma, don't think it'll be correct in many cases. Indentation is another concern which is not easy to get right. All in all, given that there are tools specializing in formatting Kotlin code, I don't think it's worth investing time into KotlinPoet's built in formatting rules (except for fixing obvious bugs and low-effort high-impact improvements). Completely understand your company's limitations though, it's a tradeoff.