Skip to content

Commit

Permalink
fix(java): Fix union type name conflict resolution (#5637)
Browse files Browse the repository at this point in the history
* fix(java): Fix union type name conflict resolution

* Add version yamls

* fix typo

* chore: update changelog

---------

Co-authored-by: Alberto <[email protected]>
  • Loading branch information
ajgateno and Alberto authored Jan 17, 2025
1 parent 497a307 commit b5e3635
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ protected List<TypeSpec> getInlineTypeSpecs() {
false,
ImmutableSet.<String>builder()
.addAll(reservedTypeNames)
.add(className.simpleName())
.add(name)
.build(),
false));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
import com.fern.ir.model.commons.Name;
import com.fern.ir.model.commons.SafeAndUnsafeString;
import com.fern.ir.model.commons.TypeId;
import com.fern.ir.model.types.DeclaredTypeName;
import com.fern.ir.model.types.TypeDeclaration;
Expand All @@ -37,6 +39,7 @@
import com.fern.java.utils.TypeReferenceUtils;
import com.fern.java.utils.TypeReferenceUtils.ContainerTypeEnum;
import com.fern.java.utils.TypeReferenceUtils.TypeReferenceToName;
import com.google.common.collect.ImmutableSet;
import com.squareup.javapoet.AnnotationSpec;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.FieldSpec;
Expand All @@ -48,6 +51,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand All @@ -57,6 +61,8 @@
import javax.lang.model.element.Modifier;

public final class UndiscriminatedUnionGenerator extends AbstractTypeGenerator {
private static final String VISITOR_CLASS_NAME = "Visitor";
private static final String VISITOR_CLASS_NAME_UNDERSCORE = "Visitor_";

private static final String TYPE_COMMENT = "If %d, value is of type %s";
private static final String TYPE_FIELD_NAME = "type";
Expand All @@ -79,6 +85,7 @@ public final class UndiscriminatedUnionGenerator extends AbstractTypeGenerator {
private final ClassName visitorClassName;
private final ClassName deserializerClassName;
private final String undiscriminatedUnionPrefix;
private final String visitorName;

public UndiscriminatedUnionGenerator(
ClassName className,
Expand All @@ -95,11 +102,33 @@ public UndiscriminatedUnionGenerator(
member -> generatorContext.getPoetTypeNameMapper().convertToTypeName(true, member.getType()))));
if (generatorContext.getCustomConfig().enableInlineTypes()) {
typeNames.putAll(overrideMemberTypeNames(
className, generatorContext, undiscriminatedUnion, undiscriminatedUnionPrefix));
className, generatorContext, undiscriminatedUnion, reservedTypeNames, undiscriminatedUnionPrefix));
}
this.memberTypeNames = typeNames;
this.duplicatedOuterContainerTypes = getDuplicatedOuterContainerTypes(undiscriminatedUnion);
this.visitorClassName = className.nestedClass("Visitor");
this.visitorName = visitorName(
// We need to take into consideration all ancestor types as well as all sibling types so that
// to prevent naming the visitor "Visitor" if we already have a variant or property called that.
ImmutableSet.<String>builder()
.addAll(reservedTypeNames)
.addAll(
generatorContext.getCustomConfig().enableInlineTypes()
? overriddenTypeDeclarations(
className,
generatorContext,
undiscriminatedUnion,
reservedTypeNames,
undiscriminatedUnionPrefix)
.values()
.stream()
.map(TypeDeclaration::getName)
.map(DeclaredTypeName::getName)
.map(Name::getPascalCase)
.map(SafeAndUnsafeString::getSafeName)
.collect(Collectors.toList())
: List.of())
.build());
this.visitorClassName = className.nestedClass(visitorName);
this.deserializerClassName = className.nestedClass("Deserializer");
this.undiscriminatedUnionPrefix = undiscriminatedUnionPrefix;
}
Expand All @@ -120,14 +149,19 @@ public static Set<ContainerTypeEnum> getDuplicatedOuterContainerTypes(
@Override
public List<TypeDeclaration> getInlineTypeDeclarations() {
return new ArrayList<>(overriddenTypeDeclarations(
className, generatorContext, undiscriminatedUnion, undiscriminatedUnionPrefix)
className,
generatorContext,
undiscriminatedUnion,
reservedTypeNames,
undiscriminatedUnionPrefix)
.values());
}

private static Map<TypeId, TypeDeclaration> overriddenTypeDeclarations(
ClassName className,
AbstractGeneratorContext<?, ?> generatorContext,
UndiscriminatedUnionTypeDeclaration undiscriminatedUnion,
Set<String> reservedTypeNames,
String undiscriminatedUnionPrefix) {
Map<TypeId, TypeDeclaration> overriddenTypeDeclarations = new HashMap<>();

Expand All @@ -149,6 +183,8 @@ private static Map<TypeId, TypeDeclaration> overriddenTypeDeclarations(
continue;
}

Set<String> allReservedTypeNames = new HashSet<>(reservedTypeNames);

String name =
rawTypeDeclaration.getName().getName().getPascalCase().getSafeName();

Expand All @@ -157,6 +193,17 @@ private static Map<TypeId, TypeDeclaration> overriddenTypeDeclarations(
name = name.substring(undiscriminatedUnionPrefix.length());
}

boolean valid;
do {
valid = !allReservedTypeNames.contains(name);

if (!valid) {
name += "_";
}
} while (!valid);

allReservedTypeNames.add(name);

TypeDeclaration overriddenTypeDeclaration = overrideTypeDeclarationName(rawTypeDeclaration, name);
overriddenTypeDeclarations.put(resolvedId.typeId(), overriddenTypeDeclaration);
}
Expand All @@ -169,9 +216,10 @@ private static Map<UndiscriminatedUnionMember, TypeName> overrideMemberTypeNames
ClassName className,
AbstractGeneratorContext<?, ?> generatorContext,
UndiscriminatedUnionTypeDeclaration undiscriminatedUnion,
Set<String> reservedTypeNames,
String undiscriminatedUnionPrefix) {
Map<TypeId, TypeDeclaration> overriddenDeclarations = overriddenTypeDeclarations(
className, generatorContext, undiscriminatedUnion, undiscriminatedUnionPrefix);
className, generatorContext, undiscriminatedUnion, reservedTypeNames, undiscriminatedUnionPrefix);
Map<DeclaredTypeName, ClassName> mapperEnclosingClasses = new HashMap<>();

for (TypeDeclaration override : overriddenDeclarations.values()) {
Expand Down Expand Up @@ -403,4 +451,8 @@ private boolean shouldDeserializeWithTypeReference(UndiscriminatedUnionMember me
}
return false;
}

private static String visitorName(Set<String> reservedTypeNames) {
return reservedTypeNames.contains(VISITOR_CLASS_NAME) ? VISITOR_CLASS_NAME_UNDERSCORE : VISITOR_CLASS_NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonUnwrapped;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fern.ir.model.commons.Name;
import com.fern.ir.model.commons.NameAndWireValue;
import com.fern.ir.model.commons.SafeAndUnsafeString;
import com.fern.ir.model.commons.TypeId;
import com.fern.ir.model.constants.Constants;
import com.fern.ir.model.types.*;
Expand All @@ -14,6 +16,7 @@
import com.fern.java.generators.union.UnionTypeSpecGenerator;
import com.fern.java.utils.InlineTypeIdResolver;
import com.fern.java.utils.NamedTypeId;
import com.google.common.collect.ImmutableSet;
import com.squareup.javapoet.AnnotationSpec;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.FieldSpec;
Expand Down Expand Up @@ -46,13 +49,14 @@ public UnionGenerator(
super(className, generatorContext, reservedTypeNames, isTopLevelClass);
this.unionTypeDeclaration = unionTypeDeclaration;
this.overriddenTypeDeclarations =
overriddenTypeDeclarations(generatorContext, unionTypeDeclaration, reservedTypeNames);
overriddenTypeDeclarations(className, generatorContext, unionTypeDeclaration, reservedTypeNames);
}

@Override
public List<TypeDeclaration> getInlineTypeDeclarations() {
return new ArrayList<>(overriddenTypeDeclarations(generatorContext, unionTypeDeclaration, reservedTypeNames)
.values());
return new ArrayList<>(
overriddenTypeDeclarations(className, generatorContext, unionTypeDeclaration, reservedTypeNames)
.values());
}

@Override
Expand Down Expand Up @@ -96,6 +100,7 @@ private static PoetTypeNameMapper overriddenTypeNameMapper(
}

private static Map<TypeId, TypeDeclaration> overriddenTypeDeclarations(
ClassName className,
AbstractGeneratorContext<?, ?> generatorContext,
UnionTypeDeclaration unionTypeDeclaration,
Set<String> reservedTypeNames) {
Expand Down Expand Up @@ -186,7 +191,21 @@ private ModelUnionTypeSpecGenerator(
unionSubType,
fernConstants,
true,
unionTypeDeclaration.getDiscriminant().getWireValue());
unionTypeDeclaration.getDiscriminant().getWireValue(),
// We need to take into consideration all ancestor types as well as all sibling types so that
// to prevent naming the visitor "Visitor" if we already have a variant or property called that.
ImmutableSet.<String>builder()
.addAll(reservedTypeNames)
.addAll(
generatorContext.getCustomConfig().enableInlineTypes()
? overriddenTypeDeclarations.values().stream()
.map(TypeDeclaration::getName)
.map(DeclaredTypeName::getName)
.map(Name::getPascalCase)
.map(SafeAndUnsafeString::getSafeName)
.collect(Collectors.toList())
: List.of())
.build());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,23 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import javax.lang.model.element.Modifier;
import org.apache.commons.lang3.StringUtils;

public abstract class UnionTypeSpecGenerator {

private static final String VISITOR_CLASS_NAME = "Visitor";
private static final String VISITOR_CLASS_NAME_UNDERSCORE = "Visitor_";
private static final TypeVariableName VISITOR_RETURN_TYPE = TypeVariableName.get("T");

private final ClassName unionClassName;
private final ClassName valueInterfaceClassName;
private final List<? extends UnionSubType> subTypes;
private final UnionSubType unknownSubType;
private final Constants fernConstants;
private final String visitorName;
private final ParameterizedTypeName visitorInterfaceClassName;
private final boolean deserializable;
private final String discriminantProperty;
Expand All @@ -41,14 +44,16 @@ public UnionTypeSpecGenerator(
UnionSubType unknownSubType,
Constants fernConstants,
boolean deserializable,
String discriminantProperty) {
String discriminantProperty,
Set<String> reservedTypeNames) {
this.unionClassName = unionClassName;
this.subTypes = subTypes;
this.unknownSubType = unknownSubType;
this.fernConstants = fernConstants;
this.valueInterfaceClassName = unionClassName.nestedClass("Value");
this.visitorName = visitorName(reservedTypeNames);
this.visitorInterfaceClassName =
ParameterizedTypeName.get(unionClassName.nestedClass(VISITOR_CLASS_NAME), VISITOR_RETURN_TYPE);
ParameterizedTypeName.get(unionClassName.nestedClass(visitorName), VISITOR_RETURN_TYPE);
this.deserializable = deserializable;
this.discriminantProperty = discriminantProperty;
}
Expand Down Expand Up @@ -170,7 +175,7 @@ private Optional<MethodSpec> getSubTypeMethodSpec(UnionSubType subType) {
}

public final TypeSpec generateVisitorInterface() {
return TypeSpec.interfaceBuilder(VISITOR_CLASS_NAME)
return TypeSpec.interfaceBuilder(visitorName)
.addModifiers(Modifier.PUBLIC)
.addTypeVariable(VISITOR_RETURN_TYPE)
.addMethods(subTypes.stream()
Expand Down Expand Up @@ -220,4 +225,8 @@ public final ClassName getUnionClassName() {
public final ClassName getValueInterfaceClassName() {
return valueInterfaceClassName;
}

private static String visitorName(Set<String> reservedTypeNames) {
return reservedTypeNames.contains(VISITOR_CLASS_NAME) ? VISITOR_CLASS_NAME_UNDERSCORE : VISITOR_CLASS_NAME;
}
}
7 changes: 7 additions & 0 deletions generators/java/model/versions.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
- changelogEntry:
- summary: |
Fix union inline type name conflict resolution.
type: fix
createdAt: '2025-01-16'
irVersion: 53
version: 1.4.1
- changelogEntry:
- summary: |
Support inline types in the Java generator.
Expand Down
7 changes: 7 additions & 0 deletions generators/java/sdk/versions.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
- changelogEntry:
- summary: |
Fix union inline type name conflict resolution.
type: fix
createdAt: '2025-01-16'
irVersion: 53
version: 2.10.1
- changelogEntry:
- summary: |
Support inline types in the Java generator.
Expand Down
7 changes: 7 additions & 0 deletions generators/java/spring/versions.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
- changelogEntry:
- summary: |
Fix union inline type name conflict resolution.
type: fix
createdAt: '2025-01-16'
irVersion: 53
version: 1.4.1
- changelogEntry:
- summary: |
Support inline types in the Java generator.
Expand Down

0 comments on commit b5e3635

Please sign in to comment.