Using @Provides + @IntoSet/@IntoMap on a non-static provider method in non-abstract module generates invalid code since 2.14

[Zhuinden]

// Generated by Dagger (https://dagger.dev).
package com.zhuinden.simplestackmultimodule.application.injection;

import com.zhuinden.simplestackmoduleexample.common.ScopedService;
import com.zhuinden.simplestackmoduleexample.navigation.core.BaseKey;
import com.zhuinden.simplestackmoduleexample.navigation.core.ViewFactory;
import com.zhuinden.simplestackmoduleexample.navigation.screens.MainKey;
import com.zhuinden.simplestackmoduleexample.navigation.screens.SplashKey;
import com.zhuinden.simplestackmultimodule.application.CustomApplication;
import com.zhuinden.simplestackmultimodule.application.CustomApplication_MembersInjector;
import com.zhuinden.simplestackmultimodule.application.MainActivity;
import com.zhuinden.simplestackmultimodule.application.MainActivity_MembersInjector;
import com.zhuinden.simplestackmultimodule.application.ServiceProvider;
import com.zhuinden.simplestackmultimodule.feature_main.MainService;
import com.zhuinden.simplestackmultimodule.feature_main.MainServiceModule;
import com.zhuinden.simplestackmultimodule.feature_main.MainServiceModule_ServiceFactory;
import com.zhuinden.simplestackmultimodule.feature_main.MainService_Factory;
import com.zhuinden.simplestackmultimodule.feature_main.MainViewFactory;
import com.zhuinden.simplestackmultimodule.feature_splash.SplashViewFactory;
import dagger.internal.MapBuilder;
import dagger.internal.Preconditions;
import java.util.Collections;
import java.util.Map;
import java.util.Set;
import javax.inject.Provider;

@SuppressWarnings({
    "unchecked",
    "rawtypes"
})
public final class DaggerAppComponent implements AppComponent {
  private final MainServiceModule mainServiceModule;

  private DaggerAppComponent(MainServiceModule mainServiceModuleParam) {
    this.mainServiceModule = mainServiceModuleParam;
  }

  public static Builder builder() {
    return new Builder();
  }

  public static AppComponent create() {
    return new Builder().build();
  }

  private Map<Class<? extends BaseKey>, ViewFactory> getMapOfClassOfAndViewFactory() {
    return MapBuilder.<Class<? extends BaseKey>, ViewFactory>newMapBuilder(2).put(SplashKey.class, new SplashViewFactory()).put(MainKey.class, new MainViewFactory()).build();}

  private Map<Class<? extends ScopedService>, Provider<ScopedService>> getMapOfClassOfAndProviderOfScopedService(
      ) {
    return Collections.<Class<? extends ScopedService>, Provider<ScopedService>>singletonMap(MainService.class, (Provider) MainService_Factory.create());}

  private ServiceProvider getServiceProvider() {
    return new ServiceProvider(getMapOfClassOfAndProviderOfScopedService());}

  private Set<Class<? extends ScopedService>> getNamedSetOfClassOf() {
    return Collections.<Class<? extends ScopedService>>singleton(MainServiceModule_ServiceFactory.service(mainServiceModule));} // ERROR HERE: MainServiceModule_ServiceFactory does not exist

  private Map<Class<? extends BaseKey>, Set<Class<? extends ScopedService>>> getMapOfClassOfAndSetOfClassOf(
      ) {
    return Collections.<Class<? extends BaseKey>, Set<Class<? extends ScopedService>>>singletonMap(MainKey.class, getNamedSetOfClassOf());}

  @Override
  public void inject(MainActivity mainActivity) {
    injectMainActivity(mainActivity);}

  @Override
  public void inject(CustomApplication customApplication) {
    injectCustomApplication(customApplication);}

  private MainActivity injectMainActivity(MainActivity instance) {
    MainActivity_MembersInjector.injectViewFactories(instance, getMapOfClassOfAndViewFactory());
    MainActivity_MembersInjector.injectServiceProvider(instance, getServiceProvider());
    return instance;
  }

  private CustomApplication injectCustomApplication(CustomApplication instance) {
    CustomApplication_MembersInjector.injectKeyMapping(instance, getMapOfClassOfAndSetOfClassOf());
    return instance;
  }

  public static final class Builder {
    private MainServiceModule mainServiceModule;

    private Builder() {
    }

    public Builder mainServiceModule(MainServiceModule mainServiceModule) {
      this.mainServiceModule = Preconditions.checkNotNull(mainServiceModule);
      return this;
    }

    public AppComponent build() {
      if (mainServiceModule == null) {
        this.mainServiceModule = new MainServiceModule();
      }
      return new DaggerAppComponent(mainServiceModule);
    }
  }
}

So the error is at

  private Set<Class<? extends ScopedService>> getNamedSetOfClassOf() {
    return Collections.<Class<? extends ScopedService>>singleton(MainServiceModule_ServiceFactory.service(mainServiceModule));} // ERROR HERE: MainServiceModule_ServiceFactory does not exist

Configuration is tricky but looks like this:

@Module(includes = [MainServiceModule::class])
abstract class MainModule {
    @Binds
    @IntoMap
    @NavigationKey(MainKey::class)
    abstract fun viewFactory(viewFactory: MainViewFactory): ViewFactory

    @Binds
    @IntoMap
    @ServiceKey(MainService::class)
    abstract fun mainService(mainService: MainService): ScopedService

    @Binds
    @IntoMap
    @NavigationKey(MainKey::class)
    abstract fun services(@Named("main") services: @JvmSuppressWildcards Set<Class<out ScopedService>>): @JvmSuppressWildcards Set<Class<out ScopedService>>

    @Binds
    @IntoSet
    @Named("main")
    abstract fun service(clazz: Class<MainService>): Class<out ScopedService>
}

@Module
class MainServiceModule {
    @Provides
    fun service(): Class<MainService> = MainService::class.java
}

where

import dagger.MapKey;

@MapKey
public @interface NavigationKey {
    Class<? extends BaseKey> value();
}

and

import dagger.MapKey;

@MapKey
public @interface ServiceKey {
    Class<? extends ScopedService> value();
}

So I'm trying to set up a multibinding of Map<Class<? extends BaseKey>, Set<Class<? extends ScopedService>>>, but while doing that, I seem to have broken something in Dagger, and now it generates stuff that invokes code that it either didn't generate, or it doesn't exist.

Any ideas what's wrong and how it can be fixed?

Right, component for sake of completion is

@Singleton
@Component(modules = [SplashModule::class, MainModule::class])
interface AppComponent {
    fun inject(mainActivity: MainActivity)
    fun inject(customApplication: CustomApplication)
}

Nothing special. And SplashModule for completion (although I'm pretty sure it is not related as the problem is caused by MainModule):

@Module
abstract class SplashModule {
    @Binds
    @IntoMap
    @NavigationKey(SplashKey::class)
    abstract fun viewFactory(splashViewFactory: SplashViewFactory): ViewFactory
}

There's actually full code available at my dagger-is-dead branch: https://github.com/Zhuinden/simple-stack-multi-module-experiment/tree/bd0ff02487b7dfb8c61ed414c37416c4d1a6c7be

[Zhuinden]

I get the incorrectly generated code from 2.25.4 down to... hey, apparently it compiles with 2.13. What's up with that?

For reference, this is the 2.13 variant (that compiles, and actually works):

// Generated by dagger.internal.codegen.ComponentProcessor (https://google.github.io/dagger).
package com.zhuinden.simplestackmultimodule.application.injection;

import com.zhuinden.simplestackmoduleexample.common.ScopedService;
import com.zhuinden.simplestackmoduleexample.navigation.core.BaseKey;
import com.zhuinden.simplestackmoduleexample.navigation.core.ViewFactory;
import com.zhuinden.simplestackmoduleexample.navigation.screens.MainKey;
import com.zhuinden.simplestackmoduleexample.navigation.screens.SplashKey;
import com.zhuinden.simplestackmultimodule.application.CustomApplication;
import com.zhuinden.simplestackmultimodule.application.CustomApplication_MembersInjector;
import com.zhuinden.simplestackmultimodule.application.MainActivity;
import com.zhuinden.simplestackmultimodule.application.MainActivity_MembersInjector;
import com.zhuinden.simplestackmultimodule.application.ServiceProvider;
import com.zhuinden.simplestackmultimodule.feature_main.MainService;
import com.zhuinden.simplestackmultimodule.feature_main.MainServiceModule;
import com.zhuinden.simplestackmultimodule.feature_main.MainService_Factory;
import com.zhuinden.simplestackmultimodule.feature_main.MainViewFactory;
import com.zhuinden.simplestackmultimodule.feature_splash.SplashViewFactory;
import dagger.internal.MapBuilder;
import dagger.internal.Preconditions;
import java.util.Collections;
import java.util.Map;
import java.util.Set;
import javax.inject.Provider;

public final class DaggerAppComponent implements AppComponent {
  private MainServiceModule mainServiceModule;

  private DaggerAppComponent(Builder builder) {
    initialize(builder);
  }

  public static Builder builder() {
    return new Builder();
  }

  public static AppComponent create() {
    return new Builder().build();
  }

  private Map<Class<? extends BaseKey>, ViewFactory> getMapOfClassOfAndViewFactory() {
    return MapBuilder.<Class<? extends BaseKey>, ViewFactory>newMapBuilder(2)
        .put(SplashKey.class, new SplashViewFactory())
        .put(MainKey.class, new MainViewFactory())
        .build();
  }

  private Map<Class<? extends ScopedService>, Provider<ScopedService>>
      getMapOfClassOfAndProviderOfScopedService() {
    return Collections.<Class<? extends ScopedService>, Provider<ScopedService>>singletonMap(
        MainService.class, (Provider) MainService_Factory.create());
  }

  private ServiceProvider getServiceProvider() {
    return new ServiceProvider(getMapOfClassOfAndProviderOfScopedService());
  }

  private Set<Class<? extends ScopedService>> getNamedSetOfClassOf() {
    return Collections.<Class<? extends ScopedService>>singleton(
        Preconditions.checkNotNull(
            mainServiceModule.service(),
            "Cannot return null from a non-@Nullable @Provides method"));
  }

  private Map<Class<? extends BaseKey>, Set<Class<? extends ScopedService>>>
      getMapOfClassOfAndSetOfClassOf() {
    return Collections.<Class<? extends BaseKey>, Set<Class<? extends ScopedService>>>singletonMap(
        MainKey.class, getNamedSetOfClassOf());
  }

  @SuppressWarnings("unchecked")
  private void initialize(final Builder builder) {
    this.mainServiceModule = builder.mainServiceModule;
  }

  @Override
  public void inject(MainActivity mainActivity) {
    injectMainActivity(mainActivity);
  }

  @Override
  public void inject(CustomApplication customApplication) {
    injectCustomApplication(customApplication);
  }

  private MainActivity injectMainActivity(MainActivity instance) {
    MainActivity_MembersInjector.injectViewFactories(instance, getMapOfClassOfAndViewFactory());
    MainActivity_MembersInjector.injectServiceProvider(instance, getServiceProvider());
    return instance;
  }

  private CustomApplication injectCustomApplication(CustomApplication instance) {
    CustomApplication_MembersInjector.injectKeyMapping(instance, getMapOfClassOfAndSetOfClassOf());
    return instance;
  }

  public static final class Builder {
    private MainServiceModule mainServiceModule;

    private Builder() {}

    public AppComponent build() {
      if (mainServiceModule == null) {
        this.mainServiceModule = new MainServiceModule();
      }
      return new DaggerAppComponent(this);
    }

    public Builder mainServiceModule(MainServiceModule mainServiceModule) {
      this.mainServiceModule = Preconditions.checkNotNull(mainServiceModule);
      return this;
    }
  }
}

[Zhuinden]

I think if it was

  private Set<Class<? extends ScopedService>> getNamedSetOfClassOf() {
    return Collections.<Class<? extends ScopedService>>singleton(mainServiceModule.service());}

instead of

  private Set<Class<? extends ScopedService>> getNamedSetOfClassOf() {
    return Collections.<Class<? extends ScopedService>>singleton(MainServiceModule_ServiceFactory.service(mainServiceModule));}

then it would work.

Why is this MainServiceModule_ServiceFactory generated there, then? :thinking:

[Zhuinden]

So I came to the realization that I can in the meantime add a MainServiceModule_ServiceFactory in my OWN code, so that Dagger would see something by that name.

class MainServiceModule_ServiceFactory {
    companion object {
        @JvmStatic
        fun service(mainServiceModule: MainServiceModule): Class<out ScopedService>
            return mainServiceModule.service()
        }
    }
}

And my code compiles!

Hope you'll fix the bug, though. I'm surprised nobody else ran into it so far.

[Zhuinden]

I just added the MainModule_InitializerFactory that wasn't generated for now:

https://github.com/Zhuinden/simple-stack-multi-module-experiment/commit/5d9c3cdc3a65c29addfc8d9911c1ec0601409944#diff-de1bca180d5e0b068f81ab20c5d1879fR3

[Zhuinden]

Can you guys set up a test like

https://github.com/google/dagger/blob/e923cd8fd1fbefd2c6fadbb6f543c4496c594ce8/javatests/dagger/internal/codegen/MultibindingTest.java#L29-L57

which would validate the

    JavaFileObject module =
        JavaFileObjects.forSourceLines(
            "test.MultibindingModule",
            "package test;",
            "",
            "import dagger.Module;",
            "import dagger.Provides;",
            "import dagger.multibindings.IntoSet;",
            "",
            "@Module",
            "class MultibindingModule {",
            "  @Provides @IntoSet Integer provideInt() { ",
            "    return 1;",
            "  }",
            "}");

that it actually compiles successfully? 😀

I'd love to do it but setting up Bazel seems very complicated.

[ZacSweers]

seems like you should be able to test that yourself in your own project

[Zhuinden]

@ZacSweers only if I inherit whatever they're using to test Dagger. 🤔

[ZacSweers]

That’s a simple test using google’s compile-testing library

[Zhuinden]

@ZacSweers eh, I'll have to be at home with an actual IntelliJ IDEA and a non-Android project, apparently I don't see javax.tools.JavaFileObject and I can't seem to find a way to see it, even in the unit tests. 🤔

Thanks for the tip though, I'll see if I can get to that.

Nonetheless, I found no test to verify the happy path of using a non-static @Provides @Into*** method in a non-abstract module as I was looking through the source code of Dagger. So I'll see when I can run an annotation processor test.

[Zhuinden]

Computing VCS working set... .ijwb/.bazelproject (added)

Querying targets in project directories... Command: "F:\Apps\Bazel 2.0.0\bazel-2.0.0-windows-x86_64.exe" query --tool_tag=ijwb:IDEA:ultimate --output=label_kind --keep_going "attr(\"tags\", \"^((?!manual).)*$\", //...:all - //bazel-bin/...:all - //bazel-genfiles/...:all - //bazel-out/...:all - //bazel-testlogs/...:all - //bazel-dagger/...:all - //.ijwb/...:all)" --

Sync finished Sync failed

Error:Deriving targets from project directories failed.

Lovely, I have no idea how to run this :smile:

As apparently I have no idea how to actually open the Dagger project using Bazel and IntelliJ IDEA, I guess I'll leave that test for someone else.

[Zhuinden]

@danysantiago thank you for tracking the issue 🙂 sorry I couldn't help further with the repro, I got stuck with the Bazel import thing. Never figured it out on my own, unfortunately.

[danysantiago]

Thanks for the repro app!

Wanted to say that another workaround would have been to apply the Dagger processor to your other Gradle modules, adding kapt deps.daggerCompiler here: https://github.com/Zhuinden/simple-stack-multi-module-experiment/blob/dagger-is-dead/feature_main/build.gradle#L32. This is a good practice to do.

We also have a very similar test to the one proposed: https://github.com/google/dagger/blob/e923cd8fd1fbefd2c6fadbb6f543c4496c594ce8/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java#L457 and while I haven't found the cause of the issue it seems to be related to your milti-gradle-module Kotlin project. I'll post an update if I find something.

[bcorso]

I think the issue is that we have an implicit expectation that usages of @Module include both com.google.dagger:dagger and com.google.dagger:dagger-compiler. Specifically, it's currently not just a best practice, it's mandatory.

One option is that we could relax this expectation, and generate the module factory classes when processing the component if one doesn't already exist, similar to what we do for @Inject factory classes. It would be a bit more work, but we could probably make it work.

Another option is to make this expectation explicit by detecting missing factories (while processing the component) and throwing an explicit error telling the user to add the processor to the gradle module containing the @Module.

[ZacSweers]

+1 with the second option, or at least tying it to the same "detect if upstream factory not generated" option

[Zhuinden]

apply the Dagger processor to your other Gradle modules, adding kapt deps.daggerCompiler

Wow, I forgot to add the Dagger-Compiler in the feature modules? :astonished: then it's a wonder it compiled at all with 2.13, and far more esoteric than I thought!

Thanks for investigating/finding the cause! I think if Dagger can throw on such developer error, that would be super useful.

[trevjonez]

I wonder if it would be possible to tell gradle through their module metadata to require the compiler artifact is on kapt/annotationProcessor configuration if the runtime exists on the compile classpath.

doesn't solve it for all downstreams but might be a nice feature to have where possible?