Skip to content

Commit

Permalink
fix(java): Omit methods with inlined types from interface definitions (
Browse files Browse the repository at this point in the history
…#5639)

* fix(java): Fix union type name conflict resolution

* Add version yamls

* fix(java): Omit methods with inline types in interfaces

* make distinction between rendered and actual method specs, and ensure to check inlined status properly

* chore: update changelog

* Don't add an override label to non-rendered interface fields

* Add version entries

---------

Co-authored-by: Alberto <[email protected]>
  • Loading branch information
ajgateno and Alberto authored Jan 17, 2025
1 parent b5e3635 commit 48a037e
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.fern.java.AbstractGeneratorContext;
import com.fern.java.output.GeneratedJavaInterface;
import com.fern.java.output.GeneratedJavaInterface.PropertyMethodSpec;
import com.fern.java.utils.TypeReferenceInlineChecker;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.JavaFile;
import com.squareup.javapoet.MethodSpec;
Expand All @@ -44,14 +45,15 @@ public InterfaceGenerator(
@Override
public GeneratedJavaInterface generateFile() {
List<PropertyMethodSpec> methodSpecsByProperties = getPropertyGetters();
List<PropertyMethodSpec> renderedSpecsByProperties = getRenderedPropertyGetters();
List<ClassName> superInterfaces = objectTypeDeclaration.getExtends().stream()
.map(declaredTypeName ->
generatorContext.getPoetClassNameFactory().getInterfaceClassName(declaredTypeName))
.collect(Collectors.toList());
TypeSpec interfaceTypeSpec = TypeSpec.interfaceBuilder(className)
.addModifiers(Modifier.PUBLIC)
.addSuperinterfaces(superInterfaces)
.addMethods(methodSpecsByProperties.stream()
.addMethods(renderedSpecsByProperties.stream()
.map(PropertyMethodSpec::methodSpec)
.collect(Collectors.toList()))
.build();
Expand Down Expand Up @@ -87,4 +89,34 @@ private List<PropertyMethodSpec> getPropertyGetters() {
})
.collect(Collectors.toList());
}

private List<PropertyMethodSpec> getRenderedPropertyGetters() {
return objectTypeDeclaration.getProperties().stream()
// Omit any types we're going to inline
.filter(objectProperty -> {
if (!generatorContext.getCustomConfig().enableInlineTypes()) {
return true;
}
return !objectProperty.getValueType().visit(new TypeReferenceInlineChecker(generatorContext));
})
.map(objectProperty -> {
TypeName poetTypeName = generatorContext
.getPoetTypeNameMapper()
.convertToTypeName(true, objectProperty.getValueType());
MethodSpec getter = MethodSpec.methodBuilder("get"
+ objectProperty
.getName()
.getName()
.getPascalCase()
.getSafeName())
.addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
.returns(poetTypeName)
.build();
return PropertyMethodSpec.builder()
.objectProperty(objectProperty)
.methodSpec(getter)
.build();
})
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.fern.java.output.GeneratedObject;
import com.fern.java.utils.InlineTypeIdResolver;
import com.fern.java.utils.NamedTypeId;
import com.fern.java.utils.TypeReferenceInlineChecker;
import com.google.common.collect.ImmutableMap;
import com.palantir.common.streams.KeyedStream;
import com.squareup.javapoet.ClassName;
Expand Down Expand Up @@ -71,8 +72,8 @@ public ObjectGenerator(
getUniqueAncestorsInLevelOrder(allExtendedInterfaces, allGeneratedInterfaces);

List<EnrichedObjectProperty> enriched = enrichedObjectProperties(
selfInterface, objectTypeDeclaration, generatorContext.getPoetTypeNameMapper());
List<ImplementsInterface> implemented = implementsInterfaces(ancestors);
generatorContext, selfInterface, objectTypeDeclaration, generatorContext.getPoetTypeNameMapper());
List<ImplementsInterface> implemented = implementsInterfaces(generatorContext, ancestors);

if (generatorContext.getCustomConfig().enableInlineTypes()) {
List<EnrichedObjectProperty> allEnrichedProperties = new ArrayList<>();
Expand Down Expand Up @@ -240,12 +241,13 @@ private static Map<EnrichedObjectProperty, EnrichedObjectProperty> overridePrope

for (EnrichedObjectProperty prop : enrichedObjectProperties) {
TypeName typeName = overriddenMapper.convertToTypeName(
false, prop.objectProperty().getValueType());
true, prop.objectProperty().getValueType());
EnrichedObjectProperty overridden = EnrichedObjectProperty.builder()
.camelCaseKey(prop.camelCaseKey())
.pascalCaseKey(prop.pascalCaseKey())
.poetTypeName(typeName)
.fromInterface(prop.fromInterface())
.inline(true)
.objectProperty(prop.objectProperty())
.wireKey(prop.wireKey())
.docs(prop.docs())
Expand All @@ -257,35 +259,51 @@ private static Map<EnrichedObjectProperty, EnrichedObjectProperty> overridePrope
return result;
}

private static List<ImplementsInterface> implementsInterfaces(List<GeneratedJavaInterface> ancestors) {
private static List<ImplementsInterface> implementsInterfaces(
AbstractGeneratorContext<?, ?> generatorContext, List<GeneratedJavaInterface> ancestors) {
return ancestors.stream()
.map(generatedInterface -> ImplementsInterface.builder()
.interfaceClassName(generatedInterface.getClassName())
.addAllInterfaceProperties(getEnrichedObjectProperties(generatedInterface))
.addAllInterfaceProperties(getEnrichedObjectProperties(generatorContext, generatedInterface))
.build())
.collect(Collectors.toList());
}

private static List<EnrichedObjectProperty> enrichedObjectProperties(
AbstractGeneratorContext<?, ?> generatorContext,
Optional<GeneratedJavaInterface> selfInterface,
ObjectTypeDeclaration objectTypeDeclaration,
PoetTypeNameMapper poetTypeNameMapper) {
if (selfInterface.isEmpty()) {
return objectTypeDeclaration.getProperties().stream()
.map(objectProperty -> EnrichedObjectProperty.of(
objectProperty,
false,
poetTypeNameMapper.convertToTypeName(true, objectProperty.getValueType())))
.map(objectProperty -> {
boolean inline =
objectProperty.getValueType().visit(new TypeReferenceInlineChecker(generatorContext));
return EnrichedObjectProperty.of(
objectProperty,
false,
inline,
poetTypeNameMapper.convertToTypeName(true, objectProperty.getValueType()));
})
.collect(Collectors.toList());
}
return List.of();
}

private static List<EnrichedObjectProperty> getEnrichedObjectProperties(
GeneratedJavaInterface generatedJavaInterface) {
AbstractGeneratorContext<?, ?> generatorContext, GeneratedJavaInterface generatedJavaInterface) {
return generatedJavaInterface.propertyMethodSpecs().stream()
.map(propertyMethodSpec -> EnrichedObjectProperty.of(
propertyMethodSpec.objectProperty(), true, propertyMethodSpec.methodSpec().returnType))
.map(propertyMethodSpec -> {
boolean inline = propertyMethodSpec
.objectProperty()
.getValueType()
.visit(new TypeReferenceInlineChecker(generatorContext));
return EnrichedObjectProperty.of(
propertyMethodSpec.objectProperty(),
true,
inline,
propertyMethodSpec.methodSpec().returnType);
})
.collect(Collectors.toList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ private static Map<UndiscriminatedUnionMember, TypeName> overrideMemberTypeNames
mapperEnclosingClasses);

for (UndiscriminatedUnionMember member : undiscriminatedUnion.getMembers()) {
TypeName typeName = overriddenMapper.convertToTypeName(false, member.getType());
TypeName typeName = overriddenMapper.convertToTypeName(true, member.getType());
result.put(member, typeName);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public interface EnrichedObjectProperty {

boolean fromInterface();

boolean inline();

Optional<String> docs();

Optional<Literal> literal();
Expand Down Expand Up @@ -83,7 +85,7 @@ public Void _visitUnknown(Object unknownType) {
.addMember("value", "$S", wireKey().get())
.build());
}
if (fromInterface()) {
if (fromInterface() && !inline()) {
getterBuilder.addAnnotation(ClassName.get("", "java.lang.Override"));
}
if (docs().isPresent()) {
Expand All @@ -96,7 +98,8 @@ static ImmutableEnrichedObjectProperty.CamelCaseKeyBuildStage builder() {
return ImmutableEnrichedObjectProperty.builder();
}

static EnrichedObjectProperty of(ObjectProperty objectProperty, boolean fromInterface, TypeName poetTypeName) {
static EnrichedObjectProperty of(
ObjectProperty objectProperty, boolean fromInterface, boolean inline, TypeName poetTypeName) {
Name name = objectProperty.getName().getName();
Optional<Literal> maybeLiteral =
objectProperty.getValueType().getContainer().flatMap(ContainerType::getLiteral);
Expand All @@ -105,6 +108,7 @@ static EnrichedObjectProperty of(ObjectProperty objectProperty, boolean fromInte
.pascalCaseKey(name.getPascalCase().getSafeName())
.poetTypeName(poetTypeName)
.fromInterface(fromInterface)
.inline(inline)
.objectProperty(objectProperty)
.wireKey(objectProperty.getName().getWireValue())
.docs(objectProperty.getDocs())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package com.fern.java.utils;

import com.fern.ir.model.types.ContainerType;
import com.fern.ir.model.types.Literal;
import com.fern.ir.model.types.MapType;
import com.fern.ir.model.types.NamedType;
import com.fern.ir.model.types.PrimitiveType;
import com.fern.ir.model.types.TypeDeclaration;
import com.fern.ir.model.types.TypeReference;
import com.fern.java.AbstractGeneratorContext;
import java.util.Optional;

public class TypeReferenceInlineChecker implements TypeReference.Visitor<Boolean>, ContainerType.Visitor<Boolean> {

private final AbstractGeneratorContext<?, ?> generatorContext;

public TypeReferenceInlineChecker(AbstractGeneratorContext<?, ?> generatorContext) {
this.generatorContext = generatorContext;
}

// Handle main types

@Override
public Boolean visitContainer(ContainerType containerType) {
return containerType.visit(this);
}

@Override
public Boolean visitNamed(NamedType namedType) {
// TODO(ajgateno): Tracking in FER-4050 that we can't just get inline from namedType.getInline()
Optional<TypeDeclaration> existingTypeDeclaration =
Optional.ofNullable(generatorContext.getTypeDeclarations().get(namedType.getTypeId()));
return existingTypeDeclaration
.map(declaration -> declaration.getInline().orElse(false))
.orElse(false);
}

@Override
public Boolean visitPrimitive(PrimitiveType primitiveType) {
return false;
}

@Override
public Boolean visitUnknown() {
return false;
}

// Handle container types

@Override
public Boolean visitList(TypeReference typeReference) {
return typeReference.visit(this);
}

@Override
public Boolean visitMap(MapType mapType) {
return mapType.getKeyType().visit(this) || mapType.getValueType().visit(this);
}

@Override
public Boolean visitOptional(TypeReference typeReference) {
return typeReference.visit(this);
}

@Override
public Boolean visitSet(TypeReference typeReference) {
return typeReference.visit(this);
}

@Override
public Boolean visitLiteral(Literal literal) {
return false;
}

// Unknown

@Override
public Boolean _visitUnknown(Object o) {
return false;
}
}
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: |
Omit methods with inlined types from interface definitions.
type: fix
createdAt: '2025-01-16'
irVersion: 53
version: 1.4.2
- changelogEntry:
- summary: |
Fix union inline type name conflict resolution.
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: |
Omit methods with inlined types from interface definitions.
type: fix
createdAt: '2025-01-16'
irVersion: 53
version: 2.10.2
- changelogEntry:
- summary: |
Fix union inline type name conflict resolution.
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: |
Omit methods with inlined types from interface definitions.
type: fix
createdAt: '2025-01-16'
irVersion: 53
version: 1.4.2
- changelogEntry:
- summary: |
Fix union inline type name conflict resolution.
Expand Down

0 comments on commit 48a037e

Please sign in to comment.