Skip to content

Commit f60dcca

Browse files
bcorsoDagger Team
authored andcommitted
Fix issue with DaggerSuperficialValidation#validateTypeHierarchyOf().
This CL fixes an issue with `DaggerSuperficialValidation#validateTypeHierarchyOf()` that can affect `@Binds` method validation. In particular, the issue is that we don't recursively validate the type hierarchy for the type arguments. For example, if `class Foo extends Bar<Baz>` and `Baz` extends an invalid type (e.g. `class Baz extends MissingType`) then we previously didn't catch this case and we assumed the type was valid. This causes issues with `@Binds` validation because if there's an invalid type in the hierarchy the `KSType#isAssignableFrom(KSType)` check will be incorrect. RELNOTES=Fix issue with DaggerSuperficialValidation#validateTypeHierarchyOf(). PiperOrigin-RevId: 808650035
1 parent 9319e81 commit f60dcca

File tree

2 files changed

+117
-3
lines changed

2 files changed

+117
-3
lines changed

dagger-compiler/main/java/dagger/internal/codegen/base/DaggerSuperficialValidation.java

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,15 @@
3939
import static dagger.internal.codegen.xprocessing.XExecutableTypes.getKindName;
4040
import static dagger.internal.codegen.xprocessing.XExecutableTypes.isMethodType;
4141
import static dagger.internal.codegen.xprocessing.XTypes.asArray;
42+
import static dagger.internal.codegen.xprocessing.XTypes.asTypeVariable;
4243
import static dagger.internal.codegen.xprocessing.XTypes.getKindName;
4344
import static dagger.internal.codegen.xprocessing.XTypes.isDeclared;
4445
import static dagger.internal.codegen.xprocessing.XTypes.isTypeOf;
46+
import static dagger.internal.codegen.xprocessing.XTypes.isTypeVariable;
4547
import static dagger.internal.codegen.xprocessing.XTypes.isWildcard;
4648

4749
import androidx.room.compiler.codegen.XClassName;
50+
import androidx.room.compiler.codegen.XTypeName;
4851
import androidx.room.compiler.processing.XAnnotation;
4952
import androidx.room.compiler.processing.XAnnotationValue;
5053
import androidx.room.compiler.processing.XElement;
@@ -68,8 +71,10 @@
6871
import dagger.internal.codegen.xprocessing.XTypes;
6972
import java.util.ArrayList;
7073
import java.util.Collection;
74+
import java.util.HashSet;
7175
import java.util.List;
7276
import java.util.Optional;
77+
import java.util.Set;
7378
import javax.inject.Inject;
7479

7580
/**
@@ -230,16 +235,32 @@ public void validateAnnotationOf(XElement element, XAnnotation annotation) {
230235
*/
231236
public void validateTypeHierarchyOf(String typeDescription, XElement element, XType type) {
232237
try {
233-
validateTypeHierarchy(typeDescription, type);
238+
validateTypeHierarchy(typeDescription, type, new HashSet<>());
234239
} catch (RuntimeException exception) {
235240
throw ValidationException.from(exception).append(element);
236241
}
237242
}
238243

239-
private void validateTypeHierarchy(String desc, XType type) {
244+
private void validateTypeHierarchy(String desc, XType type, Set<XTypeName> visited) {
245+
if (!visited.add(type.asTypeName())) {
246+
return;
247+
}
240248
validateType(desc, type);
241249
try {
242-
type.getSuperTypes().forEach(supertype -> validateTypeHierarchy("supertype", supertype));
250+
if (isArray(type)) {
251+
validateTypeHierarchy("array component type", asArray(type).getComponentType(), visited);
252+
} else if (isDeclared(type)) {
253+
type.getTypeArguments()
254+
.forEach(typeArg -> validateTypeHierarchy("type argument", typeArg, visited));
255+
type.getSuperTypes()
256+
.forEach(supertype -> validateTypeHierarchy("supertype", supertype, visited));
257+
} else if (isWildcard(type) && type.extendsBound() != null) {
258+
validateTypeHierarchy("extends bound type", type.extendsBound(), visited);
259+
} else if (isTypeVariable(type)) {
260+
asTypeVariable(type)
261+
.getUpperBounds()
262+
.forEach(bound -> validateTypeHierarchy("type variable bound type", bound, visited));
263+
}
243264
} catch (RuntimeException exception) {
244265
throw ValidationException.from(exception).append(desc, type);
245266
}

javatests/dagger/internal/codegen/DaggerSuperficialValidationTest.java

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,99 @@ public void invalidSupertypeInInterface() {
707707
});
708708
}
709709

710+
@Test
711+
public void invalidSuperInterfaceInTypeHierarchy() {
712+
runTest(
713+
CompilerTests.javaSource(
714+
"test.Foo",
715+
"package test;",
716+
"class Foo extends Bar<Baz> {}",
717+
"class Bar<T> {}",
718+
"class Baz extends MissingType {}"),
719+
CompilerTests.kotlinSource(
720+
"test.Foo.kt",
721+
"package test;",
722+
"class Foo : Bar<Baz> {}",
723+
"open class Bar<T> {}",
724+
"class Baz : MissingType {}"),
725+
(processingEnv, superficialValidation) -> {
726+
XTypeElement foo = processingEnv.findTypeElement("test.Foo");
727+
if (isKAPT(processingEnv)) {
728+
// https://youtrack.jetbrains.com/issue/KT-34193/Kapt-CorrectErrorTypes-doesnt-work-for-generics
729+
// There's no way to work around this bug in KAPT so validation doesn't catch this case.
730+
superficialValidation.validateTypeHierarchyOf("type", foo, foo.getType());
731+
return;
732+
}
733+
ValidationException exception =
734+
assertThrows(
735+
ValidationException.KnownErrorType.class,
736+
() -> superficialValidation.validateTypeHierarchyOf("type", foo, foo.getType()));
737+
assertThat(exception)
738+
.hasMessageThat()
739+
.contains(
740+
NEW_LINES.join(
741+
"Validation trace:",
742+
" => element (CLASS): test.Foo",
743+
" => type (DECLARED type): test.Foo",
744+
" => type (DECLARED supertype): test.Bar<test.Baz>",
745+
" => type (DECLARED type argument): test.Baz",
746+
" => type (ERROR supertype): MissingType"));
747+
});
748+
}
749+
750+
@Test
751+
public void invalidSuperInterfaceInArrayTypeHierarchy() {
752+
runTest(
753+
CompilerTests.javaSource(
754+
"test.Outer",
755+
"package test;",
756+
"",
757+
"final class Outer {",
758+
" static class Foo extends Bar<Baz[]> {}",
759+
" static class Bar<T> {}",
760+
" static class Baz extends MissingType {}",
761+
" Foo getFoo() { return null; }",
762+
"}"),
763+
CompilerTests.kotlinSource(
764+
"test.Outer.kt",
765+
"package test;",
766+
"",
767+
"class Outer {",
768+
" class Foo : Bar<Array<Baz>> {}",
769+
" open class Bar<T> {}",
770+
" class Baz : MissingType {}",
771+
" fun getFoo(): Foo = TODO()",
772+
"}"),
773+
(processingEnv, superficialValidation) -> {
774+
XTypeElement outerElement = processingEnv.findTypeElement("test.Outer");
775+
XMethodElement getFooMethod = outerElement.getDeclaredMethods().get(0);
776+
if (isKAPT(processingEnv)) {
777+
// https://youtrack.jetbrains.com/issue/KT-34193/Kapt-CorrectErrorTypes-doesnt-work-for-generics
778+
// There's no way to work around this bug in KAPT so validation doesn't catch this case.
779+
superficialValidation.validateTypeHierarchyOf(
780+
"return type", getFooMethod, getFooMethod.getReturnType());
781+
return;
782+
}
783+
ValidationException exception =
784+
assertThrows(
785+
ValidationException.KnownErrorType.class,
786+
() ->
787+
superficialValidation.validateTypeHierarchyOf(
788+
"return type", getFooMethod, getFooMethod.getReturnType()));
789+
final String expectedTrace =
790+
NEW_LINES.join(
791+
"Validation trace:",
792+
" => element (CLASS): test.Outer",
793+
" => element (METHOD): getFoo()",
794+
" => type (DECLARED return type): test.Outer.Foo",
795+
" => type (DECLARED supertype): test.Outer.Bar<test.Outer.Baz[]>",
796+
" => type (ARRAY type argument): test.Outer.Baz[]",
797+
" => type (DECLARED array component type): test.Outer.Baz",
798+
" => type (ERROR supertype): MissingType");
799+
assertThat(exception).hasMessageThat().contains(expectedTrace);
800+
});
801+
}
802+
710803
private void runTest(
711804
Source.JavaSource javaSource,
712805
Source.KotlinSource kotlinSource,

0 commit comments

Comments
 (0)