Skip to content

Commit 5a1a8cd

Browse files
authored
feat(jsii-pacmak): document union types in JavaDoc (#4922)
In some places union types are rendered to overloaded setters, and in other places the type is collapsed to `Object`. Whenever the type is collapsed to `Object`, any other type information is lost for the user. In this PR, emit the union type information to JavaDocs so users can find it again. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
1 parent 3326ffa commit 5a1a8cd

File tree

2 files changed

+165
-73
lines changed

2 files changed

+165
-73
lines changed

packages/jsii-pacmak/lib/targets/java.ts

Lines changed: 120 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -900,11 +900,17 @@ class JavaGenerator extends Generator {
900900
? this.toDecoratedJavaType(method.returns)
901901
: 'void';
902902
const methodName = JavaGenerator.safeJavaMethodName(method.name);
903-
this.addJavaDocs(method, {
904-
api: 'member',
905-
fqn: ifc.fqn,
906-
memberName: methodName,
907-
});
903+
this.addJavaDocs(
904+
method,
905+
{
906+
api: 'member',
907+
fqn: ifc.fqn,
908+
memberName: methodName,
909+
},
910+
{
911+
returnsUnion: method.returns?.type,
912+
},
913+
);
908914
this.emitStabilityAnnotations(method);
909915
this.code.line(
910916
`${returnType} ${methodName}(${this.renderMethodParameters(method)});`,
@@ -927,11 +933,17 @@ class JavaGenerator extends Generator {
927933

928934
// for unions we only generate overloads for setters, not getters.
929935
this.code.line();
930-
this.addJavaDocs(prop, {
931-
api: 'member',
932-
fqn: ifc.fqn,
933-
memberName: prop.name,
934-
});
936+
this.addJavaDocs(
937+
prop,
938+
{
939+
api: 'member',
940+
fqn: ifc.fqn,
941+
memberName: prop.name,
942+
},
943+
{
944+
returnsUnion: prop.type,
945+
},
946+
);
935947
this.emitStabilityAnnotations(prop);
936948
if (prop.optional) {
937949
if (prop.overrides) {
@@ -952,6 +964,7 @@ class JavaGenerator extends Generator {
952964
api: 'member',
953965
fqn: ifc.fqn,
954966
memberName: prop.name,
967+
// Setter doesn't need a union type hint because we're generating overloads
955968
});
956969
if (prop.optional) {
957970
if (prop.overrides) {
@@ -1394,11 +1407,17 @@ class JavaGenerator extends Generator {
13941407
const access = this.renderAccessLevel(prop);
13951408

13961409
this.code.line();
1397-
this.addJavaDocs(prop, {
1398-
api: 'member',
1399-
fqn: parentType.fqn,
1400-
memberName: prop.name,
1401-
});
1410+
this.addJavaDocs(
1411+
prop,
1412+
{
1413+
api: 'member',
1414+
fqn: parentType.fqn,
1415+
memberName: prop.name,
1416+
},
1417+
{
1418+
returnsUnion: prop.type,
1419+
},
1420+
);
14021421
this.emitStabilityAnnotations(prop);
14031422
this.code.line(`${access} final static ${propType} ${propName};`);
14041423
}
@@ -1437,11 +1456,17 @@ class JavaGenerator extends Generator {
14371456
// for unions we only generate overloads for setters, not getters.
14381457
if (includeGetter) {
14391458
this.code.line();
1440-
this.addJavaDocs(prop, {
1441-
api: 'member',
1442-
fqn: definingType.fqn,
1443-
memberName: prop.name,
1444-
});
1459+
this.addJavaDocs(
1460+
prop,
1461+
{
1462+
api: 'member',
1463+
fqn: definingType.fqn,
1464+
memberName: prop.name,
1465+
},
1466+
{
1467+
returnsUnion: prop.type,
1468+
},
1469+
);
14451470
if (overrides && !prop.static) {
14461471
this.code.line('@Override');
14471472
}
@@ -1478,6 +1503,7 @@ class JavaGenerator extends Generator {
14781503
api: 'member',
14791504
fqn: cls.fqn,
14801505
memberName: prop.name,
1506+
// No union type hint for setters
14811507
});
14821508
if (overrides && !prop.static) {
14831509
this.code.line('@Override');
@@ -1591,11 +1617,17 @@ class JavaGenerator extends Generator {
15911617
method,
15921618
)})`;
15931619
this.code.line();
1594-
this.addJavaDocs(method, {
1595-
api: 'member',
1596-
fqn: cls.fqn,
1597-
memberName: method.name,
1598-
});
1620+
this.addJavaDocs(
1621+
method,
1622+
{
1623+
api: 'member',
1624+
fqn: cls.fqn,
1625+
memberName: method.name,
1626+
},
1627+
{
1628+
returnsUnion: method.returns?.type,
1629+
},
1630+
);
15991631
this.emitStabilityAnnotations(method);
16001632
if (overrides && !method.static) {
16011633
this.code.line('@Override');
@@ -2303,7 +2335,7 @@ class JavaGenerator extends Generator {
23032335
);
23042336
const summary = prop.docs?.summary ?? 'the value to be set';
23052337
this.code.line(
2306-
` * ${paramJavadoc(prop.fieldName, prop.nullable, summary)}`,
2338+
` * ${this.paramJavadoc(prop.fieldName, prop.nullable, summary)}`,
23072339
);
23082340
if (prop.docs?.remarks != null) {
23092341
const indent = ' '.repeat(7 + prop.fieldName.length);
@@ -2719,10 +2751,17 @@ class JavaGenerator extends Generator {
27192751
private addJavaDocs(
27202752
doc: spec.Documentable,
27212753
apiLoc: ApiLocation,
2722-
defaultText?: string,
2754+
unionHint?: {
2755+
returnsUnion?: spec.TypeReference;
2756+
},
27232757
) {
2758+
const returnsUnion =
2759+
unionHint?.returnsUnion && containsUnionType(unionHint.returnsUnion)
2760+
? this.renderTypeReference(unionHint.returnsUnion)
2761+
: undefined;
2762+
27242763
if (
2725-
!defaultText &&
2764+
!returnsUnion &&
27262765
Object.keys(doc.docs ?? {}).length === 0 &&
27272766
!((doc as spec.Method).parameters ?? []).some(
27282767
(p) => Object.keys(p.docs ?? {}).length !== 0,
@@ -2737,8 +2776,6 @@ class JavaGenerator extends Generator {
27372776

27382777
if (docs.summary) {
27392778
paras.push(stripNewLines(myMarkDownToJavaDoc(renderSummary(docs))));
2740-
} else if (defaultText) {
2741-
paras.push(myMarkDownToJavaDoc(defaultText));
27422779
}
27432780

27442781
if (docs.remarks) {
@@ -2749,6 +2786,10 @@ class JavaGenerator extends Generator {
27492786
);
27502787
}
27512788

2789+
if (returnsUnion) {
2790+
paras.push(`Returns union: ${returnsUnion}`);
2791+
}
2792+
27522793
if (docs.default) {
27532794
paras.push(`Default: ${docs.default}`); // NOTE: there is no annotation in JavaDoc for this
27542795
}
@@ -2794,7 +2835,9 @@ class JavaGenerator extends Generator {
27942835
if (method.parameters) {
27952836
for (const param of method.parameters) {
27962837
const summary = param.docs?.summary;
2797-
tagLines.push(paramJavadoc(param.name, param.optional, summary));
2838+
tagLines.push(
2839+
this.paramJavadoc(param.name, param.optional, summary, param.type),
2840+
);
27982841
}
27992842
}
28002843
}
@@ -2818,6 +2861,28 @@ class JavaGenerator extends Generator {
28182861
this.code.line(' */');
28192862
}
28202863

2864+
private paramJavadoc(
2865+
name: string,
2866+
optional?: boolean,
2867+
summary?: string,
2868+
unionTypeHint?: spec.TypeReference,
2869+
): string {
2870+
const parts = ['@param', name];
2871+
if (summary) {
2872+
parts.push(stripNewLines(myMarkDownToJavaDoc(endWithPeriod(summary))));
2873+
}
2874+
2875+
if (unionTypeHint && containsUnionType(unionTypeHint)) {
2876+
parts.push(`Takes union: ${this.renderTypeReference(unionTypeHint)}.`);
2877+
}
2878+
2879+
if (!optional) {
2880+
parts.push('This parameter is required.');
2881+
}
2882+
2883+
return parts.join(' ');
2884+
}
2885+
28212886
private getClassBase(cls: spec.ClassType) {
28222887
if (!cls.base) {
28232888
return 'software.amazon.jsii.JsiiObject';
@@ -2992,6 +3057,30 @@ class JavaGenerator extends Generator {
29923057
}
29933058
}
29943059

3060+
/**
3061+
* Render a type reference to something human readable for use in JavaDocs
3062+
*/
3063+
private renderTypeReference(x: spec.TypeReference): string {
3064+
if (spec.isPrimitiveTypeReference(x)) {
3065+
return `{@link ${this.toJavaPrimitive(x.primitive)}}`;
3066+
}
3067+
if (spec.isNamedTypeReference(x)) {
3068+
return `{@link ${this.toNativeFqn(x.fqn)}}`;
3069+
}
3070+
if (spec.isCollectionTypeReference(x)) {
3071+
switch (x.collection.kind) {
3072+
case spec.CollectionKind.Array:
3073+
return `List<${this.renderTypeReference(x.collection.elementtype)}>`;
3074+
case spec.CollectionKind.Map:
3075+
return `Map<String, ${this.renderTypeReference(x.collection.elementtype)}>`;
3076+
}
3077+
}
3078+
if (spec.isUnionTypeReference(x)) {
3079+
return `either ${x.union.types.map((x) => this.renderTypeReference(x)).join(' or ')}`;
3080+
}
3081+
throw new Error(`Unknown type reference: ${JSON.stringify(x)}`);
3082+
}
3083+
29953084
private renderMethodCallArguments(method: spec.Callable) {
29963085
if (!method.parameters || method.parameters.length === 0) {
29973086
return '';
@@ -3483,22 +3572,6 @@ function isNullable(optionalValue: spec.OptionalValue | undefined): boolean {
34833572
);
34843573
}
34853574

3486-
function paramJavadoc(
3487-
name: string,
3488-
optional?: boolean,
3489-
summary?: string,
3490-
): string {
3491-
const parts = ['@param', name];
3492-
if (summary) {
3493-
parts.push(stripNewLines(myMarkDownToJavaDoc(endWithPeriod(summary))));
3494-
}
3495-
if (!optional) {
3496-
parts.push('This parameter is required.');
3497-
}
3498-
3499-
return parts.join(' ');
3500-
}
3501-
35023575
function endWithPeriod(s: string): string {
35033576
s = s.trimRight();
35043577
if (!s.endsWith('.')) {

0 commit comments

Comments
 (0)