Support extending base builder classes

[vRallev]

This example for factories compiles fine:

interface BaseComponent {
  fun factory(): Factory

  interface Factory {
    fun create(@BindsInstance int: Int): BaseComponent
  }
}

@Subcomponent
interface SomeComponent : BaseComponent {
  override fun factory(): SomeComponent.Factory

  @Subcomponent.Factory
  interface Factory : BaseComponent.Factory {
    override fun create(@BindsInstance int: Int): SomeComponent
  }
}

Notice how I override all the base classes to the concrete types.

Changing the Factory to a Builder doesn't work and I don't see a reason why Dagger shouldn't support it:

interface BaseComponent {
  fun builder(): Builder

  interface Builder {
    @BindsInstance fun charSequence(sequence: CharSequence): Builder
    fun build(): BaseComponent
  }
}

@Subcomponent
interface SomeComponent : BaseComponent {
  override fun builder(): SomeComponent.Builder

  @Subcomponent.Builder
  interface Builder : BaseComponent.Builder {
    @BindsInstance override fun charSequence(sequence: CharSequence): SomeComponent.Builder
    override fun build(): SomeComponent
  }
}

Dagger throws several different errors depending on other changes. The first I run into is:

[Dagger/MissingBinding] java.lang.CharSequence cannot be provided without an @Provides-annotated method.

After stopping to rely on the binding I see this error:

DaggerParentComponent.SomeComponentBuilder is not abstract and does not override abstract method charSequence(CharSequence) in Builder
  private final class SomeComponentBuilder implements SomeComponent.Builder {

[ZacSweers]

I think you need to write it like this, which is a little gross but covers everything

interface BaseComponent {
  fun builder(): Builder

  interface Builder<T : BaseComponent, B : Builder<T, B> {
    @BindsInstance fun charSequence(sequence: CharSequence): B
    fun build(): T
  }
}

@Subcomponent
interface SomeComponent : BaseComponent {
  override fun builder(): SomeComponent.Builder

  @Subcomponent.Builder
  interface Builder : BaseComponent.Builder<SomeComponent, Builder>
}

[vRallev]

More testing and it turns out my issue was something else. First issue, I used builder as method name, which Dagger accepts and doesn't complain about, but javac complains later.

@Component
interface ParentComponent {
  public fun builder(): MySubcomponent.Builder
}

I changed it to builder2() and it worked.

The second problem was that I had this method in my Builder class

@BindsInstance public fun int(int: Int): Builder

This seems to trigger an error in Dagger and it'll complain about a missing binding. Changing the method name to anything else like int124() fixes the issue.

After making these changes my initial sample works:

@Component
public interface TestAbcComponent {
  public fun createBuilder(): SomeComponent.Builder
}

public interface BaseComponent {
  public interface Builder {
    @BindsInstance public fun charSequence(sequence: CharSequence): Builder
    public fun build(): BaseComponent
  }
}

@Subcomponent
public interface SomeComponent : BaseComponent {
  @Subcomponent.Builder
  public interface Builder : BaseComponent.Builder {
    @BindsInstance override fun charSequence(sequence: CharSequence): SomeComponent.Builder
    override fun build(): SomeComponent
  }
}

[bcorso]

First issue, I used builder as method name, which Dagger accepts and doesn't complain about, but javac complains later.

Yep, the Dagger component always has a static builder and (when applicable) a static create method. I agree we should fail if users try to use these names, and I'm surprised we don't already have this check.

This seems to trigger an error in Dagger and it'll complain about a missing binding. Changing the method name to anything else like int124() fixes the issue.

I tested this, and the issue is that kapt will just skip the int() method when generating the stub, likely because it's not valid java. Thus, in your case it ends up as a missing binding because the @BindsInstance is completely missing when Dagger processes the component. It looks like the information about the missing int() method is at least in the KotlinMetadata, so we could probably try to detect this and throw a more reasonable error.

[vRallev]

That makes sense, thanks for taking a quick look. The title of this issue is now a little confusing. Do you want me to create new tickets for these two issues or do you have it covered now?

[bcorso]

Separate issues sounds great, if you don't mind. Thanks!

[vRallev]

Done #2575 and #2576.