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

Hilt: compilation error for inherited Fragments #1910

Closed
OleksiiMikhieiev opened this issue Jun 12, 2020 · 17 comments · Fixed by #1936
Closed

Hilt: compilation error for inherited Fragments #1910

OleksiiMikhieiev opened this issue Jun 12, 2020 · 17 comments · Fixed by #1936
Assignees

Comments

@OleksiiMikhieiev
Copy link

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
Copy link

FunkyMuse commented Jun 12, 2020

@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
Copy link

bcorso commented Jun 12, 2020

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.

@bcorso bcorso self-assigned this Jun 13, 2020
@Zhelyazko
Copy link

@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
Copy link
Author

@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
Copy link

bcorso commented Jun 15, 2020

@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
Copy link

@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
Copy link

dewunmi commented Jun 17, 2020

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
Copy link

bcorso commented Jun 17, 2020

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

@bcorso
Copy link

bcorso commented Jun 17, 2020

@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.

nick-someone pushed a commit that referenced this issue Jun 17, 2020
…en using plugin.

This fixes a bug when using the plugin where the child generated class uses `@Overrides void inject()` which fails because that method doesn't exist in an ancestor until after bytecode injection.

Fixes #1910

RELNOTES=Fix #1910: Allow usage of an @androidentrypoint base class when using short-form notation with the hilt gradle plugin.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=316891087
nick-someone pushed a commit that referenced this issue Jun 18, 2020
…en using plugin.

This fixes a bug when using the plugin where the child generated class uses `@Overrides void inject()` which fails because that method doesn't exist in an ancestor until after bytecode injection.

Fixes #1910

RELNOTES=Fix #1910: Allow usage of an @androidentrypoint base class when using short-form notation with the hilt gradle plugin.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=316891087
nick-someone pushed a commit that referenced this issue Jun 18, 2020
…en using plugin.

This fixes a bug when using the plugin where the child generated class uses `@Overrides void inject()` which fails because that method doesn't exist in an ancestor until after bytecode injection.

Fixes #1910

RELNOTES=Fix #1910: Allow usage of an @androidentrypoint base class when using short-form notation with the hilt gradle plugin.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=316891087
@TheGabCode
Copy link

TheGabCode commented Jun 23, 2020

@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
Copy link

bcorso commented Jun 23, 2020

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
Copy link

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
Copy link

qrezet commented Aug 3, 2020

is this fix released?

@bcorso
Copy link

bcorso commented Aug 3, 2020

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

@caiofaustino
Copy link

caiofaustino commented Nov 18, 2021

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
Copy link

bcorso commented Nov 18, 2021

@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
Copy link

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.

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 a pull request may close this issue.

8 participants