Skip to content

Commit

Permalink
Report an error for binding methods that have more than one scope ann…
Browse files Browse the repository at this point in the history 
…otation, instead of throwing an exception.

RELNOTES=Report an error for binding methods that have more than one scope annotation, instead of throwing an exception.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197897221
  • Loading branch information
@netdpb @ronshapiro
netdpb authored and ronshapiro committed May 25, 2018
1 parent 462e654 commit 192d359
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 17 deletions.
14 changes: 14 additions & 0 deletions java/dagger/internal/codegen/BindingMethodValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static dagger.internal.codegen.DaggerElements.isAnyAnnotationPresent;
import static dagger.internal.codegen.InjectionAnnotations.getQualifiers;
import static dagger.internal.codegen.MapKeys.getMapKeys;
import static dagger.internal.codegen.Scopes.scopesOf;
import static dagger.internal.codegen.Util.reentrantComputeIfAbsent;
import static java.util.stream.Collectors.joining;
import static javax.lang.model.element.Modifier.ABSTRACT;
Expand All @@ -34,6 +35,7 @@
import com.google.errorprone.annotations.OverridingMethodsMustInvokeSuper;
import dagger.MapKey;
import dagger.Provides;
import dagger.model.Scope;
import dagger.multibindings.ElementsIntoSet;
import dagger.multibindings.IntoMap;
import dagger.producers.Produces;
Expand Down Expand Up @@ -150,6 +152,7 @@ protected void checkMethod(ValidationReport.Builder<ExecutableElement> builder)
checkQualifiers(builder);
checkMapKeys(builder);
checkMultibindings(builder);
checkScopes(builder);
}

/**
Expand Down Expand Up @@ -360,6 +363,17 @@ protected void checkMultibindings(ValidationReport.Builder<ExecutableElement> bu
}
}

/** Adds an error if the method has more than one {@linkplain Scope scope} annotation. */
protected void checkScopes(ValidationReport.Builder<ExecutableElement> builder) {
ImmutableSet<Scope> scopes = scopesOf(builder.getSubject());
if (scopes.size() > 1) {
for (Scope scope : scopes) {
builder.addError(
"Cannot use more than one @Scope", builder.getSubject(), scope.scopeAnnotation());
}
}
}

/** Adds an error if the method returns a {@linkplain FrameworkTypes framework type}. */
protected void checkFrameworkType(ValidationReport.Builder<ExecutableElement> builder) {
if (FrameworkTypes.isFrameworkType(builder.getSubject().getReturnType())) {
Expand Down
4 changes: 2 additions & 2 deletions java/dagger/internal/codegen/ProducesMethodValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ final class ProducesMethodValidator extends BindingMethodValidator {
protected void checkMethod(ValidationReport.Builder<ExecutableElement> builder) {
super.checkMethod(builder);
checkNullable(builder);
checkScope(builder);
}

/** Adds a warning if a {@link Produces @Produces} method is declared nullable. */
Expand All @@ -68,7 +67,8 @@ private void checkNullable(ValidationReport.Builder<ExecutableElement> builder)
}

/** Adds an error if a {@link Produces @Produces} method has a scope annotation. */
private void checkScope(ValidationReport.Builder<ExecutableElement> builder) {
@Override
protected void checkScopes(ValidationReport.Builder<ExecutableElement> builder) {
if (!scopesOf(builder.getSubject()).isEmpty()) {
builder.addError("@Produces methods may not have scope annotations");
}
Expand Down
82 changes: 67 additions & 15 deletions javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,21 +380,6 @@ public void providesMethodReturnsProduced() {
.and().generatesSources(factoryFile);
}

private static final JavaFileObject QUALIFIER_A =
JavaFileObjects.forSourceLines("test.QualifierA",
"package test;",
"",
"import javax.inject.Qualifier;",
"",
"@Qualifier @interface QualifierA {}");
private static final JavaFileObject QUALIFIER_B =
JavaFileObjects.forSourceLines("test.QualifierB",
"package test;",
"",
"import javax.inject.Qualifier;",
"",
"@Qualifier @interface QualifierB {}");

@Test public void multipleProvidesMethods() {
JavaFileObject classXFile = JavaFileObjects.forSourceLines("test.X",
"package test;",
Expand Down Expand Up @@ -1298,6 +1283,24 @@ public void genericSubclassedModule() {
provideNonGenericTypeWithDepsFactory);
}

private static final JavaFileObject QUALIFIER_A =
JavaFileObjects.forSourceLines(
"test.QualifierA",
"package test;",
"",
"import javax.inject.Qualifier;",
"",
"@Qualifier @interface QualifierA {}");

private static final JavaFileObject QUALIFIER_B =
JavaFileObjects.forSourceLines(
"test.QualifierB",
"package test;",
"",
"import javax.inject.Qualifier;",
"",
"@Qualifier @interface QualifierB {}");

@Test public void providesMethodMultipleQualifiers() {
JavaFileObject moduleFile = JavaFileObjects.forSourceLines("test.TestModule",
"package test;",
Expand All @@ -1318,6 +1321,55 @@ public void genericSubclassedModule() {
assertThat(compilation).hadErrorContaining("Cannot use more than one @Qualifier");
}

private static final JavaFileObject SCOPE_A =
JavaFileObjects.forSourceLines(
"test.ScopeA",
"package test;",
"",
"import javax.inject.Scope;",
"",
"@Scope @interface ScopeA {}");

private static final JavaFileObject SCOPE_B =
JavaFileObjects.forSourceLines(
"test.ScopeB",
"package test;",
"",
"import javax.inject.Scope;",
"",
"@Scope @interface ScopeB {}");

@Test
public void providesMethodMultipleScopes() {
JavaFileObject moduleFile =
JavaFileObjects.forSourceLines(
"test.TestModule",
"package test;",
"",
"import dagger.Module;",
"import dagger.Provides;",
"",
"@Module",
"final class TestModule {",
" @Provides",
" @ScopeA",
" @ScopeB",
" String provideString() {",
" return \"foo\";",
" }",
"}");
Compilation compilation = daggerCompiler().compile(moduleFile, SCOPE_A, SCOPE_B);
assertThat(compilation).failed();
assertThat(compilation)
.hadErrorContaining("Cannot use more than one @Scope")
.inFile(moduleFile)
.onLineContaining("@ScopeA");
assertThat(compilation)
.hadErrorContaining("Cannot use more than one @Scope")
.inFile(moduleFile)
.onLineContaining("@ScopeB");
}

@Test public void providerDependsOnProduced() {
JavaFileObject moduleFile = JavaFileObjects.forSourceLines("test.TestModule",
"package test;",
Expand Down

0 comments on commit 192d359

Please sign in to comment.