Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fourlastor opened this issue Sep 20, 2024 · 5 comments
Labels

Comments

@fourlastor
Copy link

fourlastor commented Sep 20, 2024

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.

@fourlastor fourlastor added the bug label Sep 20, 2024
@Egorand
Copy link
Collaborator

Egorand commented Sep 20, 2024

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
Copy link
Author

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
Copy link
Collaborator

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

@fourlastor
Copy link
Author

fourlastor commented Sep 20, 2024

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
Copy link
Collaborator

Regular line breaks within a statement will indent subsequent lines.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants