Skip to content

Commit

Permalink
fix(jsii-pacmak): escape documentation in all positions (#4096)
Browse files Browse the repository at this point in the history 
The [AWS::Personalize::Solution.PerformAutoML](
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-personalize-solution.html#cfn-personalize-solution-performautoml) property immediately starts with an admonition, which in our system gets translated to:

```
/**
 * > Admonition!
 *
 * Normal description of the property
 */
```

We weren't expecting complex markup in the 'summary' section of the JavaDocs, so we weren't escaping it, and this was making javadoc sad:

```
/software/amazon/awscdk/services/personalize/CfnSolution.java:2211: error: bad use of '>'
  * > We don't recommend enabling automated machine learning.
```

Be sure to properly do MarkDown -> HTML conversion everywhere.



---

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
  • Loading branch information
@rix0rrr
rix0rrr authored May 17, 2023
1 parent fa921ec commit 6a2248d
Show file tree
Hide file tree
Showing 10 changed files with 853 additions and 169 deletions.
27 changes: 27 additions & 0 deletions packages/jsii-calc/lib/documented.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,30 @@ export class Old {
// Nothing to do
}
}

/**
* > Don't use this interface
* An interface that shouldn't be used, with the annotation in a weird place.
*/
export interface DontUseMe {
/**
* > Don't set this parameter
*
* A parameter that shouldn't be set, with the annotation in a weird place.
*/
readonly dontSetMe?: string;
}

/**
* > Don't use this class.
*
* A class that shouldn't be used, with the annotation in a weird place.
*/
export class WeirdDocs {
/**
* > Don't read this property
*
* A property that shouldn't be read, with the annotation in a weird place.
*/
public dontReadMe?: string;
}
77 changes: 76 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5132,6 +5132,42 @@
"name": "DontComplainAboutVariadicAfterOptional",
"symbolId": "lib/compliance:DontComplainAboutVariadicAfterOptional"
},
"jsii-calc.DontUseMe": {
"assembly": "jsii-calc",
"datatype": true,
"docs": {
"stability": "stable",
"summary": "> Don't use this interface An interface that shouldn't be used, with the annotation in a weird place."
},
"fqn": "jsii-calc.DontUseMe",
"kind": "interface",
"locationInModule": {
"filename": "lib/documented.ts",
"line": 74
},
"name": "DontUseMe",
"properties": [
{
"abstract": true,
"docs": {
"remarks": "A parameter that shouldn't be set, with the annotation in a weird place.",
"stability": "stable",
"summary": "> Don't set this parameter."
},
"immutable": true,
"locationInModule": {
"filename": "lib/documented.ts",
"line": 80
},
"name": "dontSetMe",
"optional": true,
"type": {
"primitive": "string"
}
}
],
"symbolId": "lib/documented:DontUseMe"
},
"jsii-calc.DoubleTrouble": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -16010,6 +16046,45 @@
],
"symbolId": "lib/compliance:VoidCallback"
},
"jsii-calc.WeirdDocs": {
"assembly": "jsii-calc",
"docs": {
"remarks": "A class that shouldn't be used, with the annotation in a weird place.",
"stability": "stable",
"summary": "> Don't use this class."
},
"fqn": "jsii-calc.WeirdDocs",
"initializer": {
"docs": {
"stability": "stable"
}
},
"kind": "class",
"locationInModule": {
"filename": "lib/documented.ts",
"line": 88
},
"name": "WeirdDocs",
"properties": [
{
"docs": {
"remarks": "A property that shouldn't be read, with the annotation in a weird place.",
"stability": "stable",
"summary": "> Don't read this property."
},
"locationInModule": {
"filename": "lib/documented.ts",
"line": 94
},
"name": "dontReadMe",
"optional": true,
"type": {
"primitive": "string"
}
}
],
"symbolId": "lib/documented:WeirdDocs"
},
"jsii-calc.WithPrivatePropertyInConstructor": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -18909,5 +18984,5 @@
}
},
"version": "3.20.120",
"fingerprint": "kOCIHox3N0mzJsbC3zUF0dELGRsdq5jP57dDIOu3fDE="
"fingerprint": "KCZMc5FKKLI0JLhaAjlqS227RmusMVNRWt2UbgA5iOM="
}
36 changes: 26 additions & 10 deletions packages/jsii-pacmak/lib/targets/java.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,7 @@ class JavaGenerator extends Generator {
this.code.openFile(packageInfoFile);
this.code.line('/**');
if (mod.readme) {
for (const line of markDownToJavaDoc(
for (const line of myMarkDownToJavaDoc(
this.convertSamplesInMarkdown(mod.readme.markdown, {
api: 'moduleReadme',
moduleFqn: mod.name,
Expand Down Expand Up @@ -1069,7 +1069,7 @@ class JavaGenerator extends Generator {
this.code.openFile(packageInfoFile);
this.code.line('/**');
if (mod.readme) {
for (const line of markDownToJavaDoc(
for (const line of myMarkDownToJavaDoc(
this.convertSamplesInMarkdown(mod.readme.markdown, {
api: 'moduleReadme',
moduleFqn,
Expand Down Expand Up @@ -2294,7 +2294,7 @@ class JavaGenerator extends Generator {
);
if (prop.docs?.remarks != null) {
const indent = ' '.repeat(7 + prop.fieldName.length);
const remarks = markDownToJavaDoc(
const remarks = myMarkDownToJavaDoc(
this.convertSamplesInMarkdown(prop.docs.remarks, {
api: 'member',
fqn: prop.definingType.fqn,
Expand Down Expand Up @@ -2723,14 +2723,14 @@ class JavaGenerator extends Generator {
const paras = [];

if (docs.summary) {
paras.push(renderSummary(docs));
paras.push(stripNewLines(myMarkDownToJavaDoc(renderSummary(docs))));
} else if (defaultText) {
paras.push(defaultText);
paras.push(myMarkDownToJavaDoc(defaultText));
}

if (docs.remarks) {
paras.push(
markDownToJavaDoc(
myMarkDownToJavaDoc(
this.convertSamplesInMarkdown(docs.remarks, apiLoc),
).trimRight(),
);
Expand All @@ -2757,22 +2757,22 @@ class JavaGenerator extends Generator {
// Hence, we just resort to HTML-encoding everything (same as we do for code
// examples that have been translated from MarkDown).
paras.push(
markDownToJavaDoc(['```', convertedExample, '```'].join('\n')),
myMarkDownToJavaDoc(['```', convertedExample, '```'].join('\n')),
);
}

const tagLines = [];

if (docs.returns) {
tagLines.push(`@return ${docs.returns}`);
tagLines.push(`@return ${myMarkDownToJavaDoc(docs.returns)}`);
}
if (docs.see) {
tagLines.push(
`@see <a href="${escape(docs.see)}">${escape(docs.see)}</a>`,
);
}
if (docs.deprecated) {
tagLines.push(`@deprecated ${docs.deprecated}`);
tagLines.push(`@deprecated ${myMarkDownToJavaDoc(docs.deprecated)}`);
}

// Params
Expand Down Expand Up @@ -3475,7 +3475,7 @@ function paramJavadoc(
): string {
const parts = ['@param', name];
if (summary) {
parts.push(endWithPeriod(summary));
parts.push(stripNewLines(myMarkDownToJavaDoc(endWithPeriod(summary))));
}
if (!optional) {
parts.push('This parameter is required.');
Expand Down Expand Up @@ -3653,3 +3653,19 @@ function containsUnionType(
containsUnionType(typeRef.collection.elementtype))
);
}

function myMarkDownToJavaDoc(source: string) {
if (source.includes('{@link') || source.includes('{@code')) {
// Slightly dirty hack: if we are seeing this, it means the docstring was provided literally
// in this file. These strings are safe to not be escaped, and in fact escaping them will
// break them: they will turn into `{&#64;`, which breaks the JavaDoc markup.
//
// Since docstring do not (or at least should not) contain JavaDoc literals, this is safe.
return source;
}
return markDownToJavaDoc(source);
}

function stripNewLines(x: string) {
return x.replace(/\n/g, '');
}
Loading

0 comments on commit 6a2248d

Please sign in to comment.