From b5e3635950f0c299fdb856c431406be154b1dec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Gate=C3=B1o?= <50761868+ajgateno@users.noreply.github.com> Date: Thu, 16 Jan 2025 19:24:01 -0500 Subject: [PATCH] fix(java): Fix union type name conflict resolution (#5637) * fix(java): Fix union type name conflict resolution * Add version yamls * fix typo * chore: update changelog --------- Co-authored-by: Alberto --- .../{2025-01-16.mdx => 2025-01-17.mdx} | 0 .../generators/AbstractTypeGenerator.java | 1 - .../UndiscriminatedUnionGenerator.java | 60 +++++++++++++++++-- .../fern/java/generators/UnionGenerator.java | 27 +++++++-- .../union/UnionTypeSpecGenerator.java | 15 ++++- generators/java/model/versions.yml | 7 +++ generators/java/sdk/versions.yml | 7 +++ generators/java/spring/versions.yml | 7 +++ 8 files changed, 112 insertions(+), 12 deletions(-) rename fern/pages/changelogs/ts-express/{2025-01-16.mdx => 2025-01-17.mdx} (100%) diff --git a/fern/pages/changelogs/ts-express/2025-01-16.mdx b/fern/pages/changelogs/ts-express/2025-01-17.mdx similarity index 100% rename from fern/pages/changelogs/ts-express/2025-01-16.mdx rename to fern/pages/changelogs/ts-express/2025-01-17.mdx diff --git a/generators/java/generator-utils/src/main/java/com/fern/java/generators/AbstractTypeGenerator.java b/generators/java/generator-utils/src/main/java/com/fern/java/generators/AbstractTypeGenerator.java index f167a331b74..f0fa1d2160e 100644 --- a/generators/java/generator-utils/src/main/java/com/fern/java/generators/AbstractTypeGenerator.java +++ b/generators/java/generator-utils/src/main/java/com/fern/java/generators/AbstractTypeGenerator.java @@ -65,7 +65,6 @@ protected List getInlineTypeSpecs() { false, ImmutableSet.builder() .addAll(reservedTypeNames) - .add(className.simpleName()) .add(name) .build(), false)); diff --git a/generators/java/generator-utils/src/main/java/com/fern/java/generators/UndiscriminatedUnionGenerator.java b/generators/java/generator-utils/src/main/java/com/fern/java/generators/UndiscriminatedUnionGenerator.java index 407b660a370..38f89219bc6 100644 --- a/generators/java/generator-utils/src/main/java/com/fern/java/generators/UndiscriminatedUnionGenerator.java +++ b/generators/java/generator-utils/src/main/java/com/fern/java/generators/UndiscriminatedUnionGenerator.java @@ -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; @@ -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; @@ -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; @@ -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"; @@ -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, @@ -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.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; } @@ -120,7 +149,11 @@ public static Set getDuplicatedOuterContainerTypes( @Override public List getInlineTypeDeclarations() { return new ArrayList<>(overriddenTypeDeclarations( - className, generatorContext, undiscriminatedUnion, undiscriminatedUnionPrefix) + className, + generatorContext, + undiscriminatedUnion, + reservedTypeNames, + undiscriminatedUnionPrefix) .values()); } @@ -128,6 +161,7 @@ private static Map overriddenTypeDeclarations( ClassName className, AbstractGeneratorContext generatorContext, UndiscriminatedUnionTypeDeclaration undiscriminatedUnion, + Set reservedTypeNames, String undiscriminatedUnionPrefix) { Map overriddenTypeDeclarations = new HashMap<>(); @@ -149,6 +183,8 @@ private static Map overriddenTypeDeclarations( continue; } + Set allReservedTypeNames = new HashSet<>(reservedTypeNames); + String name = rawTypeDeclaration.getName().getName().getPascalCase().getSafeName(); @@ -157,6 +193,17 @@ private static Map 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); } @@ -169,9 +216,10 @@ private static Map overrideMemberTypeNames ClassName className, AbstractGeneratorContext generatorContext, UndiscriminatedUnionTypeDeclaration undiscriminatedUnion, + Set reservedTypeNames, String undiscriminatedUnionPrefix) { Map overriddenDeclarations = overriddenTypeDeclarations( - className, generatorContext, undiscriminatedUnion, undiscriminatedUnionPrefix); + className, generatorContext, undiscriminatedUnion, reservedTypeNames, undiscriminatedUnionPrefix); Map mapperEnclosingClasses = new HashMap<>(); for (TypeDeclaration override : overriddenDeclarations.values()) { @@ -403,4 +451,8 @@ private boolean shouldDeserializeWithTypeReference(UndiscriminatedUnionMember me } return false; } + + private static String visitorName(Set reservedTypeNames) { + return reservedTypeNames.contains(VISITOR_CLASS_NAME) ? VISITOR_CLASS_NAME_UNDERSCORE : VISITOR_CLASS_NAME; + } } diff --git a/generators/java/generator-utils/src/main/java/com/fern/java/generators/UnionGenerator.java b/generators/java/generator-utils/src/main/java/com/fern/java/generators/UnionGenerator.java index 7ee5a355855..82c78b11700 100644 --- a/generators/java/generator-utils/src/main/java/com/fern/java/generators/UnionGenerator.java +++ b/generators/java/generator-utils/src/main/java/com/fern/java/generators/UnionGenerator.java @@ -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.*; @@ -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; @@ -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 getInlineTypeDeclarations() { - return new ArrayList<>(overriddenTypeDeclarations(generatorContext, unionTypeDeclaration, reservedTypeNames) - .values()); + return new ArrayList<>( + overriddenTypeDeclarations(className, generatorContext, unionTypeDeclaration, reservedTypeNames) + .values()); } @Override @@ -96,6 +100,7 @@ private static PoetTypeNameMapper overriddenTypeNameMapper( } private static Map overriddenTypeDeclarations( + ClassName className, AbstractGeneratorContext generatorContext, UnionTypeDeclaration unionTypeDeclaration, Set reservedTypeNames) { @@ -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.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 diff --git a/generators/java/generator-utils/src/main/java/com/fern/java/generators/union/UnionTypeSpecGenerator.java b/generators/java/generator-utils/src/main/java/com/fern/java/generators/union/UnionTypeSpecGenerator.java index ee5e239f164..d0cd1fafd59 100644 --- a/generators/java/generator-utils/src/main/java/com/fern/java/generators/union/UnionTypeSpecGenerator.java +++ b/generators/java/generator-utils/src/main/java/com/fern/java/generators/union/UnionTypeSpecGenerator.java @@ -17,6 +17,7 @@ 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; @@ -24,6 +25,7 @@ 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; @@ -31,6 +33,7 @@ public abstract class UnionTypeSpecGenerator { private final List 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; @@ -41,14 +44,16 @@ public UnionTypeSpecGenerator( UnionSubType unknownSubType, Constants fernConstants, boolean deserializable, - String discriminantProperty) { + String discriminantProperty, + Set 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; } @@ -170,7 +175,7 @@ private Optional 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() @@ -220,4 +225,8 @@ public final ClassName getUnionClassName() { public final ClassName getValueInterfaceClassName() { return valueInterfaceClassName; } + + private static String visitorName(Set reservedTypeNames) { + return reservedTypeNames.contains(VISITOR_CLASS_NAME) ? VISITOR_CLASS_NAME_UNDERSCORE : VISITOR_CLASS_NAME; + } } diff --git a/generators/java/model/versions.yml b/generators/java/model/versions.yml index b663389ebf6..67b00dc1e3c 100644 --- a/generators/java/model/versions.yml +++ b/generators/java/model/versions.yml @@ -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. diff --git a/generators/java/sdk/versions.yml b/generators/java/sdk/versions.yml index 2eb2b3a4c5c..e3240569517 100644 --- a/generators/java/sdk/versions.yml +++ b/generators/java/sdk/versions.yml @@ -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. diff --git a/generators/java/spring/versions.yml b/generators/java/spring/versions.yml index a881fe3e578..56ab4b0a07c 100644 --- a/generators/java/spring/versions.yml +++ b/generators/java/spring/versions.yml @@ -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.