piranha icon indicating copy to clipboard operation
piranha copied to clipboard

[Kotlin] Returning a lambda with multiline code creates syntax errors

Open K1rakishou opened this issue 3 months ago • 7 comments

In our project we are using a quite unorthodox way to check whether experiments are enabled/disabled. We have a special class, let's name it ExperimentChecker, which has lots of different methods, from simple ones like isOn()/isOff() to some advanced ones.

Consider the following code:

class SomeClass(
    private val expChecker: ExpChecker
) {
    fun someFunc(): SomeValue {
        return expChecker.check(Experiments.ExpName) {
            off { 
               logger.log("off branch called")
               SomeValue.OldValue
            }
            on {
               logger.log("on branch called")
               SomeValue.NewValue
            }
        }
    }
}

If we try to match and remove one of the branches along with expChecker.check(Experiments.ExpName) invocation we will end up with the following code (let's remove the off branch in this example):

class SomeClass(
    private val expChecker: ExpChecker
) {
    fun someFunc(): SomeValue {
        return logger.log("on branch")
               SomeValue.NewValue
    }
}

As you can see it introduces a syntax error which then crashes piranha.

I'm not sure if this is even supported by piranha. If it's not then we will have to figure it out on our own. But if it is supported then please could you point me to an example on how it should be handled? Thanks.

K1rakishou avatar Apr 03 '24 07:04 K1rakishou

is the expected code (we want to retain the braces right?)

class SomeClass(
    private val expChecker: ExpChecker
) {
    fun someFunc(): SomeValue {
        return expChecker.check(Experiments.ExpName) {
             {
               logger.log("on branch called")
               SomeValue.NewValue
            }
        }
    }
}

I am assuming you wrote a feature flag api cleanup rule specifically for your custom API, which produced the above change, where the off branch is elided. Could you share that with me?

ketkarameya avatar Apr 03 '24 13:04 ketkarameya

Sorry forgot to mention what it's expected to look like:

class SomeClass(
    private val expChecker: ExpChecker
) {
    fun someFunc(): SomeValue {
        logger.log("on branch called")
        return SomeValue.NewValue
    }
}

Basically the code is extracted from the on lambda and then inserted in-place. The braces are to be removed too because they are not just braces in Kotlin but a lambda function.

But it doesn't work because of return. It will also probably not work if the result of that expression were to be assigned to a variable and probably many other cases that I can't think of right now.

I am assuming you wrote a feature flag api cleanup rule specifically for your custom API, which produced the above change, where the off branch is elided.

Yes, we wrote a custom rule. Here is the full rule.

[[rules]]
name = "expchecker_check"
query = """(
(call_expression
    (call_expression
        (navigation_expression
            (simple_identifier) @expchecker_field
            (navigation_suffix (simple_identifier) @type)
        )
        (call_suffix
            (type_arguments
                (type_projection[
                    (user_type (_)
                    )
                ])
            )?
            (value_arguments
                (value_argument[
                    (navigation_expression(_) (navigation_suffix (simple_identifier) @experimentName)
                    )]
                )
            )
        )
    )
    (call_suffix ( annotated_lambda
        (lambda_literal [
                (statements(call_expression
                    (simple_identifier) @expchecker_block
                    (call_suffix (annotated_lambda
                            (lambda_literal[
                                (statements)@takenNode
                            ]))
                    ))
                )
        ]))
    )
)@ReplacedNode

(#eq? @expchecker_field "expChecker")
(#eq? @experimentName "@experiment_name")
(#eq? @expchecker_block "@taken_expchecker_lambda_name")
(#eq? @type "check")
)"""
replace_node = "ReplacedNode"
replace = "@takenNode"
holes=["taken_expchecker_lambda_name", "experiment_name"]

And then in substitutions taken_expchecker_lambda_name is going to be assigned on and experiment_name is going to be assigned ExpName,

For now, my idea to fix this is to wrap multi line code into a Kotlin's run function (think of it as of a lambda function which is invoked right away and is also then inlined directly into the JVM bytecode by Kotlin compiler). So it will look like this:

class SomeClass(
    private val expChecker: ExpChecker
) {
    fun someFunc(): SomeValue {
        return run {
            logger.log("on branch")
            SomeValue.NewValue
        }
    }
}

Not sure how to do this right now, just an idea in case there is no simple solution.

K1rakishou avatar Apr 04 '24 07:04 K1rakishou

sorry! This issue completely slipped off my mind.

(call_expression
    (call_expression
        (navigation_expression
            (simple_identifier) @expchecker_field
            (navigation_suffix (simple_identifier) @type)
        )
        (call_suffix
            (type_arguments
                (type_projection[
                    (user_type (_)
                    )
                ])
            )?
            (value_arguments
                (value_argument[
                    (navigation_expression(_) (navigation_suffix (simple_identifier) @experimentName)
                    )]
                )
            )
        )
    )
    (call_suffix ( annotated_lambda
        (lambda_literal [
                (statements(call_expression
                    (simple_identifier) @expchecker_block
                    (call_suffix (annotated_lambda
                            (lambda_literal[
                                (statements)
                            ])@takenNode )
                    ))
                )
        ]))
    )
)@ReplacedNode
(#eq? @expchecker_field "expChecker")
(#eq? @experimentName "@experiment_name")
(#eq? @expchecker_block "@taken_expchecker_lambda_name")
(#eq? @type "check")
)```

^ This query shud work ( I tried on ts-kt)

ketkarameya avatar Apr 08 '24 14:04 ketkarameya

is ur codebase open source?

ketkarameya avatar Apr 08 '24 14:04 ketkarameya

No, it's not open-source.

So the above query removes everything except for the lambda body now. So instead of this code:

class SomeClass(
    private val expChecker: ExpChecker
) {
    fun someFunc(): SomeValue {
        return logger.log("on branch")
               SomeValue.NewValue
    }
}

It now produces this code:

class SomeClass(
    private val expChecker: ExpChecker
) {
    fun someFunc(): SomeValue {
        return {
               logger.log("on branch")
               SomeValue.NewValue
        }
    }
}

Which might work in some languages, but not in Kotlin, because in Kotlin this block:

return {
   logger.log("on branch")
   SomeValue.NewValue
}

basically means return a lambda. Which in this case is a compilation error. It's possible to fix this by adding () at the end (invoke lambda right away), like so:

return {
       logger.log("on branch")
       SomeValue.NewValue
}()

But I don't know how to insert new text into the code using piranha rules. Is this possible? If that is possible then I can figure out the rest.

K1rakishou avatar Apr 09 '24 04:04 K1rakishou

yes inserting text is possible as an "update" operation. so wait, do u want :

return {
       logger.log("on branch")
       SomeValue.NewValue
}()

or

 return run {
            logger.log("on branch")
            SomeValue.NewValue
        }

Either shud be possible

u can update ur replace to replace = run @takenNode ? or replace = @takenNode()?

ketkarameya avatar Apr 09 '24 19:04 ketkarameya

Yes, this works. Thank you very much!

K1rakishou avatar Apr 10 '24 04:04 K1rakishou