Suspend lambda gets mangled during parsing/rewrite

[nomisRev]

A Kotlin suspend lambda gets rewritten incorrectly.

suspend fun example(
  title: String,
  verifyUnique: suspend (String) -> Boolean
): String = TODO()

becomes

suspend fun example(
  title: String,
  verifyUnique: Stringsuspend (String) -> Boolean
): String = TODO()

[traceyyoshima]

Hi @nomisRev , thanks for reporting the issue and creating a PR!
I'm happy with the test, but I think it should be added to MethodDeclarationTest.

I'd prefer you to have credit for the changes -- would you please add the test the LambdaTest instead?

Info:
In the test package under org.openrewrite.kotlin.tree each of the tests use ParserAssertions instead of Assertions. ParserAssertions includes an additional step to check if the source is fully parsed by checking if any text exists in Space. All of the parsing tests exist there, and we apply similar conventions across other languages.

Changes:

• Added issue annotation with a link to the open issue.
• Changed test name to communicate what is parsed.

    @Issue("https://github.com/openrewrite/rewrite-kotlin/issues/56")
    @Test
    void lambdaMethodParameterWithModifier() {
        rewriteRun(
          kotlin(
            """
            suspend fun example(
              title: String,
              verifyUnique: suspend (String) -> Boolean
            ): String = TODO()
            """,
            """            
            suspend fun example(
              title: String,
              verifyUnique: suspend (String) -> Boolean
            ): String = TODO()
            """
          )
        );
    }

[nomisRev]

Hey @traceyyoshima,

First of all thank you of thinking about giving me credit! I update the PR according to your feedback but was a little bit confused about:

I'm happy with the test, but I think it should be added to MethodDeclarationTest.

I'd prefer you to have credit for the changes -- would you please add the test the LambdaTest instead?

Looking at the tests in both MethodDeclarationTest and MethodDeclarationTest I felt it made more sense in the former.

Additionally, I also added the empty spaces as done in the other parsers tests. If you have any more feedback please let me know, and I'll happily adjust accordingly. I added @Disabled so we can go ahead and merge this test as discussed on Slack.

[traceyyoshima]

Looking at the tests in both MethodDeclarationTest and MethodDeclarationTest I felt it made more sense in the former.

Ah, that's a great observation -- Initially, I added a comment suggesting LambdaTest, but decided on MethodDeclarationTest, because I don't think suspend would be valid applied to general lambdas.

[nomisRev]

because I don't think suspend would be valid applied to general lambdas.

That is actually not the case, there is a special case suspend { } and I've written two failing cases based on them.
One related to this one, I will link it to the same issue.

And another one I am not sure what is happening, so I'll open a new issue for it. I'll raise a PR in a sec.