Skip to content

Commit 2aa273c

Browse files
authored
Smart subtype walk (#18)
* Skip sibling subtypes when only a concrete subtype is referenced A polymorphic base reached only via a concrete subtype's superclass walk now generates a plain parser instead of a discriminator switch, and its sibling subtypes are no longer walked. This lets consumers reference e.g. Point fields without forcing parsers for LineString and Polygon when those siblings only exist in the base's @JsonSubTypes list. Bases reached directly as a field or type-argument keep the existing behavior: every subtype is walked and a polymorphic dispatch parser is emitted. * Drop dead overloads and trim verbose comments The single-arg analyzeTypeAndSubtypes, the no-polymorphic-set ParserWriter constructor, and the 4-arg generateParserForFields each had exactly one caller in the previous commit; inline them so there is one entry point per operation. Also strip narration-style comments and accessor noise from fixtures that the analyzer never touches.
1 parent 5bc784d commit 2aa273c

15 files changed

Lines changed: 425 additions & 48 deletions

gwt-beans-codegen-core/src/main/java/nl/aerius/codegen/ParserGenerator.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ public static void generateParsersForClass(final Class<?> targetClass, final Str
106106
final TypeAnalyzer analyzer = new TypeAnalyzer(classFinder, logger);
107107
analyzer.setCustomParserTypes(customParserTypes);
108108
final Set<ClassName> classNames = analyzer.analyzeClass(targetClass.getName());
109+
final Set<Class<?>> polymorphicallyReachedTypes = analyzer.getPolymorphicallyReachedTypes();
109110

110111
// Filter out types that have custom parsers (this is now redundant since
111112
// TypeAnalyzer handles it)
@@ -116,7 +117,8 @@ public static void generateParsersForClass(final Class<?> targetClass, final Str
116117

117118
// Create a parser writer and generate all parsers
118119
// Pass the generator name and details (version + hash) to the writer
119-
final ParserWriter parserWriter = new ParserWriter(outputDir, parserPackage, generatorName, generatorDetails, classFinder, logger);
120+
final ParserWriter parserWriter = new ParserWriter(outputDir, parserPackage, generatorName, generatorDetails, classFinder, logger,
121+
polymorphicallyReachedTypes);
120122

121123
parserWriter.generateParsers(classFinder, filteredClassNames);
122124

gwt-beans-codegen-core/src/main/java/nl/aerius/codegen/analyzer/TypeAnalyzer.java

Lines changed: 54 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public class TypeAnalyzer {
3939
private final Set<Class<?>> processedTypes;
4040
private final Set<String> skippedTypes;
4141
private final Set<ClassName> discoveredTypes;
42+
private final Set<Class<?>> polymorphicallyReachedTypes;
4243
private final Set<String> printedTypes = new HashSet<>();
4344
private final Set<String> customParserTypes = new HashSet<>();
4445
private final ClassFinder classFinder;
@@ -80,6 +81,7 @@ public TypeAnalyzer(final ClassFinder classFinder, final Logger logger) {
8081
processedTypes = new HashSet<>();
8182
skippedTypes = new HashSet<>();
8283
discoveredTypes = new TreeSet<>(Comparator.comparing(ClassName::toString));
84+
polymorphicallyReachedTypes = new HashSet<>();
8385
}
8486

8587
/**
@@ -102,8 +104,9 @@ public Set<ClassName> analyzeClass(final String className) {
102104
skippedTypes.clear();
103105
discoveredTypes.clear();
104106
printedTypes.clear();
107+
polymorphicallyReachedTypes.clear();
105108

106-
analyzeTypeAndSubtypes(rootClass);
109+
analyzeTypeAndSubtypes(rootClass, true);
107110

108111
if (!skippedTypes.isEmpty()) {
109112
logger.info("Warning: The following types were not found and skipped:");
@@ -116,56 +119,71 @@ public Set<ClassName> analyzeClass(final String className) {
116119
}
117120
}
118121

119-
private void analyzeTypeAndSubtypes(final Class<?> type) {
122+
/**
123+
* Polymorphic bases reached directly (as a field type or type argument) in
124+
* the most recent analysis. The generator emits a discriminator switch for
125+
* these; bases reached only via a concrete subtype's superclass chain are
126+
* absent and get a standard parse method instead.
127+
*/
128+
public Set<Class<?>> getPolymorphicallyReachedTypes() {
129+
return polymorphicallyReachedTypes;
130+
}
131+
132+
private void analyzeTypeAndSubtypes(final Class<?> type, final boolean reachedDirectly) {
120133
if (!shouldAnalyzeType(type)) {
121134
return;
122135
}
123136

124-
if (!processedTypes.add(type)) {
125-
return; // Already processed
126-
}
137+
final boolean polymorphic = isPolymorphicBase(type);
138+
final boolean firstVisit = processedTypes.add(type);
139+
final boolean newPolymorphicReach = polymorphic && reachedDirectly && polymorphicallyReachedTypes.add(type);
127140

128-
// Print the class hierarchy if we haven't seen this type before
129-
if (!printedTypes.contains(type.getName())) {
130-
printClassHierarchy(type);
131-
printedTypes.add(type.getName());
141+
if (!firstVisit && !newPolymorphicReach) {
142+
return;
132143
}
133144

134-
// If this type has a custom parser, skip adding it for generation
135-
// but continue analyzing its fields and subtypes
136-
if (!hasCustomParser(type)) {
137-
addTypeForGeneration(type);
138-
} else {
139-
logger.info("Skipping parser generation for " + type.getName() + " (has custom parser)");
140-
}
145+
if (firstVisit) {
146+
if (!printedTypes.contains(type.getName())) {
147+
printClassHierarchy(type);
148+
printedTypes.add(type.getName());
149+
}
141150

142-
// Find and process superclasses
143-
final Class<?> superclass = type.getSuperclass();
144-
if (superclass != null && superclass != Object.class) {
145-
analyzeTypeAndSubtypes(superclass);
146-
}
151+
if (!hasCustomParser(type)) {
152+
addTypeForGeneration(type);
153+
} else {
154+
logger.info("Skipping parser generation for " + type.getName() + " (has custom parser)");
155+
}
147156

148-
final JsonSubTypes subTypesAnnotation = type.getAnnotation(JsonSubTypes.class);
149-
final JsonTypeInfo typeInfoAnnotation = type.getAnnotation(JsonTypeInfo.class); // Check presence of base annotation too
157+
// Superclasses are not "directly reached" - if the superclass is a
158+
// polymorphic base, sibling subtypes stay unwalked.
159+
final Class<?> superclass = type.getSuperclass();
160+
if (superclass != null && superclass != Object.class) {
161+
analyzeTypeAndSubtypes(superclass, false);
162+
}
150163

151-
if (subTypesAnnotation != null && typeInfoAnnotation != null) { // Only process if both are present
164+
for (final Field field : ConstructorAnalyzer.getParseableFields(type)) {
165+
try {
166+
analyzeField(field);
167+
} catch (final TypeNotPresentException e) {
168+
skippedTypes.add(e.typeName());
169+
}
170+
}
171+
}
172+
173+
// Only expand @JsonSubTypes when the polymorphic base was reached directly.
174+
if (newPolymorphicReach) {
175+
final JsonSubTypes subTypesAnnotation = type.getAnnotation(JsonSubTypes.class);
152176
logger.info("Found @JsonSubTypes on: " + type.getName() + ", analyzing listed subtypes...");
153177
for (final JsonSubTypes.Type subType : subTypesAnnotation.value()) {
154178
final Class<?> subTypeValue = subType.value();
155179
logger.info(" - Analyzing subtype: " + subTypeValue.getName());
156-
analyzeTypeAndSubtypes(subTypeValue); // Recursive call for the subtype
180+
analyzeTypeAndSubtypes(subTypeValue, true);
157181
}
158182
}
183+
}
159184

160-
// Always analyze fields, even for types with custom parsers
161-
// This ensures we discover all types that might need parsers
162-
for (final Field field : ConstructorAnalyzer.getParseableFields(type)) {
163-
try {
164-
analyzeField(field);
165-
} catch (final TypeNotPresentException e) {
166-
skippedTypes.add(e.typeName());
167-
}
168-
}
185+
private boolean isPolymorphicBase(final Class<?> type) {
186+
return type.isAnnotationPresent(JsonSubTypes.class) && type.isAnnotationPresent(JsonTypeInfo.class);
169187
}
170188

171189
private void analyzeField(final Field field) {
@@ -214,12 +232,12 @@ private void analyzeType(final Type type) {
214232
if (isUnsupportedType(classType)) {
215233
throw new UnsupportedTypeException(classType.getName(), "type parameter/element", Object.class);
216234
}
217-
analyzeTypeAndSubtypes(classType);
235+
analyzeTypeAndSubtypes(classType, true);
218236
} else if (type instanceof ParameterizedType) {
219237
final ParameterizedType paramType = (ParameterizedType) type;
220238
// Analyze the raw type
221239
if (paramType.getRawType() instanceof Class<?>) {
222-
analyzeTypeAndSubtypes((Class<?>) paramType.getRawType());
240+
analyzeTypeAndSubtypes((Class<?>) paramType.getRawType(), true);
223241
}
224242
// Analyze all type arguments recursively
225243
for (final Type typeArg : paramType.getActualTypeArguments()) {

gwt-beans-codegen-core/src/main/java/nl/aerius/codegen/generator/ParserWriter.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,18 @@ public class ParserWriter {
2020
private final String generatorDetails;
2121
private final ClassFinder classFinder;
2222
private final Logger logger;
23+
private final Set<Class<?>> polymorphicallyReachedTypes;
2324

2425
public ParserWriter(final String outputDir, final String parserPackage, final String generatorName,
25-
final String generatorDetails, final ClassFinder classFinder, final Logger logger) {
26+
final String generatorDetails, final ClassFinder classFinder, final Logger logger,
27+
final Set<Class<?>> polymorphicallyReachedTypes) {
2628
this.outputDir = outputDir;
2729
this.parserPackage = parserPackage;
2830
this.generatorName = generatorName;
2931
this.generatorDetails = generatorDetails;
3032
this.classFinder = classFinder;
3133
this.logger = logger;
34+
this.polymorphicallyReachedTypes = polymorphicallyReachedTypes;
3235
}
3336

3437
/**
@@ -43,8 +46,10 @@ public void generateParser(final Class<?> targetClass) throws IOException {
4346
// Create the parser type specification, passing both name and details
4447
final TypeSpec.Builder typeSpec = ParserWriterUtils.createParserTypeSpec(parserClassName, generatorName, generatorDetails);
4548

49+
final boolean usePolymorphicDispatch = polymorphicallyReachedTypes.contains(targetClass);
50+
4651
// Add parser methods
47-
ParserWriterUtils.generateParserForFields(typeSpec, targetClass, parserPackage, classFinder);
52+
ParserWriterUtils.generateParserForFields(typeSpec, targetClass, parserPackage, classFinder, usePolymorphicDispatch);
4853

4954
// Write to file
5055
ParserWriterUtils.writeParserToFile(outputDir, parserPackage, typeSpec.build(), parserClassName, logger);

gwt-beans-codegen-core/src/main/java/nl/aerius/codegen/generator/ParserWriterUtils.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,15 @@ public static void registerCustomParser(final String typeName, final String pack
119119
}
120120

121121
/**
122-
* Main entry point for generating a parser class.
123-
* Creates both parse(String) and parse(JSONObjectHandle) methods.
124-
* For constructor-based types (no setters), generates constructor-based parse method.
125-
* @param classFinder
122+
* Main entry point for generating a parser class. When
123+
* {@code usePolymorphicDispatch} is false on a class that has
124+
* {@code @JsonTypeInfo}/{@code @JsonSubTypes}, a standard parse method is
125+
* emitted instead of the discriminator switch - the caller has determined
126+
* the type is reached only via a concrete subtype and the sibling subtype
127+
* parsers are not generated.
126128
*/
127129
public static void generateParserForFields(final TypeSpec.Builder typeSpec, final Class<?> targetClass, final String parserPackage,
128-
final ClassFinder classFinder) {
130+
final ClassFinder classFinder, final boolean usePolymorphicDispatch) {
129131
typeSpec.addMethod(createStringParseMethod(targetClass));
130132

131133
// Check if this class should use constructor-based parsing
@@ -136,16 +138,16 @@ public static void generateParserForFields(final TypeSpec.Builder typeSpec, fina
136138
typeSpec.addMethod(createConstructorBasedParseMethod(targetClass, parserPackage, constructorInfo.get()));
137139
} else {
138140
// Setter-based: existing approach
139-
addSetterBasedParseMethods(typeSpec, targetClass, parserPackage);
141+
addSetterBasedParseMethods(typeSpec, targetClass, parserPackage, usePolymorphicDispatch);
140142
}
141143
}
142144

143145
/**
144146
* Adds setter-based parse methods to the type specification.
145147
*/
146148
private static void addSetterBasedParseMethods(final TypeSpec.Builder typeSpec, final Class<?> targetClass,
147-
final String parserPackage) {
148-
if (hasJsonTypeInfoWithNameDiscriminator(targetClass)) {
149+
final String parserPackage, final boolean usePolymorphicDispatch) {
150+
if (usePolymorphicDispatch) {
149151
typeSpec.addMethod(createPolymorphicObjectParseMethod(targetClass, parserPackage));
150152
} else {
151153
typeSpec.addMethod(createStandardObjectParseMethod(targetClass, parserPackage));

gwt-beans-codegen-core/src/test/java/nl/aerius/codegen/analyzer/TypeAnalyzerTest.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
import org.junit.jupiter.api.BeforeEach;
1313
import org.junit.jupiter.api.Test;
1414

15+
import com.fasterxml.jackson.annotation.JsonSubTypes;
16+
import com.fasterxml.jackson.annotation.JsonSubTypes.Type;
17+
import com.fasterxml.jackson.annotation.JsonTypeInfo;
18+
import com.fasterxml.jackson.annotation.JsonTypeInfo.Id;
1519
import com.palantir.javapoet.ClassName;
1620

1721
import nl.aerius.codegen.util.ClassFinder;
@@ -50,6 +54,25 @@ void testCustomParserTypeHandling() {
5054
assertTrue(types.contains(ClassName.get("nl.aerius.codegen.analyzer", "TypeAnalyzerTest_CustomParserTestClass")));
5155
}
5256

57+
@Test
58+
void abstractBaseReachedViaConcreteSubtypeOnlyDoesNotExpandSiblings() {
59+
final Set<ClassName> types = analyzer.analyzeClass(ConcreteOnlyRootClass.class.getName());
60+
61+
assertTrue(types.contains(ClassName.get("nl.aerius.codegen.analyzer", "TypeAnalyzerTest_PolySubAlpha")));
62+
assertTrue(types.contains(ClassName.get("nl.aerius.codegen.analyzer", "TypeAnalyzerTest_PolyAbstractBase")));
63+
assertFalse(types.contains(ClassName.get("nl.aerius.codegen.analyzer", "TypeAnalyzerTest_PolySubBeta")));
64+
assertFalse(analyzer.getPolymorphicallyReachedTypes().contains(PolyAbstractBase.class));
65+
}
66+
67+
@Test
68+
void abstractBaseReachedDirectlyExpandsAllSubtypes() {
69+
final Set<ClassName> types = analyzer.analyzeClass(AbstractFieldRootClass.class.getName());
70+
71+
assertTrue(types.contains(ClassName.get("nl.aerius.codegen.analyzer", "TypeAnalyzerTest_PolySubAlpha")));
72+
assertTrue(types.contains(ClassName.get("nl.aerius.codegen.analyzer", "TypeAnalyzerTest_PolySubBeta")));
73+
assertTrue(analyzer.getPolymorphicallyReachedTypes().contains(PolyAbstractBase.class));
74+
}
75+
5376
@Test
5477
void testNestedCollectionTypes() {
5578
final Set<ClassName> types = analyzer.analyzeClass(NestedCollectionTestClass.class.getName());
@@ -99,4 +122,29 @@ private enum TestEnum {
99122
private static class ComplexNestedTestClass {
100123
private Map<TestEnum, Map<Integer, Map<Integer, String>>> complexNestedMap;
101124
}
125+
126+
@JsonTypeInfo(use = Id.NAME, property = "_type")
127+
@JsonSubTypes({
128+
@Type(value = PolySubAlpha.class, name = "alpha"),
129+
@Type(value = PolySubBeta.class, name = "beta")
130+
})
131+
private static abstract class PolyAbstractBase {
132+
private String baseLabel;
133+
}
134+
135+
private static class PolySubAlpha extends PolyAbstractBase {
136+
private int alphaValue;
137+
}
138+
139+
private static class PolySubBeta extends PolyAbstractBase {
140+
private boolean betaFlag;
141+
}
142+
143+
private static class ConcreteOnlyRootClass {
144+
private PolySubAlpha onlyConcrete;
145+
}
146+
147+
private static class AbstractFieldRootClass {
148+
private PolyAbstractBase polymorphic;
149+
}
102150
}

gwt-beans-codegen-core/src/test/java/nl/aerius/codegen/test/GeneratedParserValidationTest.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.junit.jupiter.api.Assertions;
1111
import org.junit.jupiter.api.DynamicTest;
12+
import org.junit.jupiter.api.Test;
1213
import org.junit.jupiter.api.TestFactory;
1314
import org.junit.jupiter.api.TestInstance;
1415
import org.junit.jupiter.api.TestInstance.Lifecycle;
@@ -65,5 +66,10 @@ Stream<DynamicTest> shouldGenerateMatchingParsers() throws IOException {
6566
});
6667
}
6768

68-
// Removed compareAllParsers method as logic moved to @TestFactory
69+
@Test
70+
void shouldSkipSiblingSubtypeWhenOnlyConcreteSubtypeIsReached() {
71+
final Path siblingParser = outputDir.resolve("nl/aerius/codegen/test/generated/TestSinglePolySubYParser.java");
72+
Assertions.assertFalse(Files.exists(siblingParser),
73+
"Sibling subtype parser must not be generated when only the other concrete subtype is referenced: " + siblingParser);
74+
}
6975
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package nl.aerius.codegen.test.types;
2+
3+
import nl.aerius.codegen.test.types.polymorphic.TestSinglePolySubX;
4+
5+
/**
6+
* References only one concrete subtype of a polymorphic hierarchy. The
7+
* sibling subtype (TestSinglePolySubY) must not get a generated parser.
8+
*/
9+
public class TestConcreteSubtypeOnlyType {
10+
private TestSinglePolySubX onlySubtype;
11+
12+
public TestSinglePolySubX getOnlySubtype() {
13+
return onlySubtype;
14+
}
15+
16+
public void setOnlySubtype(TestSinglePolySubX onlySubtype) {
17+
this.onlySubtype = onlySubtype;
18+
}
19+
20+
public static TestConcreteSubtypeOnlyType createFullObject() {
21+
TestConcreteSubtypeOnlyType obj = new TestConcreteSubtypeOnlyType();
22+
obj.setOnlySubtype(new TestSinglePolySubX("labelX", 99));
23+
return obj;
24+
}
25+
}

gwt-beans-codegen-core/src/test/java/nl/aerius/codegen/test/types/TestRootObjectType.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public class TestRootObjectType {
2121
private TestNestedMapType nestedMapType;
2222

2323
private TestPolyBase testPolyBase;
24+
private TestConcreteSubtypeOnlyType concreteSubtypeOnly;
2425
private TestPrimitiveArrayType primitiveArrays;
2526
private TestConstructorBasedType constructorBased;
2627
private TestConstructorWithGenericsType constructorWithGenerics;
@@ -131,6 +132,14 @@ public void setTestPolyBase(TestPolyBase testPolyBase) {
131132
this.testPolyBase = testPolyBase;
132133
}
133134

135+
public TestConcreteSubtypeOnlyType getConcreteSubtypeOnly() {
136+
return concreteSubtypeOnly;
137+
}
138+
139+
public void setConcreteSubtypeOnly(TestConcreteSubtypeOnlyType concreteSubtypeOnly) {
140+
this.concreteSubtypeOnly = concreteSubtypeOnly;
141+
}
142+
134143
public TestPrimitiveArrayType getPrimitiveArrays() {
135144
return primitiveArrays;
136145
}
@@ -186,6 +195,7 @@ public static TestRootObjectType createFullObject() {
186195
obj.setConcreteType(ConcreteType.createFullObject());
187196
obj.setNestedMapType(TestNestedMapType.createFullObject());
188197
obj.setTestPolyBase(new TestPolySubA("BaseValueA", 123));
198+
obj.setConcreteSubtypeOnly(TestConcreteSubtypeOnlyType.createFullObject());
189199
obj.setPrimitiveArrays(TestPrimitiveArrayType.createFullObject());
190200
obj.setConstructorBased(TestConstructorBasedType.createFullObject());
191201
obj.setConstructorWithGenerics(TestConstructorWithGenericsType.createFullObject());
@@ -234,6 +244,7 @@ public static TestRootObjectType createNullObject() {
234244
obj.setConcreteType(null);
235245
obj.setNestedMapType(null);
236246
obj.setTestPolyBase(null);
247+
obj.setConcreteSubtypeOnly(null);
237248
obj.setPrimitiveArrays(null);
238249
return obj;
239250
}

0 commit comments

Comments
 (0)