Support factory interfaces for function injection

[evant]

Function injection is a useful feature especially with compose. However, because it only works with function types you lose support for named & optional arguments which can make the signature harder to read. The proposed solution is to also support factory interfaces which support these features.

Currently you can do:

typealias MyFunction = (String) -> String

@Inject
fun MyFunction(dep: Dep, @Assisted arg: String): String = ...

In addition you should be able to do:

interface MyFunction {
  operator fun invoke(arg: String = "default"): String
}

@Inject
fun MyFunction(dep: Dep, @Assisted arg: String): String = ...

which can be called as:

MyFunction()
MyFunction(arg = "value")

Alternative:

Since we know which arguments are assisted we could in theory generate this factory function ourselves. However due to processing limitations they wouldn't be able to support default arguments. It would also mean more generated code exposed throughout the codebase which it limited to MyComponent::create currently. For these reasons I propose going with the more verbose version above. This may be re-visited if/when we get a way to read default arguments.

[sergeshustoff]

Disclaimer: I seem to be failing to see the usecase for function injection. It's likely that I'm missing something But in case I'm not missing something there here is my case for deprecating the whole thing)

I see a bunch of problems with function injection in general:

• typealias is required for function to be discoverable by DI, but it generally feels off and when function is renamed the typealias is not (though the matching name is required)
• typealias goes against #253 in a way that it is kind of a qualifier for the function while we can't do that for classes anymore. So api get's inconsistent here. I know, it's far fetched as typealias here is not exactly that, but still.
• in a way typealias does for a function what @Provides binding does for a class, but it's external to the component.
• seems that it breaks when 2 different types of magic mix in #388 (compose and kotlin-inject). While it is a bug in ksp I'd expect similar problems not to be uncommon simply because mixing magic like that is dangerous.

• in case of receiverFun from test it looks like receiver works as an assisted parameter, but it's not (and can't be) marked like that.

  typealias receiverFun = String.(arg: NamedFoo) -> String
  
  @Inject
  fun String.receiverFun(dep: Foo, @Assisted arg: NamedFoo): String = this

And then wrapping the function in a class with inject constructor doesn't look like much more code while makes it much more understandable and clear. Plus this way there is no need for the assisted injection (in this case):

@Inject
class ReceiverFuner(private val dep: Foo) {
    fun receiverFun(usedToBeAReceiver: String, arg: NamedFoo): String = usedToBeAReceiver
}

This approach doesn't work well for receiver though... But is it a common case?

And while a fun interface for assisted injection fixes some of those problems the main one remains - the interface is bound to the function only by the matching name. Not by returning said function like it would a class or by an annotation explicitly referencing the function. That kind of indirect connection feels wrong as an API.

So, with this in mind, maybe it's worth considering deprecating it away and making api simpler? It's not my lib and it's totally up to you what features you want in it. And, again, maybe I'm not seeing some vital usecase for the function injection

[evant]

Disclaimer: I seem to be failing to see the usecase for function injection. It's likely that I'm missing something

I can talk about how I'm using it currently with compose.

My app architecture consists of 'Repositories', 'Controllers', and 'Pages'. For this discussion you can think of a 'Controller' as equivalent to a 'ViewModel'. Each page is top-level composable function that injects a controller factory, and takes the current page's arguments.

typealias MyPage = @Composable (Page.MyPage, Modifier) -> Unit

@Composible
fun MyPage(createMyController: CreateMyController, @Assisted page: Page.MyPage, @Assisted modifier: Modifier) {
   ...
}

You are correct this would be equivalent to doing

@Inject
class MyPage(private val createMyController: CreateMyController) {
    @Composible
    @SuppressLint("ComposeNamingUppercase")
    operator fun invoke(page: Page.MyPage, modifier: Modifier) {
        ...
    }
}

and I've considered moving to that pattern multiple times. But it feels weird, a @Composible is normally a top-level function and if I were to write things manually without an injection lib that is how I would do it.

typealias goes against https://github.com/evant/kotlin-inject/issues/253

Not really, it ends up working far more like other cases of assisted injection where it's the function type that's actually used and not the name. Compare:

typealias CreateFoo = (B) -> Foo
@Inject
class Foo(val a: A, @Assisted val b: B)

to

typealias CreateFoo = (A) -> Foo
@Inject
fun CreateFoo(a: A, @Assisted b: B) : Foo

the one difference is the name is special for it to match the function (have more thoughts on that further down)

in case of receiverFun from test it looks like receiver works as an assisted parameter, but it's not (and can't be) marked like that.

You are right, this is a weird wart, thanks for bringing it up. It looks like receivers were not properly considered when the @Assisted annotation was introduced. You can't mark a receiver as @Assisted on a @Provides function either and I think that goes in the other direction (it can only be injected).

This approach doesn't work well for receiver though... But is it a common case?

I don't know, I'm not using a receiver on an injected function in my project for one data-point.

the interface is bound to the function only by the matching name. Not by returning said function like it would a class or by an annotation explicitly referencing the function. That kind of indirect connection feels wrong as an API.

I do agree with that btw, the reason the existing api came about is because there is no way to reference a function in an annotation. I have considered requiring something like @FunctionInjection("receiverFun") but I'm not sure if that's actually better?

[sergeshustoff]

Ok, I see. I guess I'll try to look at function references in annotations or something like that to maybe use for linking interfaces or typealiases to a function...

But it feels weird, a @Composible is normally a top-level function and if I were to write things manually without an injection lib that is how I would do it.

After splitting a huge project into modules I ended up with interfaces with composable functions implemented in different modules a few times (yes, it's weird) and stopped feeling like compose functions are special and necessarily top level. The system ones are, but they're just building blocks. Plus look at Voyager - it uses composable function in 'screen" class.

Plus with interface as initially proposed above there would be composable inside it.

[sergeshustoff]

Btw

typealias CreateFoo = (A) -> Foo
 @Inject
 fun CreateFoo(a: A, @Assisted b: B) : Foo

This looks like CreateFoo behaves more like something that @Provides then something with things injected in it... Not the case for composables tough, they return Unit

[evant]

This looks like CreateFoo behaves more like something that @Provides

Not quite, you can only inject the function, not it's return value

[sergeshustoff]

Ok, so looks like we want to keep this api as is. So here is my proposal how to do it with assisted factory:

@Target(CLASS, TYPEALIAS)
annotation class InjectFunction(
    /**
     * Optional name of the injected function. Just name if it's in te same package, full package otherwise. Name of annotated interface or typealias is used otherwise
     */
    val funName: String = ""
)

@InjectFunction("someMagic")
typealias SomeMagic1 = (NamedFoo) -> String
// or
@InjectFunction("someMagic")
fun interface SomeMagic2 {
    operator fun invoke(arg: NamedFoo): String
}

fun someMagic(foo: Foo, @Assisted arg: NamedFoo): String = arg.name

Why I suggest an additional annotation: both typealias and interface look and feel too much like assisted injection while it's not that strictly speaking. Because with assisted injection logic is based on what is returned and here logic is based on specific function with the same name. fun someMagic does not contribute to dependency graph except for specific cases when typealias is used, and unlike assisted injection we can't just use (NamedFoo) -> String in random place and expect it to be resolved using someMagic function. Another option would be to reverse the link and annotate the function instead, but I'd rather have annotation on typealias/interface because this way we can also have @AssistedFactory annotation for assisted factories (both typealias and interface). This way the 2 cases are separated, yet similar enough.

Does this make sense?

PS: the naming can be better

Upd: another option would be to call it all assisted injection, but with a twist in case of function injection: @AssistedFactory(factory = "optional.package.functionName"). factory is optional and references the function being injected. The annotation would be required for fun interfaces. That can be made experimental and applied first to fun interfaces only leaving typealiases as is for now and then we'd see how it goes. Maybe then slowly deprecate old magic with matching names or maybe introduce a separate annotation for function injection if this one doesn't seem to work and add matching names magic there.

[evant]

👍 That general approach sounds good, will have to think about the naming

Another option would be to reverse the link and annotate the function instead

This is actually not possible, the processor follows types from the component that was annotated so any metadata has to be on that type. The current implementation 'cheats' a little and looks for the same name in the current file but having something more explicit here would remove that limitation.

[sergeshustoff]

I wander if it's worth trying to support multiple functions in a single assisted factory... On one hand it looks weird, on the other hand it kinda allows a weird composition instead of inheritance in a complex multi-module app (when scoped by lifecycle and not by feature).

In a multi-module app I'd do something like this with current api:

interface FeatureAComponent {
    fun depA(): DepA1
    val assistedThingA: (name: String) -> DepA2
}

interface FeatureABindings {
    @Provides
    val SomethingAImpl.bind(): SomethingA = this
}

interface FeatureBComponent {
    fun depB(): DepB1
    val assistedThingB: (name: String) -> DepB2
}

@Component
abstract class Component : FeatureAComponent, FeatureBComponent, FeatureABindings

But if assisted factory allows more than one function it can be transformed into something else entirely - kind of a component extension and used instead of per-feature component interfaces:

@PartialComponent
interface FeatureAComponent {
    fun depA(): DepA1
    fun assistedThingA(name: String): DepA2
}

interface FeatureABindings {
    @Provides
    val SomethingAImpl.bind(): SomethingA = this
}

@PartialComponent
interface FeatureBComponent {
    fun depB(): DepB1
    val assistedThingB: ThingBFactory 
}

@PartialComponent
interface ThingBFactory {
    fun build(name: String): DepB2
}

@Component
abstract class Component : FeatureABindings {
    abstract val featureA: FeatureAComponent
    abstract val featureB: FeatureBComponent
}

In this case an assisted factory would be a particular case of partial component. Any functions of partial component would be implemented just as if thery were functions of component itself (except without problems with clashing names) and use the scope of the component. Plus in incorporates #261. Plus it's a single annotation to cover a lot of functionality. There are cons too though - a bit complicated to understand coming from dagger, function reference for function injection needs to move to some other annotation that will be put on a function/property in component or on a typealias. And things might get complicated when there are several layers of those partial components inside of partial components. And also it might require the same scope annotation as the component, otherwise it all might work weirdly with multiple scopes (though that needs some researching)

Does my ranting make sense?

[evant]

Your ranting does make sense. What you came up with looks suspiciously similar to dagger's subcomponents. I had originally chosen not to implement that feature because choosing between subcomponents and component dependencies was always a source of confusion in dagger that I wanted to avoid.

[sergeshustoff]

Huh. I didn't think of this similarity. With that in mind supporting such 'almost subconponents' seems a slippery slope