Refactor transform method to extension method

[Jay-Lokhande]

Refactor transform method to extension method update implicit class name to avoid conflicts

PR Checklist (not all items are relevant to all PRs)

• Added unit-tests (for runtime code)
• Added bootstrapped code + smoke tests (when the rendering logic is modified)
• Added build-plugins integration tests (when reflection loading is required at codegen-time)
• Added alloy compliance tests (when simpleRestJson protocol behaviour is expanded/updated)
• Updated dynamic module to match generated-code behaviour
• Added documentation
• Updated changelog

[CLAassistant]

All committers have signed the CLA.

[kubukoz]

Thanks, the generated code looks alright.

I see 2.12 (at least) is failing to compile its tests, I'm guessing it has something to do with the implicit conversion not being picked up because the type of F doesn't infer nicely... feel free to attempt to resolve it somehow, we'll see if this is even possible on all the Scala versions.

[Baccata]

I think the failure is probably due to implicit scope on 2.12.

I wonder whether maybe we should just forsake it and document a unique static method that would replace the current transform.

[kubukoz]

maybe... but I would try this first:

@Jay-Lokhande lets have a separate implicit class for each method, that way you can make the receiver (the "self" value that the extension is called on) the right kind and hopefully help 2.12's inference. You'll also need to update any cross-compiled tests with 2.12 to use the appropriate variant.

[Jay-Lokhande]

maybe... but I would try this first:

@Jay-Lokhande lets have a separate implicit class for each method, that way you can make the receiver (the "self" value that the extension is called on) the right kind and hopefully help 2.12's inference. You'll also need to update any cross-compiled tests with 2.12 to use the appropriate variant.

yes sure I will try this approach

[kubukoz]

Looks like we should be good now, with no source-level breakage. For some reason the three implicit classes don't clash on 2.13/3.x, and the mono/bifunctor-specialized ones get picked up on 2.12 too.

Time to make a decision - @Baccata are we rolling with this? I think I'd be OK with a static transform too.

[Baccata]

? I think I'd be OK with a static transform too

I'd rather we did that, to be honest. Transform is not something that's being called often enough to warrant the complexity of the Ops that need generating for providing a method-like DX

[kubukoz]

alright, let's do that then. @Jay-Lokhande, do you want to make these changes?

[Jay-Lokhande]

alright, let's do that then. @Jay-Lokhande, do you want to make these changes?

Yes!

[kubukoz]

@Baccata it looks like it's the same problem as with the extension...

so it's just a matter of preference. The extension at least didn't require code changes for users.

[kubukoz]

My vote is to keep the extension functions, in a future version that drops 2.12 support we can simplify to just have one extension for all.

Getting this merged at least solves a real problem, I'd like to get it finalized

[Jay-Lokhande]

@kubukoz PR added extension methods is now ready for review....

[kubukoz]

added a changelog, will merge on green