Skip to content

Commit

Permalink
fix(java): missing remarks in builder documentation (#1111)
Browse files Browse the repository at this point in the history
The Javadoc entries for setting methods on the generated `Builder`
classes did not represent the `remarks` section of the `docs` object,
resulting in only partial documentation being available on the builders.

This makes sure the `remarks` section gets rendered correctly in all the
locations that previously ignored it, and the fix is demonstrated good
by virtue of the changes it induced in the regression test improvements.

Fixes #1094
  • Loading branch information
RomainMuller authored and mergify[bot] committed Dec 10, 2019
1 parent c19a340 commit 33e820c
Show file tree
Hide file tree
Showing 45 changed files with 242 additions and 130 deletions.
17 changes: 15 additions & 2 deletions packages/jsii-calc/lib/calculator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,21 @@ export class Power extends composition.CompositeOperation {
* Properties for Calculator.
*/
export interface CalculatorProps {
readonly initialValue?: number
readonly maximumValue?: number
/**
* The initial value of the calculator.
*
* NOTE: Any number works here, it's fine.
*
* @default 0
*/
readonly initialValue?: number;

/**
* The maximum value the calculator can store.
*
* @default none
*/
readonly maximumValue?: number;
}

/**
Expand Down
39 changes: 22 additions & 17 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,7 @@
"kind": "class",
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 260
"line": 273
},
"methods": [
{
Expand All @@ -1432,7 +1432,7 @@
},
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 299
"line": 312
},
"name": "add",
"parameters": [
Expand All @@ -1451,7 +1451,7 @@
},
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 306
"line": 319
},
"name": "mul",
"parameters": [
Expand All @@ -1470,7 +1470,7 @@
},
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 320
"line": 333
},
"name": "neg"
},
Expand All @@ -1481,7 +1481,7 @@
},
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 313
"line": 326
},
"name": "pow",
"parameters": [
Expand All @@ -1500,7 +1500,7 @@
},
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 339
"line": 352
},
"name": "readUnionValue",
"returns": {
Expand All @@ -1520,7 +1520,7 @@
"immutable": true,
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 327
"line": 340
},
"name": "expression",
"overrides": "jsii-calc.composition.CompositeOperation",
Expand All @@ -1536,7 +1536,7 @@
"immutable": true,
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 289
"line": 302
},
"name": "operationsLog",
"type": {
Expand All @@ -1556,7 +1556,7 @@
"immutable": true,
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 284
"line": 297
},
"name": "operationsMap",
"type": {
Expand All @@ -1580,7 +1580,7 @@
},
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 279
"line": 292
},
"name": "curr",
"type": {
Expand All @@ -1594,7 +1594,7 @@
},
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 294
"line": 307
},
"name": "maxValue",
"optional": true,
Expand All @@ -1609,7 +1609,7 @@
},
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 334
"line": 347
},
"name": "unionProperty",
"optional": true,
Expand Down Expand Up @@ -1649,12 +1649,15 @@
{
"abstract": true,
"docs": {
"stability": "experimental"
"default": "0",
"remarks": "NOTE: Any number works here, it's fine.",
"stability": "experimental",
"summary": "The initial value of the calculator."
},
"immutable": true,
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 235
"line": 242
},
"name": "initialValue",
"optional": true,
Expand All @@ -1665,12 +1668,14 @@
{
"abstract": true,
"docs": {
"stability": "experimental"
"default": "none",
"stability": "experimental",
"summary": "The maximum value the calculator can store."
},
"immutable": true,
"locationInModule": {
"filename": "lib/calculator.ts",
"line": 236
"line": 249
},
"name": "maximumValue",
"optional": true,
Expand Down Expand Up @@ -11743,5 +11748,5 @@
}
},
"version": "0.20.8",
"fingerprint": "w4S74J2aEQiII5p1/vLG5rXFrtvbxUP1mra7ZGBd4so="
"fingerprint": "sZ6Jp7HsZKHGh8JYB08PYxyFX0bYBgK1FMDhzjtPjVQ="
}
20 changes: 17 additions & 3 deletions packages/jsii-pacmak/lib/targets/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,7 @@ class JavaGenerator extends Generator {
const setter: spec.Method = {
name: fieldName,
docs: {
...prop.spec.docs,
stability: prop.spec.docs?.stability,
returns: '{@code this}',
},
Expand Down Expand Up @@ -1207,13 +1208,20 @@ class JavaGenerator extends Generator {
this.code.closeBlock();
}

private emitBuilderSetter(prop: JavaProp, builderName: string) {
private emitBuilderSetter(prop: JavaProp, builderName: string, builtType: string) {
for (const type of prop.javaTypes) {
this.code.line();
this.code.line('/**');
this.code.line(` * Sets the value of ${prop.propName}`);
this.code.line(` * Sets the value of {@link ${builtType}#${getterFor(prop.fieldName)}}`);
const summary = prop.docs?.summary ?? 'the value to be set';
this.code.line(` * ${paramJavadoc(prop.fieldName, prop.nullable, summary)}`);
if (prop.docs?.remarks != null) {
const indent = ' '.repeat(7 + prop.fieldName.length);
const remarks = md2html(prefixMarkdownTsCodeBlocks(prop.docs.remarks, SAMPLES_DISCLAIMER)).trimRight();
for (const line of remarks.split('\n')) {
this.code.line(` * ${indent} ${line}`);
}
}
this.code.line(' * @return {@code this}');
if (prop.docs?.deprecated) {
this.code.line(` * @deprecated ${prop.docs.deprecated}`);
Expand All @@ -1225,6 +1233,11 @@ class JavaGenerator extends Generator {
this.code.line('return this;');
this.code.closeBlock();
}

function getterFor(fieldName: string): string {
const [first, ...rest] = fieldName;
return `get${first.toUpperCase()}${rest.join('')}`;
}
}

private emitInterfaceBuilder(classSpec: spec.InterfaceType, constructorName: string, props: JavaProp[]) {
Expand All @@ -1247,7 +1260,7 @@ class JavaGenerator extends Generator {
this.code.openBlock(`public static final class ${BUILDER_CLASS_NAME}`);

props.forEach(prop => this.code.line(`private ${prop.fieldJavaType} ${prop.fieldName};`));
props.forEach(prop => this.emitBuilderSetter(prop, BUILDER_CLASS_NAME));
props.forEach(prop => this.emitBuilderSetter(prop, BUILDER_CLASS_NAME, classSpec.name));

// Start build()
this.code.line();
Expand Down Expand Up @@ -1879,6 +1892,7 @@ function paramJavadoc(name: string, optional?: boolean, summary?: string): strin
}

function endWithPeriod(s: string): string {
s = s.trimRight();
if (!s.endsWith('.')) {
return `${s}.`;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public static final class Builder {
private software.amazon.jsii.tests.calculator.baseofbase.Very foo;

/**
* Sets the value of Foo
* Sets the value of {@link VeryBaseProps#getFoo}
* @param foo the value to be set. This parameter is required.
* @return {@code this}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static final class Builder {
private software.amazon.jsii.tests.calculator.baseofbase.Very foo;

/**
* Sets the value of Bar
* Sets the value of {@link BaseProps#getBar}
* @param bar the value to be set. This parameter is required.
* @return {@code this}
*/
Expand All @@ -31,7 +31,7 @@ public Builder bar(java.lang.String bar) {
}

/**
* Sets the value of Foo
* Sets the value of {@link BaseProps#getFoo}
* @param foo the value to be set. This parameter is required.
* @return {@code this}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static final class Builder {
private java.util.List<java.lang.String> firstOptional;

/**
* Sets the value of Anumber
* Sets the value of {@link MyFirstStruct#getAnumber}
* @param anumber An awesome number value. This parameter is required.
* @return {@code this}
*/
Expand All @@ -63,7 +63,7 @@ public Builder anumber(java.lang.Number anumber) {
}

/**
* Sets the value of Astring
* Sets the value of {@link MyFirstStruct#getAstring}
* @param astring A string value. This parameter is required.
* @return {@code this}
*/
Expand All @@ -75,7 +75,7 @@ public Builder astring(java.lang.String astring) {
}

/**
* Sets the value of FirstOptional
* Sets the value of {@link MyFirstStruct#getFirstOptional}
* @param firstOptional the value to be set.
* @return {@code this}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public static final class Builder {
private java.lang.Boolean optional3;

/**
* Sets the value of Optional1
* Sets the value of {@link StructWithOnlyOptionals#getOptional1}
* @param optional1 The first optional!.
* @return {@code this}
*/
Expand All @@ -66,7 +66,7 @@ public Builder optional1(java.lang.String optional1) {
}

/**
* Sets the value of Optional2
* Sets the value of {@link StructWithOnlyOptionals#getOptional2}
* @param optional2 the value to be set.
* @return {@code this}
*/
Expand All @@ -78,7 +78,7 @@ public Builder optional2(java.lang.Number optional2) {
}

/**
* Sets the value of Optional3
* Sets the value of {@link StructWithOnlyOptionals#getOptional3}
* @param optional3 the value to be set.
* @return {@code this}
*/
Expand Down
Loading

0 comments on commit 33e820c

Please sign in to comment.