Skip to content

Commit

Permalink
Correctly escape Javadoc code snippets (#1397)
Browse files Browse the repository at this point in the history
Escaping code snippets for use in Javadoc comments is non-trivial. By
default, Javadoc comments are expected to be HTML markup. While HTML
offers `<pre>` and `<code>` tags, these only handle formatting and do
not remove the need to escape e.g. characters such as `<` and `>`. To
this end, Javadoc provides options such as `{@code ...}` for embedding
text verbatim. However, these only handle escaping and not formatting.
One thus generally combines the two approaches to yield comments like
`<pre>{@code ...}</pre>`. However, *this* combination itself introduces
challenges around escaping:

* `@`, which normally indicates a Javadoc tag (such as `@code` itself),
needs to be escaped in some situations but not others:

```java
/**
 * <pre>
 * {@code
 * public class C {
 *   // This use of @ needs to be escaped lest Javadoc get confused and think
 *   // it's a nested tag (even though @code should explicitly prevent this).
 *   @SuppressWarnings
 *   public static void m1() {}
 *
 *   // This @ does not need to be escaped.
 *   public static void m2() {}
 * }
 * }
 * </pre>
 */
```

* Braces (`{` and `}`) need to be escaped if they are not balanced,
since Javadoc counts braces in order to work out when to end the `@code`
block.
* The combination `*/` always needs to be escaped since it would
otherwise premutately terminate the Javadoc comment.

Our escaping code is currently broken (see #1363), causing crashes
because it fails to handle the case of unbalanced braces. Even in the
cases where it does not cause crashes, the Javadoc generated is not
necessarily faithful to the input, due to incorrect escaping of `@` and
`*/` depending on the context. For instance, while many sources state
that `{@literal @}` is the correct escaping of `@` within a `@code`
block, this is not in general true and will result in `{@literal @}`
being produced in the rendered output instead of the desired `@`.

This commit attempts to fix it once and for all. We do this by adopting
a strategy whereby we leave the `@code` block temporarily when the need
to escape a character arises. For example, given:

```java
This contains an @ and {unbalanced braces
```

we now generate:

```java
This contains an }{@literal @}{@code  and }{{@code unbalanced braces
```

with the expectation that this will end up inside a `<pre>{@code
...}</pre>` context. This results in generated comments that are harder
to read, but render successfully and accurately, both as HTML Javadoc
pages and e.g. hover documentation in most IDEs. We attempt to minimise
the impact this will have on real-world SDKs/programs by only escaping
if there is a need (that is, if a comment contains `@`, `*/`, or
unbalanced braces). The test suite for Javadoc processing has been
bulked out so that hopefully this does not bite again (famous last
words!).

Fixes #1363
  • Loading branch information
lunaris authored Aug 1, 2024
1 parent a9f86dd commit 210c886
Show file tree
Hide file tree
Showing 4 changed files with 453 additions and 95 deletions.
1 change: 1 addition & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@

### Bug Fixes

- Correctly escape special characters in generated Javadocs.
Original file line number Diff line number Diff line change
Expand Up @@ -702,20 +702,20 @@
* import java.nio.file.Files;
* import java.nio.file.Paths;
*
* public class App {
* public static void main(String[] args) {
* public class App }{{@code
* public static void main(String[] args) }{{@code
* Pulumi.run(App::stack);
* }
* }}{@code
*
* public static void stack(Context ctx) {
* public static void stack(Context ctx) }{{@code
* // This is an example of an Email connection.
* var passwordlessEmail = new Connection("passwordlessEmail", ConnectionArgs.builder()
* .strategy("email")
* .name("email")
* .options(ConnectionOptionsArgs.builder()
* .name("email")
* .from("{{ application.name }} \u003croot{@literal @}auth0.com\u003e")
* .subject("Welcome to {{ application.name }}")
* .from("}{{{@code application.name }}}{@code \u003croot}{@literal @}{@code auth0.com\u003e")
* .subject("Welcome to }{{{@code application.name }}}{@code ")
* .syntax("liquid")
* .template("\u003chtml\u003eThis is the body of the email\u003c/html\u003e")
* .disableSignup(false)
Expand All @@ -733,8 +733,8 @@
* .build())
* .build());
*
* }
* }
* }}{@code
* }}{@code
* }
* </pre>
* \u003c!--End PulumiCodeChooser --\u003e
Expand Down Expand Up @@ -763,12 +763,12 @@
* import java.nio.file.Files;
* import java.nio.file.Paths;
*
* public class App {
* public static void main(String[] args) {
* public class App }{{@code
* public static void main(String[] args) }{{@code
* Pulumi.run(App::stack);
* }
* }}{@code
*
* public static void stack(Context ctx) {
* public static void stack(Context ctx) }{{@code
* // This is an example of a SAML connection.
* var samlp = new Connection("samlp", ConnectionArgs.builder()
* .name("SAML-Connection")
Expand All @@ -786,11 +786,11 @@
* .protocolBinding("urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST")
* .requestTemplate("""
* \u003csamlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
* {@literal @@}AssertServiceURLAndDestination{@literal @@}
* ID="{@literal @@}ID{@literal @@}"
* IssueInstant="{@literal @@}IssueInstant{@literal @@}"
* ProtocolBinding="{@literal @@}ProtocolBinding{@literal @@}" Version="2.0"\u003e
* \u003csaml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"\u003e{@literal @@}Issuer{@literal @@}\u003c/saml:Issuer\u003e
* }{@literal @@}{@code AssertServiceURLAndDestination}{@literal @@}{@code
* ID="}{@literal @@}{@code ID}{@literal @@}{@code "
* IssueInstant="}{@literal @@}{@code IssueInstant}{@literal @@}{@code "
* ProtocolBinding="}{@literal @@}{@code ProtocolBinding}{@literal @@}{@code " Version="2.0"\u003e
* \u003csaml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"\u003e}{@literal @@}{@code Issuer}{@literal @@}{@code \u003c/saml:Issuer\u003e
* \u003c/samlp:AuthnRequest\u003e """)
* .userIdAttribute("https://saml.provider/imi/ns/identity-200810")
* .signatureAlgorithm("rsa-sha256")
Expand Down Expand Up @@ -822,21 +822,21 @@
* .signingKey(ConnectionOptionsSigningKeyArgs.builder()
* .key("""
* -----BEGIN PRIVATE KEY-----
* ...{your private key here}...
* ...}{{@code your private key here}}{@code ...
* -----END PRIVATE KEY----- """)
* .cert("""
* -----BEGIN CERTIFICATE-----
* ...{your public key cert here}...
* ...}{{@code your public key cert here}}{@code ...
* -----END CERTIFICATE----- """)
* .build())
* .decryptionKey(ConnectionOptionsDecryptionKeyArgs.builder()
* .key("""
* -----BEGIN PRIVATE KEY-----
* ...{your private key here}...
* ...}{{@code your private key here}}{@code ...
* -----END PRIVATE KEY----- """)
* .cert("""
* -----BEGIN CERTIFICATE-----
* ...{your public key cert here}...
* ...}{{@code your public key cert here}}{@code ...
* -----END CERTIFICATE----- """)
* .build())
* .idpInitiated(ConnectionOptionsIdpInitiatedArgs.builder()
Expand All @@ -847,8 +847,8 @@
* .build())
* .build());
*
* }
* }
* }}{@code
* }}{@code
* }
* </pre>
* \u003c!--End PulumiCodeChooser --\u003e
Expand Down
Loading

0 comments on commit 210c886

Please sign in to comment.