Skip to content

Commit 4941926

Browse files
bcorsoDagger Team
authored andcommitted
Require @nullable explicitly on @BINDS methods.
The way we currently calculate the nullability of @BINDS methods is misleading. In particular, the nullability is based on the nullability of the implementation, and we ignore the nullability on the actual method. This CL makes 2 changes: 1. The nullability of delegate bindings is based on the nullability of the method/return type 2. The nullability of the delegate method/return type must match the nullability of the method parameter. RELNOTES=Require @nullable explictly on @BINDS methods. PiperOrigin-RevId: 673922090
1 parent d02798b commit 4941926

File tree

3 files changed

+58
-4
lines changed

3 files changed

+58
-4
lines changed

java/dagger/internal/codegen/binding/BindingFactory.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,7 @@ private DelegateBinding delegateBinding(
396396
.bindingElement(delegateDeclaration.bindingElement().get())
397397
.contributingModule(delegateDeclaration.contributingModule().get())
398398
.delegateRequest(delegateDeclaration.delegateRequest())
399-
.nullability(
400-
actualBinding.map(ContributionBinding::nullability).orElse(Nullability.NOT_NULLABLE))
399+
.nullability(Nullability.of(delegateDeclaration.bindingElement().get()))
401400
.bindingType(bindingType)
402401
.key(
403402
bindingType == BindingType.PRODUCTION

java/dagger/internal/codegen/validation/BindsMethodValidator.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import dagger.internal.codegen.base.SetType;
3333
import dagger.internal.codegen.binding.BindsTypeChecker;
3434
import dagger.internal.codegen.binding.InjectionAnnotations;
35+
import dagger.internal.codegen.binding.Nullability;
3536
import dagger.internal.codegen.javapoet.TypeNames;
3637
import javax.inject.Inject;
3738

@@ -45,7 +46,6 @@ final class BindsMethodValidator extends BindingMethodValidator {
4546
BindsTypeChecker bindsTypeChecker,
4647
DaggerSuperficialValidation superficialValidation,
4748
XProcessingEnv processingEnv,
48-
4949
DependencyRequestValidator dependencyRequestValidator,
5050
InjectionAnnotations injectionAnnotations) {
5151
super(
@@ -110,6 +110,12 @@ protected void checkParameter(XVariableElement parameter) {
110110
// Set.addAll(Collection<? extends E>)
111111
report.addError("@Binds methods' parameter type must be assignable to the return type");
112112
}
113+
114+
Nullability parameterNullability = Nullability.of(parameter);
115+
Nullability methodNullability = Nullability.of(method);
116+
if (parameterNullability.isNullable() != methodNullability.isNullable()) {
117+
report.addError("@Binds methods' nullability must match the nullability of its parameter");
118+
}
113119
}
114120

115121
private XType boxIfNecessary(XType maybePrimitive) {

javatests/dagger/internal/codegen/BindsMethodValidationTest.java

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,6 @@ public void bindsMissingTypeInParameterHierarchy() {
276276
});
277277
}
278278

279-
280279
@Test
281280
public void bindsMissingTypeInReturnTypeHierarchy() {
282281
Source module =
@@ -342,6 +341,56 @@ public void bindsMissingTypeInReturnTypeHierarchy() {
342341
});
343342
}
344343

344+
@Test
345+
public void bindsNullableToNonNullable_fails() {
346+
Source module =
347+
CompilerTests.javaSource(
348+
"test.TestComponent",
349+
"package test;",
350+
"",
351+
"import dagger.Binds;",
352+
"import dagger.Module;",
353+
"import javax.annotation.Nullable;",
354+
"",
355+
"@Module",
356+
"interface TestModule {",
357+
" @Binds Object bind(@Nullable String str);",
358+
"}");
359+
360+
CompilerTests.daggerCompiler(module)
361+
.compile(
362+
subject -> {
363+
subject.hasErrorCount(1);
364+
subject.hasErrorContaining(
365+
"@Binds methods' nullability must match the nullability of its parameter");
366+
});
367+
}
368+
369+
@Test
370+
public void bindsNonNullableToNullable_fails() {
371+
Source module =
372+
CompilerTests.javaSource(
373+
"test.TestComponent",
374+
"package test;",
375+
"",
376+
"import dagger.Binds;",
377+
"import dagger.Module;",
378+
"import javax.annotation.Nullable;",
379+
"",
380+
"@Module",
381+
"interface TestModule {",
382+
" @Binds @Nullable Object bind(String str);",
383+
"}");
384+
385+
CompilerTests.daggerCompiler(module)
386+
.compile(
387+
subject -> {
388+
subject.hasErrorCount(1);
389+
subject.hasErrorContaining(
390+
"@Binds methods' nullability must match the nullability of its parameter");
391+
});
392+
}
393+
345394
private DaggerModuleMethodSubject assertThatMethod(String method) {
346395
return assertThatModuleMethod(method).withDeclaration(moduleDeclaration);
347396
}

0 commit comments

Comments
 (0)