Skip to content

Commit b1aca4d

Browse files
Oleksandr Gumencgruber
Oleksandr Gumen
authored and
cgruber
committed
Fix a bug that causing invalid code generated when using multiple AutoFactories implementing same interface.
------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=92862023
1 parent 2f7ceda commit b1aca4d

File tree

5 files changed

+184
-57
lines changed

5 files changed

+184
-57
lines changed

‎factory/src/main/java/com/google/auto/factory/processor/AutoFactoryProcessor.java‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.common.collect.ImmutableList;
2525
import com.google.common.collect.ImmutableListMultimap;
2626
import com.google.common.collect.ImmutableSet;
27+
import com.google.common.collect.ImmutableSetMultimap;
2728
import com.google.common.collect.ImmutableSortedSet;
2829
import com.google.common.collect.Iterables;
2930
import com.google.common.collect.Multimaps;
@@ -97,25 +98,30 @@ private void doProcess(RoundEnvironment roundEnv) {
9798

9899
ImmutableListMultimap.Builder<String, FactoryMethodDescriptor> indexedMethods =
99100
ImmutableListMultimap.builder();
100-
ImmutableSet.Builder<ImplementationMethodDescriptor> implementationMethodDescriptors =
101-
ImmutableSet.builder();
101+
ImmutableSetMultimap.Builder<String, ImplementationMethodDescriptor>
102+
implementationMethodDescriptorsBuilder = ImmutableSetMultimap.builder();
102103
for (Element element : roundEnv.getElementsAnnotatedWith(AutoFactory.class)) {
103104
Optional<AutoFactoryDeclaration> declaration = declarationFactory.createIfValid(element);
104105
if (declaration.isPresent()) {
106+
String factoryName = declaration.get().getFactoryName(
107+
elements.getPackageOf(element).getQualifiedName(),
108+
getAnnotatedType(element).getSimpleName());
109+
105110
TypeElement extendingType = declaration.get().extendingType();
106111
List<ExecutableElement> supertypeMethods =
107112
ElementFilter.methodsIn(elements.getAllMembers(extendingType));
108113
for (ExecutableElement supertypeMethod : supertypeMethods) {
109114
if (supertypeMethod.getModifiers().contains(Modifier.ABSTRACT)) {
110115
ExecutableType methodType = Elements2.getExecutableElementAsMemberOf(
111116
types, supertypeMethod, extendingType);
112-
implementationMethodDescriptors.add(new ImplementationMethodDescriptor.Builder()
113-
.name(supertypeMethod.getSimpleName().toString())
114-
.returnType(getAnnotatedType(element).getQualifiedName().toString())
115-
.publicMethod()
116-
.passedParameters(Parameter.forParameterList(
117-
supertypeMethod.getParameters(), methodType.getParameterTypes()))
118-
.build());
117+
implementationMethodDescriptorsBuilder.put(factoryName,
118+
new ImplementationMethodDescriptor.Builder()
119+
.name(supertypeMethod.getSimpleName().toString())
120+
.returnType(getAnnotatedType(element).getQualifiedName().toString())
121+
.publicMethod()
122+
.passedParameters(Parameter.forParameterList(
123+
supertypeMethod.getParameters(), methodType.getParameterTypes()))
124+
.build());
119125
}
120126
}
121127
for (TypeElement implementingType : declaration.get().implementingTypes()) {
@@ -125,13 +131,14 @@ private void doProcess(RoundEnvironment roundEnv) {
125131
if (interfaceMethod.getModifiers().contains(Modifier.ABSTRACT)) {
126132
ExecutableType methodType = Elements2.getExecutableElementAsMemberOf(
127133
types, interfaceMethod, implementingType);
128-
implementationMethodDescriptors.add(new ImplementationMethodDescriptor.Builder()
129-
.name(interfaceMethod.getSimpleName().toString())
130-
.returnType(getAnnotatedType(element).getQualifiedName().toString())
131-
.publicMethod()
132-
.passedParameters(Parameter.forParameterList(
133-
interfaceMethod.getParameters(), methodType.getParameterTypes()))
134-
.build());
134+
implementationMethodDescriptorsBuilder.put(factoryName,
135+
new ImplementationMethodDescriptor.Builder()
136+
.name(interfaceMethod.getSimpleName().toString())
137+
.returnType(getAnnotatedType(element).getQualifiedName().toString())
138+
.publicMethod()
139+
.passedParameters(Parameter.forParameterList(
140+
interfaceMethod.getParameters(), methodType.getParameterTypes()))
141+
.build());
135142
}
136143
}
137144
}
@@ -147,6 +154,9 @@ private void doProcess(RoundEnvironment roundEnv) {
147154
}));
148155
}
149156

157+
ImmutableSetMultimap<String, ImplementationMethodDescriptor>
158+
implementationMethodDescriptors = implementationMethodDescriptorsBuilder.build();
159+
150160
for (Entry<String, Collection<FactoryMethodDescriptor>> entry
151161
: indexedMethods.build().asMap().entrySet()) {
152162
ImmutableSet.Builder<String> extending = ImmutableSet.builder();
@@ -180,8 +190,7 @@ private void doProcess(RoundEnvironment roundEnv) {
180190
implementing.build(),
181191
publicType,
182192
ImmutableSet.copyOf(entry.getValue()),
183-
// TODO(gak): this needs to be indexed too
184-
implementationMethodDescriptors.build(),
193+
implementationMethodDescriptors.get(entry.getKey()),
185194
allowSubclasses));
186195
} catch (IOException e) {
187196
messager.printMessage(Kind.ERROR, "failed");

‎factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java‎

Lines changed: 53 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
package com.google.auto.factory.processor;
1717

18-
import static com.google.common.truth.Truth.assert_;
18+
import static com.google.common.truth.Truth.assertAbout;
1919
import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource;
2020
import static com.google.testing.compile.JavaSourcesSubjectFactory.javaSources;
2121

@@ -34,15 +34,15 @@
3434
@RunWith(JUnit4.class)
3535
public class AutoFactoryProcessorTest {
3636
@Test public void simpleClass() {
37-
assert_().about(javaSource())
37+
assertAbout(javaSource())
3838
.that(JavaFileObjects.forResource("good/SimpleClass.java"))
3939
.processedWith(new AutoFactoryProcessor())
4040
.compilesWithoutError()
4141
.and().generatesSources(JavaFileObjects.forResource("expected/SimpleClassFactory.java"));
4242
}
4343

4444
@Test public void simpleClassNonFinal() {
45-
assert_().about(javaSource())
45+
assertAbout(javaSource())
4646
.that(JavaFileObjects.forResource("good/SimpleClassNonFinal.java"))
4747
.processedWith(new AutoFactoryProcessor())
4848
.compilesWithoutError()
@@ -51,23 +51,23 @@ public class AutoFactoryProcessorTest {
5151
}
5252

5353
@Test public void publicClass() {
54-
assert_().about(javaSource())
54+
assertAbout(javaSource())
5555
.that(JavaFileObjects.forResource("good/PublicClass.java"))
5656
.processedWith(new AutoFactoryProcessor())
5757
.compilesWithoutError()
5858
.and().generatesSources(JavaFileObjects.forResource("expected/PublicClassFactory.java"));
5959
}
6060

6161
@Test public void simpleClassCustomName() {
62-
assert_().about(javaSource())
62+
assertAbout(javaSource())
6363
.that(JavaFileObjects.forResource("good/SimpleClassCustomName.java"))
6464
.processedWith(new AutoFactoryProcessor())
6565
.compilesWithoutError()
6666
.and().generatesSources(JavaFileObjects.forResource("expected/CustomNamedFactory.java"));
6767
}
6868

6969
@Test public void simpleClassMixedDeps() {
70-
assert_().about(javaSources())
70+
assertAbout(javaSources())
7171
.that(ImmutableSet.of(
7272
JavaFileObjects.forResource("good/SimpleClassMixedDeps.java"),
7373
JavaFileObjects.forResource("support/AQualifier.java")))
@@ -78,7 +78,7 @@ public class AutoFactoryProcessorTest {
7878
}
7979

8080
@Test public void simpleClassPassedDeps() {
81-
assert_().about(javaSource())
81+
assertAbout(javaSource())
8282
.that(JavaFileObjects.forResource("good/SimpleClassPassedDeps.java"))
8383
.processedWith(new AutoFactoryProcessor())
8484
.compilesWithoutError()
@@ -87,7 +87,7 @@ public class AutoFactoryProcessorTest {
8787
}
8888

8989
@Test public void simpleClassProvidedDeps() {
90-
assert_().about(javaSources())
90+
assertAbout(javaSources())
9191
.that(ImmutableSet.of(
9292
JavaFileObjects.forResource("support/AQualifier.java"),
9393
JavaFileObjects.forResource("support/BQualifier.java"),
@@ -99,7 +99,7 @@ public class AutoFactoryProcessorTest {
9999
}
100100

101101
@Test public void constructorAnnotated() {
102-
assert_().about(javaSource())
102+
assertAbout(javaSource())
103103
.that(JavaFileObjects.forResource("good/ConstructorAnnotated.java"))
104104
.processedWith(new AutoFactoryProcessor())
105105
.compilesWithoutError()
@@ -108,7 +108,7 @@ public class AutoFactoryProcessorTest {
108108
}
109109

110110
@Test public void constructorAnnotatedNonFinal() {
111-
assert_().about(javaSource())
111+
assertAbout(javaSource())
112112
.that(JavaFileObjects.forResource("good/ConstructorAnnotatedNonFinal.java"))
113113
.processedWith(new AutoFactoryProcessor())
114114
.compilesWithoutError()
@@ -117,7 +117,7 @@ public class AutoFactoryProcessorTest {
117117
}
118118

119119
@Test public void simpleClassImplementingMarker() {
120-
assert_().about(javaSource())
120+
assertAbout(javaSource())
121121
.that(JavaFileObjects.forResource("good/SimpleClassImplementingMarker.java"))
122122
.processedWith(new AutoFactoryProcessor())
123123
.compilesWithoutError()
@@ -126,7 +126,7 @@ public class AutoFactoryProcessorTest {
126126
}
127127

128128
@Test public void simpleClassImplementingSimpleInterface() {
129-
assert_().about(javaSource())
129+
assertAbout(javaSource())
130130
.that(JavaFileObjects.forResource("good/SimpleClassImplementingSimpleInterface.java"))
131131
.processedWith(new AutoFactoryProcessor())
132132
.compilesWithoutError()
@@ -135,7 +135,7 @@ public class AutoFactoryProcessorTest {
135135
}
136136

137137
@Test public void mixedDepsImplementingInterfaces() {
138-
assert_().about(javaSource())
138+
assertAbout(javaSource())
139139
.that(JavaFileObjects.forResource("good/MixedDepsImplementingInterfaces.java"))
140140
.processedWith(new AutoFactoryProcessor())
141141
.compilesWithoutError()
@@ -145,31 +145,31 @@ public class AutoFactoryProcessorTest {
145145

146146
@Test public void failsWithMixedFinals() {
147147
JavaFileObject file = JavaFileObjects.forResource("bad/MixedFinals.java");
148-
assert_().about(javaSource())
149-
.that(file)
150-
.processedWith(new AutoFactoryProcessor())
151-
.failsToCompile()
152-
.withErrorContaining(
153-
"Cannot mix allowSubclasses=true and allowSubclasses=false in one factory.")
154-
.in(file).onLine(25).atColumn(3)
155-
.and().withErrorContaining(
156-
"Cannot mix allowSubclasses=true and allowSubclasses=false in one factory.")
157-
.in(file).onLine(26).atColumn(3);
148+
assertAbout(javaSource())
149+
.that(file)
150+
.processedWith(new AutoFactoryProcessor())
151+
.failsToCompile()
152+
.withErrorContaining(
153+
"Cannot mix allowSubclasses=true and allowSubclasses=false in one factory.")
154+
.in(file).onLine(25).atColumn(3)
155+
.and().withErrorContaining(
156+
"Cannot mix allowSubclasses=true and allowSubclasses=false in one factory.")
157+
.in(file).onLine(26).atColumn(3);
158158
}
159159

160160
@Test public void failsOnGenericClass() {
161161
JavaFileObject file = JavaFileObjects.forResource("bad/GenericClass.java");
162-
assert_().about(javaSource())
163-
.that(file)
164-
.processedWith(new AutoFactoryProcessor())
165-
.failsToCompile()
166-
.withErrorContaining("AutoFactory does not support generic types")
167-
.in(file).onLine(21).atColumn(14);
162+
assertAbout(javaSource())
163+
.that(file)
164+
.processedWith(new AutoFactoryProcessor())
165+
.failsToCompile()
166+
.withErrorContaining("AutoFactory does not support generic types")
167+
.in(file).onLine(21).atColumn(14);
168168
}
169169

170170
@Test public void providedButNoAutoFactory() {
171171
JavaFileObject file = JavaFileObjects.forResource("bad/ProvidedButNoAutoFactory.java");
172-
assert_().about(javaSource())
172+
assertAbout(javaSource())
173173
.that(file)
174174
.processedWith(new AutoFactoryProcessor())
175175
.failsToCompile()
@@ -180,7 +180,7 @@ public class AutoFactoryProcessorTest {
180180

181181
@Test public void providedOnMethodParameter() {
182182
JavaFileObject file = JavaFileObjects.forResource("bad/ProvidedOnMethodParameter.java");
183-
assert_().about(javaSource())
183+
assertAbout(javaSource())
184184
.that(file)
185185
.processedWith(new AutoFactoryProcessor())
186186
.failsToCompile()
@@ -191,7 +191,7 @@ public class AutoFactoryProcessorTest {
191191

192192
@Test public void invalidCustomName() {
193193
JavaFileObject file = JavaFileObjects.forResource("bad/InvalidCustomName.java");
194-
assert_().about(javaSource())
194+
assertAbout(javaSource())
195195
.that(file)
196196
.processedWith(new AutoFactoryProcessor())
197197
.failsToCompile()
@@ -200,7 +200,7 @@ public class AutoFactoryProcessorTest {
200200
}
201201

202202
@Test public void factoryExtendingAbstractClass() {
203-
assert_().about(javaSource())
203+
assertAbout(javaSource())
204204
.that(JavaFileObjects.forResource("good/FactoryExtendingAbstractClass.java"))
205205
.processedWith(new AutoFactoryProcessor())
206206
.compilesWithoutError()
@@ -211,7 +211,7 @@ public class AutoFactoryProcessorTest {
211211
@Test public void factoryExtendingAbstractClass_withConstructorParams() {
212212
JavaFileObject file =
213213
JavaFileObjects.forResource("good/FactoryExtendingAbstractClassWithConstructorParams.java");
214-
assert_().about(javaSource())
214+
assertAbout(javaSource())
215215
.that(file)
216216
.processedWith(new AutoFactoryProcessor())
217217
.failsToCompile()
@@ -225,15 +225,15 @@ public class AutoFactoryProcessorTest {
225225
@Test public void factoryExtendingAbstractClass_multipleConstructors() {
226226
JavaFileObject file = JavaFileObjects.forResource(
227227
"good/FactoryExtendingAbstractClassWithMultipleConstructors.java");
228-
assert_().about(javaSource())
228+
assertAbout(javaSource())
229229
.that(file)
230230
.processedWith(new AutoFactoryProcessor())
231231
.compilesWithoutError();
232232
}
233233

234234
@Test public void factoryExtendingInterface() {
235235
JavaFileObject file = JavaFileObjects.forResource("bad/InterfaceSupertype.java");
236-
assert_().about(javaSource())
236+
assertAbout(javaSource())
237237
.that(file)
238238
.processedWith(new AutoFactoryProcessor())
239239
.failsToCompile()
@@ -244,7 +244,7 @@ public class AutoFactoryProcessorTest {
244244

245245
@Test public void factoryExtendingEnum() {
246246
JavaFileObject file = JavaFileObjects.forResource("bad/EnumSupertype.java");
247-
assert_().about(javaSource())
247+
assertAbout(javaSource())
248248
.that(file)
249249
.processedWith(new AutoFactoryProcessor())
250250
.failsToCompile()
@@ -256,7 +256,7 @@ public class AutoFactoryProcessorTest {
256256

257257
@Test public void factoryExtendingFinalClass() {
258258
JavaFileObject file = JavaFileObjects.forResource("bad/FinalSupertype.java");
259-
assert_().about(javaSource())
259+
assertAbout(javaSource())
260260
.that(file)
261261
.processedWith(new AutoFactoryProcessor())
262262
.failsToCompile()
@@ -268,11 +268,25 @@ public class AutoFactoryProcessorTest {
268268
@Test public void factoryImplementingGenericInterfaceExtension() {
269269
JavaFileObject file =
270270
JavaFileObjects.forResource("good/FactoryImplementingGenericInterfaceExtension.java");
271-
assert_().about(javaSource())
271+
assertAbout(javaSource())
272272
.that(file)
273273
.processedWith(new AutoFactoryProcessor())
274274
.compilesWithoutError()
275275
.and().generatesSources(JavaFileObjects.forResource(
276276
"expected/FactoryImplementingGenericInterfaceExtension.java"));
277277
}
278+
279+
@Test public void multipleFactoriesImpementingInterface() {
280+
JavaFileObject file =
281+
JavaFileObjects.forResource("good/MultipleFactoriesImplementingInterface.java");
282+
assertAbout(javaSource())
283+
.that(file)
284+
.processedWith(new AutoFactoryProcessor())
285+
.compilesWithoutError()
286+
.and().generatesSources(
287+
JavaFileObjects.forResource(
288+
"expected/MultipleFactoriesImplementingInterfaceAFactory.java"),
289+
JavaFileObjects.forResource(
290+
"expected/MultipleFactoriesImplementingInterfaceBFactory.java"));
291+
}
278292
}

‎factory/src/test/resources/expected/MultipleFactoriesImplementingInterfaceAFactory.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright (C) 2015 Google, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package tests;
17+
18+
import javax.annotation.Generated;
19+
import javax.inject.Inject;
20+
import tests.MultipleFactoriesImplementingInterface.Base.Factory;
21+
22+
@Generated("com.google.auto.factory.processor.AutoFactoryProcessor")
23+
final class MultipleFactoriesImplementingInterfaceAFactory implements Factory {
24+
@Inject
25+
MultipleFactoriesImplementingInterfaceAFactory() {}
26+
27+
MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceA create() {
28+
return new MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceA();
29+
}
30+
31+
@Override
32+
public MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceA
33+
abstractNonDefaultCreate() {
34+
return create();
35+
}
36+
}

0 commit comments

Comments
 (0)