Skip to content

Commit

Permalink
Revert "Fix handling of subclassed plugins in Log4j Docgen (#117)"
Browse files Browse the repository at this point in the history 
This reverts commit f8b0cb4.
  • Loading branch information
@vy
vy committed May 7, 2024
1 parent f8b0cb4 commit f9e80c3
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 143 deletions.
125 changes: 16 additions & 109 deletions ...j-docgen/src/main/java/org/apache/logging/log4j/docgen/generator/internal/TypeLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand All @@ -45,108 +42,18 @@ private TypeLookup(final Iterable<? extends PluginSet> pluginSets, final Predica

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);
});
});
}

Expand Down
40 changes: 14 additions & 26 deletions ...j-docgen/src/main/java/org/apache/logging/log4j/docgen/processor/DescriptorGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -253,8 +253,7 @@ public SourceVersion getSupportedSourceVersion() {
@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
Expand All @@ -266,39 +265,28 @@ public boolean process(final Set<? extends TypeElement> unused, final RoundEnvir
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());
Expand Down
8 changes: 0 additions & 8 deletions src/changelog/.0.x.x/fix-docgen-duplicate-type.xml
View file

This file was deleted.

0 comments on commit f9e80c3

Please sign in to comment.