Skip to content

Commit 46718eb

Browse files
netdpbronshapiro
authored andcommitted
Fix bug where ProcessingStep.process(...) was called with too many elements when there had been deferred elements.
RELNOTES=Fix bug where `ProcessingStep.process(...)` was called with too many elements when there had been deferred elements. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=218513715
1 parent 66a57ec commit 46718eb

File tree

2 files changed

+64
-16
lines changed

2 files changed

+64
-16
lines changed

common/src/main/java/com/google/auto/common/BasicAnnotationProcessor.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ private void process(ImmutableSetMultimap<Class<? extends Annotation>, Element>
321321
for (ProcessingStep step : steps) {
322322
ImmutableSetMultimap<Class<? extends Annotation>, Element> stepElements =
323323
new ImmutableSetMultimap.Builder<Class<? extends Annotation>, Element>()
324-
.putAll(indexByAnnotation(elementsDeferredBySteps.get(step)))
324+
.putAll(indexByAnnotation(elementsDeferredBySteps.get(step), step.annotations()))
325325
.putAll(filterKeys(validElements, Predicates.<Object>in(step.annotations())))
326326
.build();
327327
if (stepElements.isEmpty()) {
@@ -343,15 +343,14 @@ public ElementName apply(Element element) {
343343
}
344344

345345
private ImmutableSetMultimap<Class<? extends Annotation>, Element> indexByAnnotation(
346-
Set<ElementName> annotatedElements) {
347-
ImmutableSet<? extends Class<? extends Annotation>> supportedAnnotationClasses =
348-
getSupportedAnnotationClasses();
346+
Set<ElementName> annotatedElements,
347+
Set<? extends Class<? extends Annotation>> annotationClasses) {
349348
ImmutableSetMultimap.Builder<Class<? extends Annotation>, Element> deferredElements =
350349
ImmutableSetMultimap.builder();
351350
for (ElementName elementName : annotatedElements) {
352351
Optional<? extends Element> element = elementName.getElement(elements);
353352
if (element.isPresent()) {
354-
findAnnotatedElements(element.get(), supportedAnnotationClasses, deferredElements);
353+
findAnnotatedElements(element.get(), annotationClasses, deferredElements);
355354
}
356355
}
357356
return deferredElements.build();
@@ -377,7 +376,7 @@ private ImmutableSetMultimap<Class<? extends Annotation>, Element> indexByAnnota
377376
*/
378377
private static void findAnnotatedElements(
379378
Element element,
380-
ImmutableSet<? extends Class<? extends Annotation>> annotationClasses,
379+
Set<? extends Class<? extends Annotation>> annotationClasses,
381380
ImmutableSetMultimap.Builder<Class<? extends Annotation>, Element> annotatedElements) {
382381
for (Element enclosedElement : element.getEnclosedElements()) {
383382
if (!enclosedElement.getKind().isClass() && !enclosedElement.getKind().isInterface()) {

common/src/test/java/com/google/auto/common/BasicAnnotationProcessorTest.java

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,24 @@
1515
*/
1616
package com.google.auto.common;
1717

18+
import static com.google.common.collect.Multimaps.transformValues;
1819
import static com.google.common.truth.Truth.assertAbout;
1920
import static com.google.common.truth.Truth.assertThat;
2021
import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource;
2122
import static com.google.testing.compile.JavaSourcesSubjectFactory.javaSources;
22-
import static java.lang.annotation.ElementType.TYPE;
2323
import static javax.tools.StandardLocation.SOURCE_OUTPUT;
2424

2525
import com.google.common.collect.ImmutableList;
2626
import com.google.common.collect.ImmutableSet;
27+
import com.google.common.collect.ImmutableSetMultimap;
2728
import com.google.common.collect.SetMultimap;
29+
import com.google.common.truth.Correspondence;
2830
import com.google.testing.compile.JavaFileObjects;
2931
import java.io.IOException;
3032
import java.io.PrintWriter;
3133
import java.lang.annotation.Annotation;
3234
import java.lang.annotation.Retention;
3335
import java.lang.annotation.RetentionPolicy;
34-
import java.lang.annotation.Target;
3536
import java.util.Set;
3637
import javax.annotation.processing.Filer;
3738
import javax.lang.model.SourceVersion;
@@ -51,9 +52,11 @@ public class BasicAnnotationProcessorTest {
5152
/**
5253
* Rejects elements unless the class generated by {@link GeneratesCode}'s processor is present.
5354
*/
54-
public class RequiresGeneratedCodeProcessor extends BasicAnnotationProcessor {
55+
private static final class RequiresGeneratedCodeProcessor extends BasicAnnotationProcessor {
5556

56-
private int rejectedRounds;
57+
int rejectedRounds;
58+
final ImmutableList.Builder<ImmutableSetMultimap<Class<? extends Annotation>, Element>>
59+
processArguments = ImmutableList.builder();
5760

5861
@Override
5962
public SourceVersion getSupportedSourceVersion() {
@@ -67,6 +70,7 @@ protected Iterable<? extends ProcessingStep> initSteps() {
6770
@Override
6871
public Set<Element> process(
6972
SetMultimap<Class<? extends Annotation>, Element> elementsByAnnotation) {
73+
processArguments.add(ImmutableSetMultimap.copyOf(elementsByAnnotation));
7074
TypeElement requiredClass =
7175
processingEnv.getElementUtils().getTypeElement("test.SomeGeneratedClass");
7276
if (requiredClass == null) {
@@ -81,8 +85,24 @@ public Set<Element> process(
8185
public Set<? extends Class<? extends Annotation>> annotations() {
8286
return ImmutableSet.of(RequiresGeneratedCode.class);
8387
}
88+
},
89+
new ProcessingStep() {
90+
@Override
91+
public Set<? extends Class<? extends Annotation>> annotations() {
92+
return ImmutableSet.of(AnAnnotation.class);
93+
}
94+
95+
@Override
96+
public Set<? extends Element> process(
97+
SetMultimap<Class<? extends Annotation>, Element> elementsByAnnotation) {
98+
return ImmutableSet.of();
99+
}
84100
});
85101
}
102+
103+
ImmutableList<ImmutableSetMultimap<Class<? extends Annotation>, Element>> processArguments() {
104+
return processArguments.build();
105+
}
86106
}
87107

88108
@Retention(RetentionPolicy.SOURCE)
@@ -115,7 +135,6 @@ public Set<? extends Class<? extends Annotation>> annotations() {
115135
}
116136
}
117137

118-
@Target(TYPE)
119138
public @interface AnAnnotation {}
120139

121140
/** When annotating a type {@code Foo}, generates a class called {@code FooXYZ}. */
@@ -250,11 +269,16 @@ public void properlyDefersProcessing_nestedTypeValidBeforeOuterType() {
250269

251270
@Test
252271
public void properlyDefersProcessing_rejectsElement() {
253-
JavaFileObject classAFileObject = JavaFileObjects.forSourceLines("test.ClassA",
254-
"package test;",
255-
"",
256-
"@" + RequiresGeneratedCode.class.getCanonicalName(),
257-
"public class ClassA {}");
272+
JavaFileObject classAFileObject =
273+
JavaFileObjects.forSourceLines(
274+
"test.ClassA",
275+
"package test;",
276+
"",
277+
"@" + RequiresGeneratedCode.class.getCanonicalName(),
278+
"public class ClassA {",
279+
" @" + AnAnnotation.class.getCanonicalName(),
280+
" public void method() {}",
281+
"}");
258282
JavaFileObject classBFileObject = JavaFileObjects.forSourceLines("test.ClassB",
259283
"package test;",
260284
"",
@@ -270,6 +294,31 @@ public void properlyDefersProcessing_rejectsElement() {
270294
.generatesFileNamed(
271295
SOURCE_OUTPUT, "test", "GeneratedByRequiresGeneratedCodeProcessor.java");
272296
assertThat(requiresGeneratedCodeProcessor.rejectedRounds).isEqualTo(1);
297+
298+
// Re b/118372780: Assert that the right deferred elements are passed back, and not any enclosed
299+
// elements annotated with annotations from a different step.
300+
assertThat(requiresGeneratedCodeProcessor.processArguments())
301+
.comparingElementsUsing(setMultimapValuesByString())
302+
.containsExactly(
303+
ImmutableSetMultimap.of(RequiresGeneratedCode.class, "test.ClassA"),
304+
ImmutableSetMultimap.of(RequiresGeneratedCode.class, "test.ClassA"))
305+
.inOrder();
306+
}
307+
308+
private static <K, V>
309+
Correspondence<SetMultimap<K, V>, SetMultimap<K, String>> setMultimapValuesByString() {
310+
return new Correspondence<SetMultimap<K, V>, SetMultimap<K, String>>() {
311+
@Override
312+
public boolean compare(SetMultimap<K, V> actual, SetMultimap<K, String> expected) {
313+
return ImmutableSetMultimap.copyOf(transformValues(actual, Object::toString))
314+
.equals(expected);
315+
}
316+
317+
@Override
318+
public String toString() {
319+
return "is equivalent comparing multimap values by `toString()` to";
320+
}
321+
};
273322
}
274323

275324
@Test public void reportsMissingType() {

0 commit comments

Comments
 (0)