Skip to content

Commit

Permalink
Allow Hilt view constructor to contain non-declared types.
Browse files Browse the repository at this point in the history 
This CL fixes an issue in ViewGenerator#isRestrictedApiConstructor() where we were assuming that the parameter type could be represented by a TypeElement. To fix this, we now first check that the parameter type is a TypeKind.DECLARED type before getting a TypeElement for it.

Fixes #3296

RELNOTES=Allow Hilt view constructor to contain non-declared types.
PiperOrigin-RevId: 434522808
  • Loading branch information
@bcorso
bcorso authored and Dagger Team committed Mar 14, 2022
1 parent c0608e3 commit dc76e82
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 11 deletions.
26 changes: 15 additions & 11 deletions java/dagger/hilt/android/processor/internal/androidentrypoint/ViewGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package dagger.hilt.android.processor.internal.androidentrypoint;

import static com.google.auto.common.MoreTypes.asTypeElement;

import com.google.auto.common.MoreElements;
import com.google.auto.common.Visibility;
import com.squareup.javapoet.AnnotationSpec;
Expand All @@ -29,7 +31,6 @@
import java.io.IOException;
import java.util.List;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeKind;
Expand Down Expand Up @@ -117,6 +118,11 @@ private MethodSpec constructorMethod(ExecutableElement constructorElement) {
MethodSpec.Builder constructor =
Generators.copyConstructor(constructorElement).toBuilder();

// TODO(b/210544481): Once this bug is fixed we should require that the user adds this
// annotation to their constructor and we'll propagate it from there rather than trying to
// guess whether this needs @TargetApi from the signature. This check is a bit flawed. For
// example, the user could write a 5 parameter constructor that calls the restricted 4 parameter
// constructor and we would miss adding @TargetApi to it.
if (isRestrictedApiConstructor(constructorElement)) {
// 4 parameter constructors are only available on @TargetApi(21).
constructor.addAnnotation(
Expand All @@ -136,15 +142,14 @@ private boolean isRestrictedApiConstructor(ExecutableElement constructor) {
List<? extends VariableElement> constructorParams = constructor.getParameters();
for (int i = 0; i < constructorParams.size(); i++) {
TypeMirror type = constructorParams.get(i).asType();
Element element = env.getTypeUtils().asElement(type);
switch (i) {
case 0:
if (!isFirstRestrictedParameter(element)) {
if (!isFirstRestrictedParameter(type)) {
return false;
}
break;
case 1:
if (!isSecondRestrictedParameter(element)) {
if (!isSecondRestrictedParameter(type)) {
return false;
}
break;
Expand Down Expand Up @@ -176,15 +181,14 @@ private static boolean isThirdRestrictedParameter(TypeMirror type) {
&& Processors.getPrimitiveType(type).getKind() == TypeKind.INT;
}

private static boolean isSecondRestrictedParameter(Element element) {
return MoreElements.isType(element)
&& Processors.isAssignableFrom(
MoreElements.asType(element), AndroidClassNames.ATTRIBUTE_SET);
private static boolean isSecondRestrictedParameter(TypeMirror type) {
return type.getKind() == TypeKind.DECLARED
&& Processors.isAssignableFrom(asTypeElement(type), AndroidClassNames.ATTRIBUTE_SET);
}

private static boolean isFirstRestrictedParameter(Element element) {
return MoreElements.isType(element)
&& Processors.isAssignableFrom(MoreElements.asType(element), AndroidClassNames.CONTEXT);
private static boolean isFirstRestrictedParameter(TypeMirror type) {
return type.getKind() == TypeKind.DECLARED
&& Processors.isAssignableFrom(asTypeElement(type), AndroidClassNames.CONTEXT);
}

private boolean isInOurPackage(ExecutableElement constructorElement) {
Expand Down
111 changes: 111 additions & 0 deletions javatests/dagger/hilt/android/processor/internal/GeneratorsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,117 @@ public void copyConstructorParametersConvertsAndroidInternalNullableToExternal()
"}"));
}

// This is a regression test for https://github.com/google/dagger/issues/3296
@Test
public void isRestrictedApiConstructorWithPrimitiveParameterTest() {
JavaFileObject baseView =
JavaFileObjects.forSourceLines(
"test.BaseView",
"package test;",
"",
"import android.content.Context;",
"import android.util.AttributeSet;",
"import android.view.View;",
"",
"public class BaseView extends View {",
" public BaseView(int i, int j, Context context, AttributeSet attrs) {",
" super(context, attrs);",
" }",
"}");
JavaFileObject myView =
JavaFileObjects.forSourceLines(
"test.MyView",
"package test;",
"",
"import android.content.Context;",
"import android.util.AttributeSet;",
"import android.view.View;",
"import dagger.hilt.android.AndroidEntryPoint;",
"",
"@AndroidEntryPoint(BaseView.class)",
"public class MyView extends Hilt_MyView {",
" public MyView(int i, int j, Context context, AttributeSet attrs) {",
" super(i, j, context, attrs);",
" }",
"}");
Compilation compilation = compiler().compile(baseView, myView);
assertThat(compilation).succeeded();
}

// This is a regression test for https://github.com/google/dagger/issues/3296
@Test
public void isRestrictedApiConstructorWithArrayParameterTest() {
JavaFileObject baseView =
JavaFileObjects.forSourceLines(
"test.BaseView",
"package test;",
"",
"import android.content.Context;",
"import android.util.AttributeSet;",
"import android.view.View;",
"",
"public class BaseView extends View {",
" public BaseView(String[] strs, int i, Context context, AttributeSet attrs) {",
" super(context, attrs);",
" }",
"}");
JavaFileObject myView =
JavaFileObjects.forSourceLines(
"test.MyView",
"package test;",
"",
"import android.content.Context;",
"import android.util.AttributeSet;",
"import android.view.View;",
"import dagger.hilt.android.AndroidEntryPoint;",
"",
"@AndroidEntryPoint(BaseView.class)",
"public class MyView extends Hilt_MyView {",
" public MyView(String[] strs, int i, Context context, AttributeSet attrs) {",
" super(strs, i, context, attrs);",
" }",
"}");
Compilation compilation = compiler().compile(baseView, myView);
assertThat(compilation).succeeded();
}

// This is a regression test for https://github.com/google/dagger/issues/3296
@Test
public void isRestrictedApiConstructorWithTypeParameterTest() {
JavaFileObject baseView =
JavaFileObjects.forSourceLines(
"test.BaseView",
"package test;",
"",
"import android.content.Context;",
"import android.util.AttributeSet;",
"import android.view.View;",
"",
"public class BaseView<T> extends View {",
" public BaseView(T t, int i, Context context, AttributeSet attrs) {",
" super(context, attrs);",
" }",
"}");
JavaFileObject myView =
JavaFileObjects.forSourceLines(
"test.MyView",
"package test;",
"",
"import android.content.Context;",
"import android.util.AttributeSet;",
"import android.view.View;",
"import dagger.hilt.android.AndroidEntryPoint;",
"",
"@AndroidEntryPoint(BaseView.class)",
"public class MyView extends Hilt_MyView<String> {",
" public MyView(String str, int i, Context context, AttributeSet attrs) {",
" super(str, i, context, attrs);",
" }",
"}");
Compilation compilation = compiler().compile(baseView, myView);
assertThat(compilation).succeeded();
}

@Test
public void copyTargetApiAnnotationActivity() {
JavaFileObject myActivity =
Expand Down

0 comments on commit dc76e82

Please sign in to comment.