Skip to content

Commit 4f00a34

Browse files
authored
[OCaml][Fix] Unreferenced enum + Direct recursive types (#23005)
* [OCaml] Fix bugs around enum parsing Problem: The OCaml client generator threw IllegalArgumentException: Unreferenced enum when encountering enums inside composed schemas (anyOf/allOf/oneOf). Root Causes: 1. The enum collection logic didn't traverse into composed schemas 2. The enum hashing used order-dependent string concatenation, causing lookups to fail when enum values appeared in different orders 3. Enums directly within composed schema branches (not in properties) weren't collected Solution: 1. Added composed schema support: - New `collectEnumSchemasFromComposed()` method handles `anyOf/allOf/oneOf` - New `collectEnumSchemasFromList()` method recursively processes composed schema branches - Enums directly in composed schemas (not just in properties) are now collected 2. Refactored enum hashing to use Set: - Changed from comma-joined strings to `TreeSet<String>` for order-independent, collision-free hashing - Handles edge cases like empty string enums `""` 3. Added test case: - Tests enums in nested composed schemas - Tests enum with empty string value in anyOf * OCaml: Add support for direct recursive types * OCaml: Fix enums in anyOf * OCaml: fix recursive types * Fix recursion tests * Fix recursive types, improve tests * [OCaml] Improve title of generated README.md
1 parent 1894a07 commit 4f00a34

54 files changed

Lines changed: 1406 additions & 106 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/samples-ocaml.yaml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,16 @@ on:
77
- 'samples/client/petstore/ocaml-fake-petstore/**'
88
- 'samples/client/petstore/ocaml-oneOf-primitive/**'
99
- 'samples/client/petstore/ocaml-additional-properties/**'
10+
- 'samples/client/petstore/ocaml-enum-in-composed-schema/**'
11+
- 'samples/client/petstore/ocaml-recursion-test/**'
1012
pull_request:
1113
paths:
1214
- 'samples/client/petstore/ocaml/**'
1315
- 'samples/client/petstore/ocaml-fake-petstore/**'
1416
- 'samples/client/petstore/ocaml-oneOf-primitive/**'
1517
- 'samples/client/petstore/ocaml-additional-properties/**'
18+
- 'samples/client/petstore/ocaml-enum-in-composed-schema/**'
19+
- 'samples/client/petstore/ocaml-recursion-test/**'
1620

1721
jobs:
1822
build:
@@ -26,12 +30,14 @@ jobs:
2630
- 'samples/client/petstore/ocaml-fake-petstore/'
2731
- 'samples/client/petstore/ocaml-oneOf-primitive/'
2832
- 'samples/client/petstore/ocaml-additional-properties/'
33+
- 'samples/client/petstore/ocaml-enum-in-composed-schema/'
34+
- 'samples/client/petstore/ocaml-recursion-test/'
2935
steps:
3036
- uses: actions/checkout@v5
3137
- name: Set-up OCaml
3238
uses: ocaml/setup-ocaml@v3
3339
with:
34-
ocaml-compiler: 5
40+
ocaml-compiler: 5.3
3541
- name: Install
3642
run: opam install . --deps-only --with-test
3743
working-directory: ${{ matrix.sample }}

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,8 @@ samples/client/petstore/ocaml/_build/
304304
samples/client/petstore/ocaml-fake-petstore/_build/
305305
samples/client/petstore/ocaml-oneOf-primitive/_build/
306306
samples/client/petstore/ocaml-additional-properties/_build/
307+
samples/client/petstore/ocaml-enum-in-composed-schema/_build/
308+
samples/client/petstore/ocaml-recursion-test/_build/
307309

308310
# jetbrain http client
309311
samples/client/jetbrains/adyen/checkout71/http/client/Apis/http-client.private.env.json
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
generatorName: ocaml
2+
outputDir: samples/client/petstore/ocaml-enum-in-composed-schema
3+
inputSpec: modules/openapi-generator/src/test/resources/3_0/ocaml/enum-in-composed-schema.yaml
4+
templateDir: modules/openapi-generator/src/main/resources/ocaml
5+
additionalProperties:
6+
packageName: petstore_client
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
generatorName: ocaml
2+
outputDir: samples/client/petstore/ocaml-recursion-test
3+
inputSpec: modules/openapi-generator/src/test/resources/3_0/ocaml/direct-recursion.yaml
4+
templateDir: modules/openapi-generator/src/main/resources/ocaml
5+
additionalProperties:
6+
packageName: recursion_test

docs/generators/ocaml.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ title: Documentation for the ocaml Generator
1111
| generator type | CLIENT | |
1212
| generator language | OCaml | |
1313
| generator default templating engine | mustache | |
14-
| helpTxt | Generates an OCaml client library (beta). | |
14+
| helpTxt | Generates an OCaml client library. | |
1515

1616
## CONFIG OPTIONS
1717
These options may be applied as additional-properties (cli) or configOptions (plugins). Refer to [configuration docs](https://openapi-generator.tech/docs/configuration) for more details.

modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/OCamlClientCodegen.java

Lines changed: 162 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,18 @@ public class OCamlClientCodegen extends DefaultCodegen implements CodegenConfig
5151

5252
static final String X_MODEL_MODULE = "x-model-module";
5353

54-
@Setter protected String packageName = "openapi";
55-
@Setter protected String packageVersion = "1.0.0";
54+
@Setter
55+
protected String packageName = "openapi";
56+
@Setter
57+
protected String packageVersion = "1.0.0";
5658
protected String apiDocPath = "docs/";
5759
protected String modelDocPath = "docs/";
5860
protected String apiFolder = "src/apis";
5961
protected String modelFolder = "src/models";
6062

61-
private Map<String, List<String>> enumNames = new HashMap<>();
62-
private Map<String, Schema> enumHash = new HashMap<>();
63-
private Map<String, String> enumUniqNames;
63+
private Map<Set<String>, List<String>> enumNames = new HashMap<>();
64+
private Map<Set<String>, Schema> enumHash = new HashMap<>();
65+
private Map<Set<String>, String> enumUniqNames;
6466

6567
@Override
6668
public CodegenType getTag() {
@@ -74,7 +76,7 @@ public String getName() {
7476

7577
@Override
7678
public String getHelp() {
77-
return "Generates an OCaml client library (beta).";
79+
return "Generates an OCaml client library.";
7880
}
7981

8082
public OCamlClientCodegen() {
@@ -233,6 +235,105 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> supero
233235

234236
}
235237

238+
/**
239+
* Add support for direct recursive types (e.g., A -> A).
240+
* This does *not* support mutually recursive types (e.g., A -> B -> A), as this is a much more complex beast in OCaml (since mutually recursive types must live in the same file).
241+
*/
242+
@Override
243+
public ModelsMap postProcessModels(ModelsMap objs) {
244+
objs = super.postProcessModels(objs);
245+
246+
for (ModelMap mo : objs.getModels()) {
247+
CodegenModel cm = mo.getModel();
248+
249+
// Check if any property is a self-reference
250+
boolean hasSelfRef = cm.allVars.stream()
251+
.anyMatch(prop -> prop.isSelfReference);
252+
253+
if (hasSelfRef) {
254+
// Collect names of self-referencing properties
255+
Set<String> selfRefPropNames = cm.allVars.stream()
256+
.filter(p -> p.isSelfReference)
257+
.map(p -> p.name)
258+
.collect(Collectors.toSet());
259+
260+
// The property lists (vars, allVars, etc.) contain DIFFERENT objects
261+
// Match by name since isSelfReference might only be set in allVars
262+
List<List<CodegenProperty>> allPropertyLists = Arrays.asList(
263+
cm.allVars, cm.vars, cm.requiredVars, cm.optionalVars,
264+
cm.readOnlyVars, cm.readWriteVars, cm.parentVars
265+
);
266+
267+
for (List<CodegenProperty> propList : allPropertyLists) {
268+
for (CodegenProperty prop : propList) {
269+
if (selfRefPropNames.contains(prop.name)) {
270+
if (prop.isContainer && prop.items != null) {
271+
// For containers, update items and reconstruct the container type
272+
prop.items.dataType = "t";
273+
prop.items.datatypeWithEnum = "t";
274+
if (prop.items.baseType != null) {
275+
prop.items.baseType = "t";
276+
}
277+
if (prop.items.complexType != null) {
278+
prop.items.complexType = "t";
279+
}
280+
281+
// Reconstruct the container type based on the updated items
282+
if (prop.isArray) {
283+
prop.dataType = "t list";
284+
prop.datatypeWithEnum = "t list";
285+
} else if (prop.isMap) {
286+
prop.dataType = "(string * t) list";
287+
prop.datatypeWithEnum = "(string * t) list";
288+
}
289+
} else {
290+
// For non-containers, just replace the type directly
291+
prop.dataType = "t";
292+
prop.datatypeWithEnum = "t";
293+
}
294+
295+
// Update baseType and complexType for all cases
296+
if (prop.baseType != null) {
297+
prop.baseType = "t";
298+
}
299+
if (prop.complexType != null) {
300+
prop.complexType = "t";
301+
}
302+
}
303+
}
304+
}
305+
}
306+
307+
// Fix enum references in composed schemas (anyOf, oneOf, allOf)
308+
if (cm.getComposedSchemas() != null) {
309+
fixEnumReferencesInComposedSchemas(cm.getComposedSchemas().getAnyOf());
310+
fixEnumReferencesInComposedSchemas(cm.getComposedSchemas().getOneOf());
311+
fixEnumReferencesInComposedSchemas(cm.getComposedSchemas().getAllOf());
312+
}
313+
}
314+
315+
return objs;
316+
}
317+
318+
private void fixEnumReferencesInComposedSchemas(List<CodegenProperty> schemas) {
319+
if (schemas == null) {
320+
return;
321+
}
322+
323+
for (CodegenProperty schema : schemas) {
324+
// If this schema is an enum, add Enums. prefix to datatypeWithEnum
325+
if (schema.isEnum) {
326+
if (!schema.datatypeWithEnum.startsWith("Enums.")) {
327+
schema.datatypeWithEnum = "Enums." + schema.datatypeWithEnum;
328+
}
329+
// Also update dataType for the variant constructor
330+
if (!schema.dataType.startsWith("Enums.")) {
331+
schema.dataType = "Enums." + schema.dataType;
332+
}
333+
}
334+
}
335+
}
336+
236337
private void enrichPropertiesWithEnumDefaultValues(List<CodegenProperty> properties) {
237338
for (CodegenProperty property : properties) {
238339
if (property.get_enum() != null && property.get_enum().size() == 1) {
@@ -276,8 +377,10 @@ protected void updateDataTypeWithEnumForArray(CodegenProperty property) {
276377
}
277378

278379
@SuppressWarnings("unchecked")
279-
private String hashEnum(Schema schema) {
280-
return ((List<Object>) schema.getEnum()).stream().map(String::valueOf).collect(Collectors.joining(","));
380+
private Set<String> hashEnum(Schema schema) {
381+
return ((List<Object>) schema.getEnum()).stream()
382+
.map(String::valueOf)
383+
.collect(Collectors.toCollection(TreeSet::new));
281384
}
282385

283386
private boolean isEnumSchema(Schema schema) {
@@ -290,7 +393,7 @@ private void collectEnumSchemas(String parentName, String sName, Schema schema)
290393
} else if (ModelUtils.isMapSchema(schema) && schema.getAdditionalProperties() instanceof Schema) {
291394
collectEnumSchemas(parentName, sName, (Schema) schema.getAdditionalProperties());
292395
} else if (isEnumSchema(schema)) {
293-
String h = hashEnum(schema);
396+
Set<String> h = hashEnum(schema);
294397
if (!enumHash.containsKey(h)) {
295398
enumHash.put(h, schema);
296399
enumNames.computeIfAbsent(h, k -> new ArrayList<>()).add(sName.toLowerCase(Locale.ROOT));
@@ -299,6 +402,8 @@ private void collectEnumSchemas(String parentName, String sName, Schema schema)
299402
}
300403
}
301404
}
405+
// Note: Composed schemas (anyOf, allOf, oneOf) are handled in the Map-based method
406+
// via collectEnumSchemasFromComposed() which properly processes their structure
302407
}
303408

304409
private void collectEnumSchemas(String sName, Schema schema) {
@@ -327,6 +432,47 @@ private void collectEnumSchemas(String parentName, Map<String, Schema> schemas)
327432
collectEnumSchemas(pName, ModelUtils.getSchemaItems(schema));
328433
}
329434
}
435+
436+
// Handle composed schemas (anyOf, allOf, oneOf) - recursively process their structure
437+
collectEnumSchemasFromComposed(pName, schema);
438+
}
439+
}
440+
441+
private void collectEnumSchemasFromComposed(String parentName, Schema schema) {
442+
if (schema.getAnyOf() != null) {
443+
collectEnumSchemasFromList(parentName, schema.getAnyOf());
444+
}
445+
446+
if (schema.getAllOf() != null) {
447+
collectEnumSchemasFromList(parentName, schema.getAllOf());
448+
}
449+
450+
if (schema.getOneOf() != null) {
451+
collectEnumSchemasFromList(parentName, schema.getOneOf());
452+
}
453+
}
454+
455+
private void collectEnumSchemasFromList(String parentName, List<Schema> schemas) {
456+
int index = 0;
457+
for (Schema composedSchema : schemas) {
458+
// Check if the composed schema itself is an enum
459+
if (isEnumSchema(composedSchema)) {
460+
String enumName = composedSchema.getName() != null ? composedSchema.getName() : "any_of_" + index;
461+
collectEnumSchemas(parentName, enumName, composedSchema);
462+
}
463+
464+
if (composedSchema.getProperties() != null) {
465+
collectEnumSchemas(parentName, composedSchema.getProperties());
466+
}
467+
if (composedSchema.getAdditionalProperties() != null && composedSchema.getAdditionalProperties() instanceof Schema) {
468+
collectEnumSchemas(parentName, composedSchema.getName(), (Schema) composedSchema.getAdditionalProperties());
469+
}
470+
if (ModelUtils.isArraySchema(composedSchema) && ModelUtils.getSchemaItems(composedSchema) != null) {
471+
collectEnumSchemas(parentName, composedSchema.getName(), ModelUtils.getSchemaItems(composedSchema));
472+
}
473+
// Recursively handle nested composed schemas
474+
collectEnumSchemasFromComposed(parentName, composedSchema);
475+
index++;
330476
}
331477
}
332478

@@ -378,8 +524,8 @@ private String sanitizeOCamlTypeName(String name) {
378524
}
379525

380526
private void computeEnumUniqNames() {
381-
Map<String, String> definitiveNames = new HashMap<>();
382-
for (String h : enumNames.keySet()) {
527+
Map<String, Set<String>> definitiveNames = new HashMap<>();
528+
for (Set<String> h : enumNames.keySet()) {
383529
boolean hasDefName = false;
384530
List<String> nameCandidates = enumNames.get(h);
385531
for (String name : nameCandidates) {
@@ -600,13 +746,13 @@ public String getTypeDeclaration(Schema p) {
600746
String prefix = inner.getEnum() != null ? "Enums." : "";
601747
return "(string * " + prefix + getTypeDeclaration(inner) + ") list";
602748
} else if (p.getEnum() != null) {
603-
String h = hashEnum(p);
749+
Set<String> h = hashEnum(p);
604750
return enumUniqNames.get(h);
605751
}
606752

607753
Schema referencedSchema = ModelUtils.getReferencedSchema(openAPI, p);
608754
if (referencedSchema != null && referencedSchema.getEnum() != null) {
609-
String h = hashEnum(referencedSchema);
755+
Set<String> h = hashEnum(referencedSchema);
610756
return "Enums." + enumUniqNames.get(h);
611757
}
612758

@@ -739,8 +885,8 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
739885
}
740886
}
741887

742-
for (Map.Entry<String, String> e : enumUniqNames.entrySet()) {
743-
allModels.add(buildEnumModelWrapper(e.getValue(), e.getKey()));
888+
for (Map.Entry<Set<String>, String> e : enumUniqNames.entrySet()) {
889+
allModels.add(buildEnumModelWrapper(e.getValue(), String.join(",", e.getKey())));
744890
}
745891

746892
enumUniqNames.clear();
@@ -770,7 +916,7 @@ public String escapeUnsafeCharacters(String input) {
770916

771917
@Override
772918
public String toEnumName(CodegenProperty property) {
773-
String hash = String.join(",", property.get_enum());
919+
Set<String> hash = new TreeSet<>(property.get_enum());
774920

775921
if (enumUniqNames.containsKey(hash)) {
776922
return enumUniqNames.get(hash);

modules/openapi-generator/src/main/resources/ocaml/readme.mustache

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# {{{projectName}}}
1+
# OCaml API client for {{{packageName}}}
22
{{#appDescriptionWithNewLines}}
33
{{{.}}}
44
{{/appDescriptionWithNewLines}}

0 commit comments

Comments
 (0)