Skip to content

Commit fbf0363

Browse files
christiangoerdesrrayst
christiangoerdes
and
rrayst
authored
fix SpringConfigurationXSDGeneratingAnnotationProcessor #2178 (#2195)
* implement error for duplicate setters * add error for attribute - childelement name clash * Validate and enforce unique element/attribute/child names and detect clashes - Replace old uniqueness check with getUniquenessError(ElementInfo, MainInfo) - Collect named groups (attributes, textContent, child elements) and check duplicates per-group - Detect name clashes across groups and report them - Add helpers: NameGroup, collectNameGroups, group(), duplicates(), refined text/content handling - Improve child element name handling (validate list declarations, throw ProcessingException on duplicate child names) - Remove noisy debug/legacy checks and integrate uniqueness checks into main processing loop - Update example proxies.yaml to include ssl.key.private placeholder * minor improvements * Refine element processing logic and add top-level MCElement filter for `java.lang.Object` assignments. * Simplify conditional logic in `SpringConfigurationXSDGeneratingAnnotationProcessor` for element processing. --------- Co-authored-by: Tobias Polley <[email protected]>
1 parent 5891ae9 commit fbf0363

File tree

1 file changed

+143
-15
lines changed

1 file changed

+143
-15
lines changed

‎annot/src/main/java/com/predic8/membrane/annot/SpringConfigurationXSDGeneratingAnnotationProcessor.java‎

Lines changed: 143 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,16 @@
2121
import javax.lang.model.*;
2222
import javax.lang.model.element.*;
2323
import javax.lang.model.type.*;
24+
import javax.lang.model.util.Elements;
25+
import javax.lang.model.util.Types;
2426
import javax.tools.Diagnostic.*;
2527
import javax.tools.*;
2628
import java.io.*;
2729
import java.lang.annotation.*;
2830
import java.util.*;
31+
import java.util.function.Function;
32+
import java.util.stream.Collectors;
33+
import java.util.stream.Stream;
2934

3035
import static javax.tools.StandardLocation.*;
3136

@@ -86,7 +91,7 @@ private void read() {
8691
}
8792
if (element != null) {
8893
if (element.getAnnotation(annotationClass) != null)
89-
currentSet.add(element);
94+
Objects.requireNonNull(currentSet).add(element);
9095
}
9196
} else {
9297
try {
@@ -214,17 +219,14 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
214219

215220
scan(m, main, ii);
216221

217-
String uniquenessError = getUniquenessError(ii);
218-
if (uniquenessError != null)
219-
throw new ProcessingException(uniquenessError, ii.getElement());
220222
if (ii.getAnnotation().noEnvelope()) {
221223
if (ii.getAnnotation().topLevel())
222224
throw new ProcessingException("@MCElement(..., noEnvelope=true, topLevel=true) is invalid.", ii.getElement());
223225
if (ii.getAnnotation().mixed())
224226
throw new ProcessingException("@MCElement(..., noEnvelope=true, mixed=true) is invalid.", ii.getElement());
225227
if (ii.getChildElementSpecs().size() != 1)
226228
throw new ProcessingException("@MCElement(noEnvelope=true) requires exactly one @MCChildElement.", ii.getElement());
227-
if (!ii.getChildElementSpecs().get(0).isList())
229+
if (!ii.getChildElementSpecs().getFirst().isList())
228230
throw new ProcessingException("@MCElement(noEnvelope=true) requires its @MCChildElement() to be a List or Collection.", ii.getElement());
229231
if (!ii.getAis().isEmpty())
230232
throw new ProcessingException("@MCElement(noEnvelope=true) requires @MCAttribute to be not present.", ii.getElement());
@@ -246,12 +248,16 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
246248
ElementInfo ei = main.getElements().get(f.getKey());
247249

248250
if (ei != null)
249-
cedi.getElementInfo().add(ei);
251+
cedi.getElementInfo().add(ei); // e.g. SSLParser
250252
else {
251-
for (Map.Entry<TypeElement, ElementInfo> e : main.getElements().entrySet())
252-
if (processingEnv.getTypeUtils().isAssignable(e.getKey().asType(), f.getKey().asType()))
253-
cedi.getElementInfo().add(e.getValue());
254-
}
253+
boolean targetIsObject = processingEnv.getTypeUtils().isSameType(f.getKey().asType(), processingEnv.getElementUtils().getTypeElement("java.lang.Object").asType());
254+
// e.g. AuthorizationService
255+
for (Map.Entry<TypeElement, ElementInfo> e : main.getElements().entrySet()) {
256+
if (!processingEnv.getTypeUtils().isAssignable(e.getKey().asType(), f.getKey().asType())) continue;
257+
if (targetIsObject && !isTopLevelMCElement(e.getKey())) continue; // only allow topLevel MCElements for Object
258+
cedi.getElementInfo().add(e.getValue());
259+
}
260+
}
255261

256262
for (ElementInfo ei2 : cedi.getElementInfo())
257263
ei2.addUsedBy(f.getValue());
@@ -263,6 +269,14 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
263269
}
264270
}
265271

272+
for (MainInfo main : m.getMains()) {
273+
for (Map.Entry<TypeElement, ElementInfo> f : main.getElements().entrySet()) {
274+
List < String > uniquenessErrors = getUniquenessError(f.getValue(), main);
275+
if (!uniquenessErrors.isEmpty())
276+
throw new ProcessingException(String.join(System.lineSeparator(), uniquenessErrors), f.getValue().getElement());
277+
}
278+
}
279+
266280

267281
if (mcmains.isEmpty()) {
268282
processingEnv.getMessager().printMessage(Kind.ERROR, "@MCMain but no @MCElement found.", mcmains.iterator().next());
@@ -281,12 +295,126 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
281295
}
282296
}
283297

284-
private String getUniquenessError(ElementInfo ii) {
285-
// TODO ii.getChildElementSpecs().map(::getPropertyName) aber isList siehe JsonSchemaGenerator:140
286-
return null;
287-
}
298+
private boolean isTopLevelMCElement(TypeElement type) {
299+
MCElement mcElement = type.getAnnotation(MCElement.class);
300+
return (mcElement != null) && mcElement.topLevel();
301+
}
302+
303+
private List<String> getUniquenessError(ElementInfo ii, MainInfo main) {
304+
List<String> errors = new ArrayList<>();
305+
var groups = collectNameGroups(ii, main);
306+
307+
// Check for duplicates within a group
308+
for (var g : groups) {
309+
var dups = duplicates(g.names());
310+
if (!dups.isEmpty()) {
311+
errors.add("Duplicate " + g.label() + ": " + String.join(", ", dups));
312+
}
313+
}
314+
315+
Map<String, Set<String>> index = new TreeMap<>();
316+
for (var g : groups) {
317+
// use a set to avoid inflating clashes due to internal duplicates
318+
for (String n : new TreeSet<>(g.names())) {
319+
index.computeIfAbsent(n, __ -> new TreeSet<>()).add(g.label());
320+
}
321+
}
322+
index.forEach((name, usedIn) -> {
323+
if (usedIn.size() > 1) {
324+
errors.add("Name clash: '" + name + "' used by " + String.join(" & ", usedIn));
325+
}
326+
});
327+
328+
return errors;
329+
}
330+
331+
private List<NameGroup> collectNameGroups(ElementInfo ii, MainInfo main) {
332+
var groups = new ArrayList<NameGroup>();
333+
groups.add(group("attributes", attributeNames(ii)));
334+
groups.add(group("textContent", textContentNames(ii)));
335+
groups.addAll(childElementNames(ii, main));
336+
return groups.stream().filter(g -> !g.names().isEmpty()).toList();
337+
}
338+
339+
private Stream<String> textContentNames(ElementInfo ii) {
340+
if (ii.getTci() != null) {
341+
return ii.getTci().getPropertyName().lines();
342+
}
343+
return Stream.<String>builder().build();
344+
}
345+
346+
private NameGroup group(String label, Stream<String> names) {
347+
return new NameGroup(
348+
label,
349+
names.filter(Objects::nonNull).map(String::trim).filter(s -> !s.isEmpty()).toList()
350+
);
351+
}
352+
353+
private record NameGroup(String label, List<String> names) {}
354+
355+
private List<String> duplicates(Collection<String> names) {
356+
return names.stream()
357+
.collect(Collectors.groupingBy(Function.identity(), Collectors.counting()))
358+
.entrySet().stream()
359+
.filter(e -> e.getValue() > 1)
360+
.map(Map.Entry::getKey)
361+
.sorted()
362+
.toList();
363+
}
364+
365+
366+
private Stream<String> attributeNames(ElementInfo ii) {
367+
return ii.getAis().stream()
368+
.map(AttributeInfo::getXMLName)
369+
.filter(n -> n != null && !n.isBlank())
370+
.map(String::trim);
371+
}
372+
373+
private ArrayList<NameGroup> childElementNames(ElementInfo ii, MainInfo main) {
374+
var groups = new ArrayList<NameGroup>();
375+
376+
for (ChildElementInfo cei : ii.getChildElementSpecs()) {
377+
List<String> names = new ArrayList<>();
378+
if (cei.isList()) {
379+
// e.g. api.interceptors
380+
names.add(cei.getPropertyName());
381+
var decl = main.getChildElementDeclarations().get(cei.getTypeDeclaration());
382+
if (decl != null) {
383+
var dupes = decl.getElementInfo().stream()
384+
.map(ei -> {
385+
var ann = ei.getAnnotation();
386+
return ann == null ? null : ann.name();
387+
})
388+
.filter(n -> n != null && !n.isBlank())
389+
.map(String::trim)
390+
.collect(java.util.stream.Collectors.groupingBy(java.util.function.Function.identity(),
391+
java.util.stream.Collectors.counting()))
392+
.entrySet().stream()
393+
.filter(e -> e.getValue() > 1)
394+
.map(java.util.Map.Entry::getKey)
395+
.sorted()
396+
.toList();
397+
if (!dupes.isEmpty()) {
398+
throw new ProcessingException(
399+
"Duplicate child names for setter '" + cei.getPropertyName() + "': "
400+
+ String.join(", ", dupes),
401+
cei.getTypeDeclaration());
402+
}
403+
}
404+
} else {
405+
// e.g. api.path, oauth2resource2.google
406+
for (ElementInfo ei : main.getChildElementDeclarations().get(cei.getTypeDeclaration()).getElementInfo()) {
407+
names.add(ei.getAnnotation().name());
408+
}
409+
}
410+
411+
groups.add(new NameGroup("childElement '" + cei.getPropertyName() + "'", names));
412+
}
413+
return groups;
414+
}
415+
288416

289-
private static final String REQUIRED = "com.predic8.membrane.annot.Required";
417+
private static final String REQUIRED = "com.predic8.membrane.annot.Required";
290418

291419
private void scan(Model m, MainInfo main, ElementInfo ii) {
292420
scan(m, main, ii, ii.getElement());

0 commit comments

Comments
 (0)