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

AssistedInject for a type with generic parameter produces code that doesn't compile #2279

Open
lwasyl opened this issue Jan 15, 2021 · 15 comments

Comments

@lwasyl
Copy link

lwasyl commented Jan 15, 2021

Dagger 2.31
The following class generates code that doesn't compile:

package com.example

import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject

class AssistedInjectRepro<T> @AssistedInject constructor(
    @Assisted obj: T,
) {

    @AssistedFactory
    interface Factory {

        fun <T> create(
            obj: T,
        ): AssistedInjectRepro<T>
    }
}

generated code:

// Generated by Dagger (https://dagger.dev).
package com.example;

import dagger.internal.InstanceFactory;
import javax.inject.Provider;

@SuppressWarnings({
    "unchecked",
    "rawtypes"
})
public class AssistedInjectRepro_Factory_Impl implements AssistedInjectRepro.Factory {
  private final AssistedInjectRepro_Factory<T> delegateFactory;

  AssistedInjectRepro_Factory_Impl(AssistedInjectRepro_Factory<T> delegateFactory) {
    this.delegateFactory = delegateFactory;
  }

  @Override
  public <T> AssistedInjectRepro<T> create(T obj) {
    return delegateFactory.get(obj);
  }

  public static <T> Provider<AssistedInjectRepro.Factory> create(
      AssistedInjectRepro_Factory<T> delegateFactory) {
    return InstanceFactory.create(new AssistedInjectRepro_Factory_Impl(delegateFactory));
  }
}

which fails with

> Task :app:kaptDebugKotlin FAILED
...generated/source/kapt/debug/com/example/AssistedInjectRepro_Factory_Impl.java:12: error: cannot find symbol
  private final AssistedInjectRepro_Factory<T> delegateFactory;
                                            ^
  symbol:   class T
  location: class AssistedInjectRepro_Factory_Impl/.../build/generated/source/kapt/debug/com/example/AssistedInjectRepro_Factory_Impl.java:14: error: cannot find symbol
  AssistedInjectRepro_Factory_Impl(AssistedInjectRepro_Factory<T> delegateFactory) {
                                                               ^
  symbol:   class T
  location: class AssistedInjectRepro_Factory_Impl
FAILURE: Build failed with an exception.
@bcorso
Copy link

bcorso commented Jan 16, 2021

@lwasyl, thanks for the repro case!

I agree we should support this. I should be able to get a fix out soon.

@bcorso
Copy link

bcorso commented Jan 20, 2021

Btw, now that I've had some time to look into this, just wanted to clarify some of the restrictions I think we'll have to make with this feature.

We would only allow this feature if the type parameter is not used as a binding (i.e not used in constructor/field/method injection) within the assisted inject class. As an example, take the assisted inject class shown below

final class Foo<T1, T2, T3, T4, T5> {
  @Inject T1 t1;
  private final T2 t2;
  private final T3 t3;
  private final T3 assistedT3;
  private final T4 assistedT4;
  private T5 t5; // e.g. this can be set via a setter method

  @AssistedInject
  Foo(T2 t2, T3 t3, @Assisted T3 assistedT3, @Assisted T4 assistedT4) {
     this.t2 = t2;
     this.t3 = t3;
     this.assistedT3 = assistedT3;
     this.assistedT4 = assistedT4;
  }
}

In this case, T1, T2, and T3 are used as bindings and must be declared directly on the factory type. However, T4 and T5 are not used as bindings, so they can be defined either on the factory type or the factory method. Thus, the factories below are valid factory definitions:

// This is a valid factory definition
@AssistedFactory
interface FooFactory<T1, T2, T3, T4, T5> {
  Foo<T1, T2, T3, T4, T5> create(T3 assistedT3, T4 assistedT4);
}

// This is a valid factory definition
@AssistedFactory
interface FooFactory<T1, T2, T3> {
  <T4, T5> Foo<T1, T2, T3, T4, T5>  create(T3 assistedT3, T4 assistedT4);
}

However, the factory below is not a valid factory because even though T3 is used as in an assisted type, it is also used as a binding:

// This is an **invalid** factory definition
@AssistedFactory
interface FooFactory<T1, T2> {
  <T3, T4, T5> Foo<T1, T2, T3, T4, T5> create(T3 assistedT3, T4 assistedT4);
}

@bcorso bcorso self-assigned this Jan 22, 2021
@bcorso
Copy link

bcorso commented Jan 28, 2021

@lwasyl, just a heads up that we're still considering this but it's unlikely it'll be in the next release.

For now, I'm going to add some validation in our processor so that at least we'll catch this and fail fast with a better error message rather than generating invalid java.

copybara-service bot pushed a commit that referenced this issue Jan 28, 2021
…er with the @AssistedFactory creator method.

We're considering supporting this feature in the future, but until then we're adding a better error message.

See #2279

RELNOTES=Issue #2279: Adds better error message when trying to use a type parameter with @AssistedFactory creator method.
PiperOrigin-RevId: 354324578
copybara-service bot pushed a commit that referenced this issue Jan 28, 2021
…er with the @AssistedFactory creator method.

We're considering supporting this feature in the future, but until then we're adding a better error message.

See #2279

RELNOTES=Issue #2279: Adds better error message when trying to use a type parameter with @AssistedFactory creator method.
PiperOrigin-RevId: 354389636
@gusuly0rum
Copy link

Hello, I tried using the above suggestions but I am still getting an error.
I am using the 2.40.1 dagger-hilt version, has this feature been released?

FunViewModelFactory.java:7: error: [ViewModelFactory.create(T)]
@AssistedFactory does not currently support type parameters in the creator method.
See https://github.com/google/dagger/issues/2279
public abstract interface FunViewModelFactory extends ViewModelFactory<FunViewModel> {
                ^
@AssistedFactory
interface FunViewModelFactory : ViewModelFactory<FunViewModel>

interface ViewModelFactory<VM : ViewModel> {
    fun <T> create(navArgs: T? = null): VM
}

@bcorso
Copy link

bcorso commented Nov 19, 2021

@gusuly0rum which suggestion are you referring to?

Unfortunately, I haven't had time to get to this yet, so your code is failing because this feature doesn't exist yet.

@gusuly0rum
Copy link

@bcorso I was referring to your example with the valid and invalid factory examples.
Ah I see, are there any workarounds we can use in the meantime?

@bcorso
Copy link

bcorso commented Nov 19, 2021

All of the the type arguments must currently be specified at the time @AssistedFactory is defined. Thus, you would first have to move navArgs type argument onto the interface, and then specify the type when defining FunViewModelFactory. For example:

@AssistedFactory
interface FunViewModelFactory : ViewModelFactory<FunViewModel, FunNavArgs>

interface ViewModelFactory<VM : ViewModel, NavArgsT> {
    fun create(navArgs: NavArgsT? = null): VM
}

@D-Soft-Tech
Copy link

@bcorso I also ran into the same error, it will be good if this issue is fixed ASAP. Because this will give enough freedom in working with Hilt. Now I have to my rewrite my project di from scratch using Dagger.

@bcorso
Copy link

bcorso commented Feb 7, 2022

Because this will give enough freedom in working with Hilt. Now I have to my rewrite my project di from scratch using Dagger.

Hi @D-Soft-Tech,

Can you give more details about your use case? In particular, I'd like to better understand why not having this feature would cause you to have to rewrite your project's DI from scratch.

@h908714124
Copy link

h908714124 commented Mar 13, 2022

Hi, I think I might have a somewhat motivating use case. In class MappingFinder, the goal is to make the generic parameter <M extends AnnotatedMethod> M sourceMethod an @AssistedInject constructor parameter. Currently sourceMethod is simply an argument of findMapping.

Note: MappingFinder would become a generic class with type argument M, and it would drop the scoping annotation @ValidateScope.

@h908714124
Copy link

h908714124 commented Mar 26, 2022

MappingFactory might be a better example, because it is already using @AssistedInject. One of its inputs is Match<M> match, but since this has a type parameter, it cannot currently be supplied via constructor. It has to be passed as a method parameter to checkMatchingMatch, and then passed through as a parameter again to the private toMapping method.

@FunkyMuse
Copy link

Any updates on this?

@fdoyle
Copy link

fdoyle commented May 10, 2023

I have a use case for something that produces this error message, I think it's in scope for this ticket.

I have some object, MyScreenState, which represents a UI state, and which implements a number of interfaces. Translator, Toaster, things like that.

I've got a separate shared UI state component called FarmPickerUIFlow. I want it to implement some of those interfaces by its host, which I'm passing in via an assisted constructor.

Ideally, I'd be able to do something like

@AssistedFactory
interface FarmPickerUIFlowFactory {
    fun <T> create(host: T): FarmPickerUIFlow where T : TakePicture, T : Survey
}
class FarmPickerUIFlow<T> @AssistedInject constructor(
    val host: T,
    @Assisted farms: FarmPickerUseCase,
) : Survey by host,
    TakePicture by StandardTakePicture(), KotlinSyntaxFix where T : Survey, T : TakePicture  {

(KotlinSyntaxFix is an empty interface necessary because apparently kotlin doesn't like when you put a by clause next to a where clause, but it's perfectly happy when there's something in between)

My only option currently is to have separate parameters for each of the interfaces I want to implement. That's not ideal, since there's a few more than two, and it's all just the same object being passed in.

@dholzenburg
Copy link

I also would like to use a generic type parameter with @AssistedInject, but in a different way. I would like to use:

@AssistedFactory
interface SomeTypeFactory<N> {

	@Override
	<W> SomeTypeImpl<N, W> create(final SomeOtherType<N, W> someOther);
}

This gives an error during generation of the code in version 2.50 (and maybe others), referencing to this issue.

As a workaround, I tried with wildcards as well:

@AssistedFactory
interface SomeTypeFactory<N> {

	@Override
	SomeTypeImpl<N, ?> create(final SomeOtherType<N, ?> someOther);
}

This will successfully generate the code, but will fail during compilation. Now I'm a bit stuck here.

Is there any chance to get stuff like this working besides not using @AssistedInject?

@DHosseiny
Copy link

this future can be useful if get implemented.

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

No branches or pull requests

10 participants