Fix handling of subclassed plugins in Log4j Docgen (#117)
diff --git a/log4j-docgen/src/main/java/org/apache/logging/log4j/docgen/generator/internal/TypeLookup.java b/log4j-docgen/src/main/java/org/apache/logging/log4j/docgen/generator/internal/TypeLookup.java
index d32bd00..f87ba02 100644
--- a/log4j-docgen/src/main/java/org/apache/logging/log4j/docgen/generator/internal/TypeLookup.java
+++ b/log4j-docgen/src/main/java/org/apache/logging/log4j/docgen/generator/internal/TypeLookup.java
@@ -16,14 +16,17 @@
*/
package org.apache.logging.log4j.docgen.generator.internal;
+import org.apache.logging.log4j.docgen.AbstractType;
+import org.apache.logging.log4j.docgen.PluginSet;
+import org.apache.logging.log4j.docgen.PluginType;
+import org.apache.logging.log4j.docgen.ScalarType;
+import org.apache.logging.log4j.docgen.Type;
+import org.jspecify.annotations.Nullable;
+
import java.util.Iterator;
import java.util.Set;
import java.util.TreeMap;
import java.util.function.Predicate;
-import org.apache.logging.log4j.docgen.AbstractType;
-import org.apache.logging.log4j.docgen.PluginSet;
-import org.apache.logging.log4j.docgen.PluginType;
-import org.jspecify.annotations.Nullable;
public final class TypeLookup extends TreeMap<String, ArtifactSourcedType> {
@@ -42,18 +45,108 @@
private void mergeDescriptors(Iterable<? extends PluginSet> pluginSets) {
pluginSets.forEach(pluginSet -> {
- pluginSet.getScalars().forEach(scalar -> {
- final ArtifactSourcedType sourcedType = ArtifactSourcedType.ofPluginSet(pluginSet, scalar);
- putIfAbsent(scalar.getClassName(), sourcedType);
- });
- pluginSet.getAbstractTypes().forEach(abstractType -> {
- final ArtifactSourcedType sourcedType = ArtifactSourcedType.ofPluginSet(pluginSet, abstractType);
- putIfAbsent(abstractType.getClassName(), sourcedType);
- });
- pluginSet.getPlugins().forEach(pluginType -> {
- final ArtifactSourcedType sourcedType = ArtifactSourcedType.ofPluginSet(pluginSet, pluginType);
- putIfAbsent(pluginType.getClassName(), sourcedType);
- });
+ mergeScalarTypes(pluginSet);
+ mergeAbstractTypes(pluginSet);
+ mergePluginTypes(pluginSet);
+ });
+ }
+
+ @SuppressWarnings("StatementWithEmptyBody")
+ private void mergeScalarTypes(PluginSet pluginSet) {
+ pluginSet.getScalars().forEach(newType -> {
+
+ final String className = newType.getClassName();
+ @Nullable final ArtifactSourcedType oldSourcedType = get(className);
+
+ // If the entry doesn't exist yet, add it
+ if (oldSourcedType == null) {
+ final ArtifactSourcedType newSourcedType = ArtifactSourcedType.ofPluginSet(pluginSet, newType);
+ put(className, newSourcedType);
+ }
+
+ // If the entry already exists and is of expected type, we should ideally extend it.
+ // Since Modello doesn't generate `hashCode()`, `equals()`, etc. it is difficult to compare instances.
+ // Hence, we will be lazy, and just assume they are the same.
+ else if (oldSourcedType.type instanceof ScalarType) {}
+
+ // If the entry already exists, but with an unexpected type, fail
+ else {
+ conflictingTypeFailure(className, oldSourcedType.type, newType);
+ }
+ });
+ }
+
+ private static void conflictingTypeFailure(final String className, final Type oldType, final Type newType) {
+ final String message = String.format(
+ "`%s` class occurs multiple times with conflicting types: `%s` and `%s`",
+ className,
+ oldType.getClass().getSimpleName(),
+ newType.getClass().getSimpleName());
+ throw new IllegalArgumentException(message);
+ }
+
+ private void mergeAbstractTypes(PluginSet pluginSet) {
+ pluginSet.getAbstractTypes().forEach(newType -> {
+
+ final String className = newType.getClassName();
+ @Nullable final ArtifactSourcedType oldSourcedType = get(className);
+
+ // If the entry doesn't exist yet, add it
+ if (oldSourcedType == null) {
+ final ArtifactSourcedType newSourcedType = ArtifactSourcedType.ofPluginSet(pluginSet, newType);
+ put(className, newSourcedType);
+ }
+
+ // If the entry already exists and is of expected type, extend it
+ else if (oldSourcedType.type instanceof AbstractType) {
+ final AbstractType oldType = (AbstractType) oldSourcedType.type;
+ newType.getImplementations().forEach(oldType::addImplementation);
+ }
+
+ // If the entry already exists, but with an unexpected type, fail
+ else {
+ conflictingTypeFailure(className, oldSourcedType.type, newType);
+ }
+ });
+ }
+
+ private void mergePluginTypes(PluginSet pluginSet) {
+ pluginSet.getPlugins().forEach(newType -> {
+
+ final String className = newType.getClassName();
+ @Nullable final ArtifactSourcedType oldSourcedType = get(className);
+
+ // If the entry doesn't exist yet, add it
+ if (oldSourcedType == null) {
+ final ArtifactSourcedType newSourcedType = ArtifactSourcedType.ofPluginSet(pluginSet, newType);
+ put(className, newSourcedType);
+ }
+
+ // If the entry already exists, but is of `AbstractType`, promote it to `PluginType`.
+ //
+ // The most prominent example to this is `LoggerConfig`, which is a plugin.
+ // Assume `AsyncLoggerConfig` (extending from `LoggerConfig`) is encountered first.
+ // This results in `LoggerConfig` getting registered as an `AbstractType`.
+ // When the actual `LoggerConfig` definition is encountered, the type needs to be promoted to `PluginType`.
+ // Otherwise, `LoggerConfig` plugin definition will get skipped.
+ else if (oldSourcedType.type instanceof AbstractType && !(oldSourcedType.type instanceof PluginType)) {
+ final ArtifactSourcedType newSourcedType = ArtifactSourcedType.ofPluginSet(pluginSet, newType);
+ put(className, newSourcedType);
+ // Preserve old implementations
+ final AbstractType oldType = (AbstractType) oldSourcedType.type;
+ oldType.getImplementations().forEach(newType::addImplementation);
+ }
+
+ // If the entry already exists and is of expected type, extend it
+ else if (oldSourcedType.type instanceof PluginType) {
+ final PluginType oldType = (PluginType) oldSourcedType.type;
+ newType.getImplementations().forEach(oldType::addImplementation);
+ }
+
+ // If the entry already exists, but with an unexpected type, fail
+ else {
+ conflictingTypeFailure(className, oldSourcedType.type, newType);
+ }
});
}
diff --git a/log4j-docgen/src/main/java/org/apache/logging/log4j/docgen/processor/DescriptorGenerator.java b/log4j-docgen/src/main/java/org/apache/logging/log4j/docgen/processor/DescriptorGenerator.java
index eb9b82a..cd82d7b 100644
--- a/log4j-docgen/src/main/java/org/apache/logging/log4j/docgen/processor/DescriptorGenerator.java
+++ b/log4j-docgen/src/main/java/org/apache/logging/log4j/docgen/processor/DescriptorGenerator.java
@@ -135,11 +135,11 @@
*/
private static final String IMPOSSIBLE_REGEX = "(?!.*)";
- // Abstract types to process
- private final Collection<TypeElement> abstractTypesToDocument = new HashSet<>();
+ private final Set<TypeElement> pluginTypesToDocument = new HashSet<>();
- // Scalar types to process
- private final Collection<TypeElement> scalarTypesToDocument = new HashSet<>();
+ private final Set<TypeElement> abstractTypesToDocument = new HashSet<>();
+
+ private final Set<TypeElement> scalarTypesToDocument = new HashSet<>();
private Predicate<String> classNameFilter;
@@ -253,7 +253,8 @@
@Override
public boolean process(final Set<? extends TypeElement> unused, final RoundEnvironment roundEnv) {
// First step: document plugins
- roundEnv.getElementsAnnotatedWithAny(annotations.getPluginAnnotations()).forEach(this::addPluginDocumentation);
+ populatePluginTypesToDocument(roundEnv);
+ pluginTypesToDocument.forEach(this::addPluginDocumentation);
// Second step: document abstract types
abstractTypesToDocument.forEach(this::addAbstractTypeDocumentation);
// Second step: document scalars
@@ -265,28 +266,39 @@
return false;
}
- private void addPluginDocumentation(final Element element) {
- try {
+ private void populatePluginTypesToDocument(final RoundEnvironment roundEnv) {
+ roundEnv.getElementsAnnotatedWithAny(annotations.getPluginAnnotations()).forEach(element -> {
if (element instanceof TypeElement) {
- final PluginType pluginType = new PluginType();
- pluginType.setName(annotations.getPluginSpecifiedName(element).orElseGet(() -> element.getSimpleName()
- .toString()));
- pluginType.setNamespace(
- annotations.getPluginSpecifiedNamespace(element).orElse("Core"));
- populatePlugin((TypeElement) element, pluginType);
- pluginSet.addPlugin(pluginType);
+ pluginTypesToDocument.add((TypeElement) element);
} else {
messager.printMessage(
Diagnostic.Kind.WARNING, "Found @Plugin annotation on unexpected element.", element);
}
+ });
+ }
+
+ private void addPluginDocumentation(final TypeElement element) {
+ try {
+ final PluginType pluginType = new PluginType();
+ pluginType.setName(annotations.getPluginSpecifiedName(element).orElseGet(() -> element.getSimpleName()
+ .toString()));
+ pluginType.setNamespace(
+ annotations.getPluginSpecifiedNamespace(element).orElse("Core"));
+ populatePlugin(element, pluginType);
+ pluginSet.addPlugin(pluginType);
} catch (final Exception error) {
final String message = String.format("failed to process element `%s`", element);
throw new RuntimeException(message, error);
}
}
+ @SuppressWarnings("SuspiciousMethodCalls")
private void addAbstractTypeDocumentation(final QualifiedNameable element) {
try {
+ // Short-circuit if the type is already documented as a plugin
+ if (pluginTypesToDocument.contains(element)) {
+ return;
+ }
final AbstractType abstractType = new AbstractType();
final ElementImports imports = importsFactory.ofElement(element);
final String qualifiedClassName = getClassName(element.asType());
diff --git a/src/changelog/.0.x.x/fix-docgen-duplicate-type.xml b/src/changelog/.0.x.x/fix-docgen-duplicate-type.xml
new file mode 100644
index 0000000..f5a925b
--- /dev/null
+++ b/src/changelog/.0.x.x/fix-docgen-duplicate-type.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="https://logging.apache.org/xml/ns"
+ xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="117" link="https://github.com/apache/logging-log4j-tools/issues/117"/>
+ <description format="asciidoc">Fix handling of subclassed plugins in Log4j Docgen</description>
+</entry>