CodeBlock.endFlowControl shouldn't insert a new line by default

[Egorand]

Probably a relict of JavaPoet where a control flow would almost certainly be followed by new line. With Kotlin, new line by default makes code like below look bad:

@Test fun controlFlowsAsExpressions() {
  val controlFlows = buildList { 
    repeat(2) { 
      add(
        CodeBlock.builder()
          .beginControlFlow("if (num %% 2 == 0)")
          .addStatement("println(%S)", "foo")
          .nextControlFlow("else")
          .addStatement("println(%S)", "bar")
          .endControlFlow()
          .build()
      )
    }
  }
  val listOfControlFlows = controlFlows
    .joinToCode(prefix = "listOf(⇥\n", separator = ",\n", suffix = ",⇤\n)")
  assertThat(listOfControlFlows.toString()).isEqualTo(
    """
    listOf(
      if (num % 2 == 0) {
        println("foo")
      } else {
        println("bar")
      }
      ,
      if (num % 2 == 0) {
        println("foo")
      } else {
        println("bar")
      }
      ,
    )
    """.trimIndent()
  )
}

[JakeWharton]

I think the counterpoint is when you're writing it as statements in a body so I'm not sure omitting is a great default either.

In Kotlin it actually feels like we should model more constructs like control flow as a type which you can either emit as a statement or part of an expression.

Another one that comes to mind in this space is that you cannot use beginControlFlow for a trailing lambda with a named argument because it handles emitting the curly.

[Egorand]

I think that addStatement should be responsible for emitting the new line, by default a control flow should be an expression. Maybe every CodeBlock should be considered an expression by default, and addStatement("%L", codeBlock) should be how it can be turned into a statement? It'd be interesting to explore introducing a type to represent control flows though.

Another one that comes to mind in this space is that you cannot use beginControlFlow for a trailing lambda with a named argument because it handles emitting the curly.

This actually is supported!

kotlinpoet/kotlinpoet/src/test/java/com/squareup/kotlinpoet/CodeBlockTest.kt

Lines 454 to 468 in a1163da

	@Test fun beginControlFlowWithParams() {
	val controlFlow = CodeBlock.builder()
	.beginControlFlow("list.forEach { element ->")
	.addStatement("println(element)")
	.endControlFlow()
	.build()
	assertThat(controlFlow.toString()).isEqualTo(
	"""
	|list.forEach { element ->
	| println(element)
	|}
	|
	""".trimMargin(),
	)
	}

[JakeWharton]

Tough to pull this off without breaking like 100% of users though. Might be time to rework the whole "code" part of the API.

[Virtlink]

Another example where it's a problem that endControlFlow inserts a newline. If I want to generate this (Http4K) code:

public fun route(): ContractRoute {
    // Route
    return "/items" meta {
        summary = "Summary."
        description = "Description."
        returning(OK to "Success.")
    } bindContract GET to ::handler
}

This:

FunSpec.builder("route").apply {
    returns(ClassName("org.http4k.contract", "ContractRoute"))
    addComment("Route")
    beginControlFlow("return %S meta {", "/items")
    addStatement("summary = %S", "Summary.")
    addStatement("description = %S", "Description.")
    addStatement("returning(OK to %S)", "Success.")
    endControlFlow().addStatement("bindContract GET to ::handler")
}.build()

Actually generates this code with a newline before bindConstrant, which is invalid:

public fun route(): ContractRoute {
    // Route
    return "/items" meta {
        summary = "Summary."
        description = "Description."
        returning(OK to "Success.")
    }
    bindContract GET to ::handler
}

[JakeWharton]

That is not control flow. It's a multiline single statement.