square / kotlinpoet

A Kotlin API for generating .kt source files.
https://square.github.io/kotlinpoet/
Apache License 2.0
3.88k stars 286 forks source link

Function declaration with direct return and multiple statements throws `IllegalStateException` #1979

Open fourlastor opened 1 day ago

fourlastor commented 1 day ago

Describe the bug

I tried generating a function where there are multiple addStatement calls, the first of which is a return, but somehow the first opening statement symbol is lost when calling toString() on the function spec, and the generation fails with the following exception:

java.lang.IllegalStateException: Can't close a statement that hasn't been opened (closing » is not preceded by an
opening «).
Current code block:
- Format parts: [1, \n, », «, .plus(2), \n]
- Arguments: []

Note how the format parts are missing the initial «.

To Reproduce

  @Test fun returnExpressionMultipleStatements() {
    val spec = FunSpec.builder("three")
      .returns(INT)
      .addStatement("return 1")
      .addStatement(".plus(2)")
      .build()

    assertThat(spec.toString()).isEqualTo(
      """
      |public fun three(): kotlin.Int =
      | 1
      | .plus(2)
      """.trimIndent()
    )
  }

Note that this can be worked around changing addStatement to addCode for the first statement (the one containing the return), however this makes the indentation a bit confusing in the final result, also not having a return in the first statement would make this compile (on a Unit function, for example)

Expected behavior

The function is generated without throwing.

Egorand commented 1 day ago

Yep, I can see what the issue is. The format parts of the function body originally look like this:

[«, return 1, \n, », «, .plus(2), \n, »]

The «» here delineate the statements. The library tries to find out if the body starts with return or throw and hence the function can be printed out as an expression function, and for that it needs to trim the special characters, resulting in the following sequence:

[return 1, \n, », «, .plus(2), \n]

This of course is now broken cause brackets are mismatched.

Not sure what the correct solution here is (perhaps we check for the presence of multiple statements in the body and print the function normally if there are multiple statements?), but the obvious workaround is to merge statements into a single statement, either in code, or via CodeBlock.joinToCode.

Thanks for the report!

fourlastor commented 1 day ago

Thanks for the suggestion! I can confirm that the workaround with CodeBlock.joinToCode works as expected:


  @Test fun returnExpressionMultipleStatements() {
    val spec = FunSpec.builder("three")
      .returns(INT)
      .addCode(listOf(
        CodeBlock.of("return 1"),
        CodeBlock.of(".plus(2)"),
      ).joinToCode("\n"))
      .build()

    assertThat(spec.toString()).isEqualTo(
      """
      |public fun three(): kotlin.Int {
      |  return 1
      |  .plus(2)
      |}
      |""".trimMargin(),
    )
  }
JakeWharton commented 1 day ago

Note that .plus(2) is not actually a statement. The expression body code should not kick in for what you wrote, though.

fourlastor commented 1 day ago

What would be the correct way to write this? The example is oversimplified (to get a simple example that breaks), the real case is something on the lines of:

fun foo(): Thing = Thing.builder()
  .setX(123)
  .setY(456)
  .setZ("Hello")
  .setW("World")
  .build()

I was basically trying to get this correctly indented and one function call per line (as they might be quite a few calls)

JakeWharton commented 1 day ago

Regular line breaks within a statement will indent subsequent lines.

So addStatement("return 1\n.plus(2)") should work.