google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.45k stars 2.02k forks source link

Hilt: compilation error for inherited Fragments #1910

Closed OleksiiMikhieiev closed 4 years ago

OleksiiMikhieiev commented 4 years ago

Compilation fails for the next code:

@AndroidEntryPoint
open class CreateNoteFragment: Fragment() {
    @Inject
    lateinit var someValA: SomeClassA
        ...
}
@AndroidEntryPoint
class EditNoteFragment: CreateNoteFragment() {
    @Inject
    lateinit var someValB: SomeClassB
        ...
}

Compilation error:

error: method does not override or implement a method from a supertype
  @Override

Method from the class where the error occurs:

public abstract class Hilt_EditNoteFragment extends CreateNoteFragment {
  ...
  @Override
  protected void inject() {
    if (!injected) {
      injected = true;
      ((EditNoteFragment_GeneratedInjector) generatedComponent()).injectEditNoteFragment(UnsafeCasts.<EditNoteFragment>unsafeCast(this));
    }
  }

Possible problem: Hilt_CreateNoteFragment is generated for CreateNoteFragment class Hilt_EditNoteFragment is generated for EditNoteFragment class Hilt_EditNoteFragment extends CreateNoteFragment that doesn't have inject() method

FunkyMuse commented 4 years ago
@AndroidEntryPoint
class EditNoteFragment: CreateNoteFragment() {

        ...
}

I think it's safe to remove the @AndroidEntryPoint

Edit: What you're doing is you're putting a class inheritance of two entry points and Dagger gets confused, either move out all the injection in the base fragment and be okay with it or do as the guy below has written a beautiful explanation.

bcorso commented 4 years ago

We should be able to fix this internally. I'll work on getting this fix out shortly, but in the meantime there's the following workarounds.

Workaround

In the meantime, you should be able to work around this using the long-form notation for @AndroidEntryPoint on your base class, e.g.

@AndroidEntryPoint(Fragment.class)
public class CreateNoteFragment extends Hilt_CreateNoteFragment {...}

Edit: @CraZyLegenD's suggestion of removing @AndroidEntryPoint from the base class will also work if you don't need to instantiate CreateNoteFragment directly.

Details:

The issue is that when using the short-form notation for @AndroidEntryPoint, the class hierarchy isn't quite right until after bytecode injection. The class hierarchy for the generated parent class should be:

"Hilt_EditNoteFragment > CreateNoteFragment > Hilt_CreateNoteFragment > Fragment"

But before the bytecode injection it's actually:

"Hilt_EditNoteFragment > CreateNoteFragment > Fragment"

Thus, using the long-form notation for @AndroidEntryPoint on CreateNoteFragment avoids relying on bytecode injection so that your class hierarchy is correct at compile time.

Zhelyazko commented 4 years ago

@bcorso Thanks for the workaround. It does seem to work. Is there a workaround for the case where the chain of inheritance is deeper - e.g. building upon @AlekseyMiheev's example: CreateNoteFragment extends NoteFragment

and NoteFragment extends Fragment

So the complete chain is: EditNoteFragment extends CreateNoteFragment extends NoteFragment extends Fragment

OleksiiMikhieiev commented 4 years ago

@bcorso workaround works for me, thanks. I'm wondering if this case is mentioned somewhere in release notes, documentation, etc.

I think we can close this issue as workaround works.

bcorso commented 4 years ago

@Zhelyazko, for deeper inheritance you can use the same workaround for each base class, i.e. both CreateNoteFragment and NoteFragment would use the long-form notation for @AndroidEntryPoint.

@AlekseyMiheev, I'll be submitting a fix to avoid having to use the workaround (i.e. you should be able to use the short-form of the annotation everywhere). The workaround isn't mentioned in the release notes/documentation because it was more of an oversight than something we intended.

I can close out this issue once that fix is submitted.

FunkyMuse commented 4 years ago

@Zhelyazko, for deeper inheritance you can use the same workaround for each base class, i.e. both CreateNoteFragment and NoteFragment would use the long-form notation for @AndroidEntryPoint.

@AlekseyMiheev, I'll be submitting a fix to avoid having to use the workaround (i.e. you should be able to use the short-form of the annotation everywhere). The workaround isn't mentioned in the release notes/documentation because it was more of an oversight than something we intended.

I can close out this issue once that fix is submitted.

I think it's smart if you can use

@AndroidEntryPoint
abstract class AbstractFragment : Fragment() {

}

and when you have

class TasksFragment : AbstractFragment()

You wouldn't have to do

@AndroidEntryPoint
class TasksFragment : AbstractFragment()
dewunmi commented 4 years ago

I'm experiencing a similar issue but I make use of a BasFragment which takes a generic type T , none of the hacks work for me (T is meant to be a generated binding class)

bcorso commented 4 years ago

@dewunmi can you post the code that isn't working for you, and what failure you are getting?

bcorso commented 4 years ago

@CraZyLegenD, the reason you can't just annotate the base class is because Dagger needs the exact type when doing members injection (See "a note about covariance" section).

For each @AndroidEntryPoint, we generate a new inject method on the component for that type, e.g. void inject(TaskFragment taskFragment) which we use to inject the fragment.

Injecting TaskFragment with void inject(AbstractFragment abstractFragment) would technically compile, but at runtime it would only inject the fields in the base class.

TheGabCode commented 4 years ago

@bcorso The latest release for dagger 2.28.1 does not seem to fix the issue for using @AndroidEntryPoint for fragments with deeper inheritance?? I still get the same error:

  symbol:   method generatedComponent()
  location: class Hilt_FragmentA

I wish I could do the workaround instead but for my case FragmentA : BaseFragmentB: BaseFragmentC, -BaseFragmentB uses some variables and methods from BaseFragmentC, having done the workaround BaseFragmentB : Hilt_BaseFragmentB wouldn't work.

bcorso commented 4 years ago

The latest release for dagger 2.28.1 does not seem to fix the issue for using @AndroidEntryPoint for fragments with deeper inheritance?? I still get the same error:

@TheGabCode Can you reproduce this with a minimal example and post here?

I wish I could do the workaround instead but for my case FragmentA : BaseFragmentB: BaseFragmentC, -BaseFragmentB uses some variables and methods from BaseFragmentC, having done the workaround BaseFragmentB : Hilt_BaseFragmentB wouldn't work.

Can you check the generated class, Hilt_BaseFragmentB. It should extend BaseFragmentC, so the workaround should work, but let me know if that's not the case.

TheGabCode commented 4 years ago

I checked the generated class Hilt_BaseFragmentB and it does extend BaseFragmentC, I might have just misused the @AndroidEntryPoint long-term notation, it now compiles successfully. I wish I can provide a minimal project for replication but not right now, here's how it goes anyway:

I have this subclass @AndroidEntryPoint DiscoverUsersFragment: MainBaseFragment() {}`

and the following super classes

@AndroidEntryPoint MainBaseFragment: BaseFragment() {}`

@AndroidEntryPoint BaseFragment: Fragment() {}`

but right, turns out I was misusing the long-term notation, so this might be non-issue for now. Thanks for the clarification with using the long-term notation.

qrezet commented 4 years ago

is this fix released?

bcorso commented 4 years ago

@qrezet, it should be fixed in the latest release, 2.28.3.

caiofaustino commented 3 years ago

Sorry for resurrecting this but I just wanted to double check if there is any way around my similar issue.

I have MyService which extends LibService from a library I don't control. And I want to annotate MyService with @AndroidEntryPoint.

I'm getting @AndroidEntryPoint class expected to extend Hilt_MyService. Found: LibService

Is there any way for me to annotate the service for injection?

bcorso commented 3 years ago

@caiofaustino

It sounds like you don't have the Hilt Gradle plugin applied to you library.

In particular, without the plugin you would need MyService to directly extend the generated class:

@AndroidEntryPoint(LibService::class)
class MyService: Hilt_MyService {...}

With the plugin, you can extend LibService directly:

@AndroidEntryPoint
class MyService: LibService {...}
caiofaustino commented 2 years ago

Thx for the reply, I was already using the Hilt Gradle Plugin and still getting that error, that's why I was a bit worried that there wouldn't be a way around it. But looks like it was a caching issue of some sort and now it's working as expected. Thanks again.