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] Lint flags generated DialogFragment class with "Error: Use of LayoutInflater.from... detected. Consider using getLayoutInflater() instead" #3222

Closed alvindizon closed 2 years ago

alvindizon commented 2 years ago

I have a class that extends from DialogFragment that is annotated with @AndroidEntryPoint, something like this:

@AndroidEntryPoint
class OurFragment : DialogFragment() {

I upgraded my project's Android Gradle Plugin to7.2.0-alpha06 and Gradle to 7.3.3, and running lintRelease produces this error:

Error: Use of LayoutInflater.from(FragmentComponentManager.createContextWrapper(inflater, this)) detected. Consider using getLayoutInflater() instead [UseGetLayoutInflater from fragment-1.4.1]
    return LayoutInflater.from(FragmentComponentManager.createContextWrapper(inflater, this));
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

   Explanation for issues of type "UseGetLayoutInflater":
   Using LayoutInflater.from(Context) can return a LayoutInflater             
       that does not have the correct theme.

   Vendor: Android Open Source Project (fragment-1.4.1)
   Identifier: fragment-1.4.1
   Feedback: https://issuetracker.google.com/issues/new?component=192731

Checking the generated class, I can see this:

/**
 * A generated base class to be extended by the @dagger.hilt.android.AndroidEntryPoint annotated class. If using the Gradle plugin, this is swapped as the base class via bytecode transformation.
 */
public abstract class Hilt_OurCardFragment extends DialogFragment implements GeneratedComponentManagerHolder {
   ...
  @Override
  public LayoutInflater onGetLayoutInflater(Bundle savedInstanceState) {
    LayoutInflater inflater = super.onGetLayoutInflater(savedInstanceState);
    return LayoutInflater.from(FragmentComponentManager.createContextWrapper(inflater, this));
  }

I'm using version 2.40.5 of Dagger btw. I also have other classes that extend from Fragment, ~but they do not get flagged by lint, even if their generated classes also use LayoutInflater.from(FragmentComponentManager.createContextWrapper(inflater, this))~. Now I'm not sure this is the correct place to bring this up, feel free to close this if this is not appropriate, but I'd like to bring this to your attention as I'm not sure how to handle this. I'm thinking of temporarily silencing this error via adding this issue to lint.xml.

edit: apologies, other classes extending from Fragment also trigger this lint error. I have silenced this error by adding the UseGetLayoutInflater issue in our lint.xml.

Chang-Eric commented 2 years ago

Thanks for the report on this. I think we can suppress this in the generated code by adding a @SuppressLint annotation. In this case, unless you are seeing bugs with the theme, suppressing seems like the right call since we are implementing getLayoutInflater() to sneak our Hilt context wrapper in so we can't do what the lint suggests and call it.

alvindizon commented 2 years ago

Hi @Chang-Eric , thanks for your response. Apologies but I don't quite understand what you're saying about adding the annotation--we can't add it to generated code, or are you talking about annotating my Fragment both with @SuppressLint and @AndroidEntryPoint?

Chang-Eric commented 2 years ago

I meant I will submit a change to Hilt to add it =)

alvindizon commented 2 years ago

Ah got it, @Chang-Eric thanks :)

Chang-Eric commented 2 years ago

This will be fixed when that is change is released. It ended up that instead of suppressing the warning, we should have been using cloneInContext(). Thanks for bringing this to our attention!