Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement contribution merging in KSP #1001

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

Implement contribution merging in KSP #1001

wants to merge 15 commits into from

Conversation

ZacSweers
Copy link
Collaborator

@ZacSweers ZacSweers commented May 14, 2024

This is a second, clean restart on implementing contribution merging in KSP. This would make Anvil compatible with dagger KSP (while also being backward compatible with dagger apt, but more on that later).

How it works

This approach builds on our recent simplifications to how hints are generated. Now that they are just in a single package, we can use public KSP APIs. It does change a little about how Anvil's historically worked. Instead of modifying in-memory class representations directly, this generates an intermediate component interface with the correct annotations (essentially a source file version of what we did in-memory before). This would then get processed by Dagger's KSP processor in a following round.

// In source, AppComponent.kt
@MergeComponent(AppScope::class)
interface AppComponent {
  @Component.Factory
  interface Factory {
    fun create(): AppComponent
  }
}

// Generated by Anvil KSP
@Component(modules = [AppModule::class])
interface AnvilAppComponent : AppComponent, ContributedInterface {
  @Component.Factory
  interface Factory : AppComponent.Factory {
    fun create(): AnvilAppComponent
  }
}

// Generated by Dagger KSP
class DaggerAnvilAppComponent : AnvilAppComponent {
  // ...
}

@Component.Factory and @Component.Builder are both covered.

Since folks usually have a DaggerAppComponent entry point they expect, this also generates a corresponding shim like so, so that theoretically no source changes are needed.

// Source-compatible shim to the new Dagger-generated location
object DaggerAppComponent {
  @JvmStatic
  fun factory() = DaggerAnvilAppComponent.factory()
}

Lastly, this is actually backward compatible with dagger apt as well. It would be not ideal to run both, but given that dagger-ksp is quite slow it may be convenient. All that would need to be done would be for Anvil KSP's contribution merging to run before kapt and its sources be inputs into kapt. Kapt wouldn't know any difference.

Open questions

  • @MergeModules no longer works in this case. That one exclusively relies on modifying sources and it's not clear how we can handle that here. One idea is we could generate a module in a "known" location, such as AppModule -> MergedAppModule and just say people need to include it directly.
  • Similarly, @MergeInterfaces no longer works.
  • There exists an ordering concern. While with IR we could always assume all contribution generation had happened by the time it runs, here we may have contributions happening during the same around and merge too soon. Couple ideas:
    • Check first for any contribution annotations. Do one of two things
      a. Immediately defer the element until the next round (simplest)
      b. Generate anyway, but "know" where and what it will generate to and expect those sources to be generated.
    • Merge all processors into one composite processor, make it run component merging last. This is what dagger does too (I think).

TODO

  • Update tests (will make a separate PR first to set up infra for it)
  • Make sure that subclassing with overrides with subclass's type works as expected

@ZacSweers
Copy link
Collaborator Author

Looks like this pattern of overrides + indirections all work as expected 👍

interface UserComponent {
  interface Factory {
    fun create(): UserComponent
  }

  interface Builder {
    fun input(input: String): Builder
    fun build(): UserComponent
  }
}

interface AnvilUserComponent : UserComponent {
  interface Factory : UserComponent.Factory {
    override fun create(): AnvilUserComponent
  }
  interface Builder : UserComponent.Builder {
    override fun input(input: String): Builder
    override fun build(): AnvilUserComponent
  }
}

class DaggerAnvilUserComponent : AnvilUserComponent {
  companion object {
    fun factory(): AnvilUserComponent.Factory {
      TODO()
    }
    
    fun builder(): AnvilUserComponent.Builder {
      TODO()
    }
  }
}

object DaggerUserComponent {
  fun factory(): UserComponent.Factory {
    return DaggerAnvilUserComponent.factory()
  }
  fun builder(): UserComponent.Builder {
    return DaggerAnvilUserComponent.builder()
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant