This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/logging-log4j-tools.git
The following commit(s) were added to refs/heads/main by this push:
new f9e80c3 Revert "Fix handling of subclassed plugins in Log4j Docgen
(#117)"
f9e80c3 is described below
commit f9e80c3dcaa2911a3bb7a4d2f9963cd8c6330d91
Author: Volkan Yazıcı <[email protected]>
AuthorDate: Tue May 7 10:35:24 2024 +0200
Revert "Fix handling of subclassed plugins in Log4j Docgen (#117)"
This reverts commit f8b0cb45bdf40cb90bcb85595c05a5ddadf15877.
---
.../docgen/generator/internal/TypeLookup.java | 125 +++------------------
.../docgen/processor/DescriptorGenerator.java | 40 +++----
src/changelog/.0.x.x/fix-docgen-duplicate-type.xml | 8 --
3 files changed, 30 insertions(+), 143 deletions(-)
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 f87ba02..d32bd00 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,17 +16,14 @@
*/
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> {
@@ -45,108 +42,18 @@ public final class TypeLookup extends TreeMap<String,
ArtifactSourcedType> {
private void mergeDescriptors(Iterable<? extends PluginSet> pluginSets) {
pluginSets.forEach(pluginSet -> {
- 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);
- }
+ 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);
+ });
});
}
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 cd82d7b..eb9b82a 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 @@ public class DescriptorGenerator extends
AbstractProcessor {
*/
private static final String IMPOSSIBLE_REGEX = "(?!.*)";
- private final Set<TypeElement> pluginTypesToDocument = new HashSet<>();
+ // Abstract types to process
+ private final Collection<TypeElement> abstractTypesToDocument = new
HashSet<>();
- private final Set<TypeElement> abstractTypesToDocument = new HashSet<>();
-
- private final Set<TypeElement> scalarTypesToDocument = new HashSet<>();
+ // Scalar types to process
+ private final Collection<TypeElement> scalarTypesToDocument = new
HashSet<>();
private Predicate<String> classNameFilter;
@@ -253,8 +253,7 @@ public class DescriptorGenerator extends AbstractProcessor {
@Override
public boolean process(final Set<? extends TypeElement> unused, final
RoundEnvironment roundEnv) {
// First step: document plugins
- populatePluginTypesToDocument(roundEnv);
- pluginTypesToDocument.forEach(this::addPluginDocumentation);
+
roundEnv.getElementsAnnotatedWithAny(annotations.getPluginAnnotations()).forEach(this::addPluginDocumentation);
// Second step: document abstract types
abstractTypesToDocument.forEach(this::addAbstractTypeDocumentation);
// Second step: document scalars
@@ -266,39 +265,28 @@ public class DescriptorGenerator extends
AbstractProcessor {
return false;
}
- private void populatePluginTypesToDocument(final RoundEnvironment
roundEnv) {
-
roundEnv.getElementsAnnotatedWithAny(annotations.getPluginAnnotations()).forEach(element
-> {
+ private void addPluginDocumentation(final Element element) {
+ try {
if (element instanceof TypeElement) {
- pluginTypesToDocument.add((TypeElement) element);
+ 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);
} 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
deleted file mode 100644
index f5a925b..0000000
--- a/src/changelog/.0.x.x/fix-docgen-duplicate-type.xml
+++ /dev/null
@@ -1,8 +0,0 @@
-<?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>
