From 251c9240a174345954d96ff1fedd06ab333ccda1 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Tue, 30 Jul 2024 12:19:16 +0100 Subject: [PATCH] Correctly escape Javadoc code snippets Escaping code snippets for use in Javadoc comments is non-trivial. By default, Javadoc comments are expected to be HTML markup. While HTML offers `
` and
`` 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 `
{@code ...}
`. 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 /** *
 * {@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() {}
 * }
 * }
 * 
*/ ``` * 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 `
{@code ...}
` 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. The test suite for Javadoc processing has been bulked out so that hopefully this does not bite again (famous last words!). Fixes #1363 --- CHANGELOG_PENDING.md | 1 + .../repro_auth0_failure.golden | 250 +++++++++--------- pkg/codegen/java/utilities.go | 211 +++++++++++---- pkg/codegen/java/utilities_test.go | 170 ++++++++++-- 4 files changed, 435 insertions(+), 197 deletions(-) rename pkg/codegen/java/testdata/{TestFormatForignComments => TestFormatForeignComments}/repro_auth0_failure.golden (87%) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 58a79ae1412..1c5e3f2ebfa 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,3 +3,4 @@ ### Bug Fixes +- Correctly escape special characters in generated Javadocs. \ No newline at end of file diff --git a/pkg/codegen/java/testdata/TestFormatForignComments/repro_auth0_failure.golden b/pkg/codegen/java/testdata/TestFormatForeignComments/repro_auth0_failure.golden similarity index 87% rename from pkg/codegen/java/testdata/TestFormatForignComments/repro_auth0_failure.golden rename to pkg/codegen/java/testdata/TestFormatForeignComments/repro_auth0_failure.golden index c5f1bf6b2ab..6c7ab571f23 100644 --- a/pkg/codegen/java/testdata/TestFormatForignComments/repro_auth0_failure.golden +++ b/pkg/codegen/java/testdata/TestFormatForeignComments/repro_auth0_failure.golden @@ -32,12 +32,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 an Auth0 connection. * var myConnection = new Connection("myConnection", ConnectionArgs.builder() * .name("Example-Connection") @@ -55,9 +55,9 @@ * .requiresUsername(true) * .disableSignup(false) * .customScripts(Map.of("get_user", """ - * function getByEmail(email, callback) { + * function getByEmail(email, callback) }{{@code * return callback(new Error("Whoops!")); - * } + * }}{@code * """)) * .configuration(Map.ofEntries( * Map.entry("foo", "bar"), @@ -99,8 +99,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } *
* \u003c!--End PulumiCodeChooser --\u003e @@ -127,12 +127,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 Google OAuth2 connection. * var googleOauth2 = new Connection("googleOauth2", ConnectionArgs.builder() * .name("Google-OAuth2-Connection") @@ -155,8 +155,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -182,12 +182,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 * var googleApps = new Connection("googleApps", ConnectionArgs.builder() * .name("connection-google-apps") * .isDomainConnection(false) @@ -219,8 +219,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -245,12 +245,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 Facebook connection. * var facebook = new Connection("facebook", ConnectionArgs.builder() * .name("Facebook-Connection") @@ -270,8 +270,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -296,12 +296,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 an Apple connection. * var apple = new Connection("apple", ConnectionArgs.builder() * .name("Apple-Connection") @@ -324,8 +324,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -350,12 +350,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 an LinkedIn connection. * var linkedin = new Connection("linkedin", ConnectionArgs.builder() * .name("Linkedin-Connection") @@ -375,8 +375,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -401,12 +401,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 an GitHub connection. * var github = new Connection("github", ConnectionArgs.builder() * .name("GitHub-Connection") @@ -426,8 +426,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -452,12 +452,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 an SalesForce connection. * var salesforce = new Connection("salesforce", ConnectionArgs.builder() * .name("Salesforce-Connection") @@ -476,8 +476,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -504,12 +504,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 an OAuth2 connection. * var oauth2 = new Connection("oauth2", ConnectionArgs.builder() * .name("OAuth2-Connection") @@ -526,9 +526,9 @@ * .pkceEnabled(true) * .iconUrl("https://auth.example.com/assets/logo.png") * .scripts(Map.of("fetchUserProfile", """ - * function fetchUserProfile(accessToken, context, callback) { + * function fetchUserProfile(accessToken, context, callback) }{{@code * return callback(new Error("Whoops!")); - * } + * }}{@code * """)) * .setUserRootAttributes("on_each_login") * .nonPersistentAttrs( @@ -537,8 +537,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -564,12 +564,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 * var ad = new Connection("ad", ConnectionArgs.builder() * .name("connection-active-directory") * .displayName("Active Directory Connection") @@ -602,8 +602,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -629,12 +629,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 * var azureAd = new Connection("azureAd", ConnectionArgs.builder() * .name("connection-azure-ad") * .strategy("waad") @@ -673,8 +673,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -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) @@ -733,8 +733,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -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") @@ -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 @}{@literal @}{@code AssertServiceURLAndDestination}{@literal @}{@literal @}{@code + * ID="}{@literal @}{@literal @}{@code ID}{@literal @}{@literal @}{@code " + * IssueInstant="}{@literal @}{@literal @}{@code IssueInstant}{@literal @}{@literal @}{@code " + * ProtocolBinding="}{@literal @}{@literal @}{@code ProtocolBinding}{@literal @}{@literal @}{@code " Version="2.0"\u003e + * \u003csaml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"\u003e}{@literal @}{@literal @}{@code Issuer}{@literal @}{@literal @}{@code \u003c/saml:Issuer\u003e * \u003c/samlp:AuthnRequest\u003e """) * .userIdAttribute("https://saml.provider/imi/ns/identity-200810") * .signatureAlgorithm("rsa-sha256") @@ -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() @@ -847,8 +847,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -873,12 +873,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 WindowsLive connection. * var windowslive = new Connection("windowslive", ConnectionArgs.builder() * .name("Windowslive-Connection") @@ -897,8 +897,8 @@ * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -926,12 +926,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 an OIDC connection. * var oidc = new Connection("oidc", ConnectionArgs.builder() * .name("oidc-connection") @@ -966,20 +966,20 @@ * .userinfoScope("openid email profile groups") * .attributes(serializeJson( * jsonObject( - * jsonProperty("name", "${context.tokenset.name}"), - * jsonProperty("email", "${context.tokenset.email}"), - * jsonProperty("email_verified", "${context.tokenset.email_verified}"), - * jsonProperty("nickname", "${context.tokenset.nickname}"), - * jsonProperty("picture", "${context.tokenset.picture}"), - * jsonProperty("given_name", "${context.tokenset.given_name}"), - * jsonProperty("family_name", "${context.tokenset.family_name}") + * jsonProperty("name", "$}{{@code context.tokenset.name}}{@code "), + * jsonProperty("email", "$}{{@code context.tokenset.email}}{@code "), + * jsonProperty("email_verified", "$}{{@code context.tokenset.email_verified}}{@code "), + * jsonProperty("nickname", "$}{{@code context.tokenset.nickname}}{@code "), + * jsonProperty("picture", "$}{{@code context.tokenset.picture}}{@code "), + * jsonProperty("given_name", "$}{{@code context.tokenset.given_name}}{@code "), + * jsonProperty("family_name", "$}{{@code context.tokenset.family_name}}{@code ") * ))) * .build()) * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e @@ -1007,12 +1007,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 an Okta Workforce connection. * var okta = new Connection("okta", ConnectionArgs.builder() * .name("okta-connection") @@ -1050,20 +1050,20 @@ * .userinfoScope("openid email profile groups") * .attributes(serializeJson( * jsonObject( - * jsonProperty("name", "${context.tokenset.name}"), - * jsonProperty("email", "${context.tokenset.email}"), - * jsonProperty("email_verified", "${context.tokenset.email_verified}"), - * jsonProperty("nickname", "${context.tokenset.nickname}"), - * jsonProperty("picture", "${context.tokenset.picture}"), - * jsonProperty("given_name", "${context.tokenset.given_name}"), - * jsonProperty("family_name", "${context.tokenset.family_name}") + * jsonProperty("name", "$}{{@code context.tokenset.name}}{@code "), + * jsonProperty("email", "$}{{@code context.tokenset.email}}{@code "), + * jsonProperty("email_verified", "$}{{@code context.tokenset.email_verified}}{@code "), + * jsonProperty("nickname", "$}{{@code context.tokenset.nickname}}{@code "), + * jsonProperty("picture", "$}{{@code context.tokenset.picture}}{@code "), + * jsonProperty("given_name", "$}{{@code context.tokenset.given_name}}{@code "), + * jsonProperty("family_name", "$}{{@code context.tokenset.family_name}}{@code ") * ))) * .build()) * .build()) * .build()); * - * } - * } + * }}{@code + * }}{@code * } * * \u003c!--End PulumiCodeChooser --\u003e diff --git a/pkg/codegen/java/utilities.go b/pkg/codegen/java/utilities.go index 78dac2315a8..7a6e59cfc65 100644 --- a/pkg/codegen/java/utilities.go +++ b/pkg/codegen/java/utilities.go @@ -1,4 +1,16 @@ -// Copyright 2022, Pulumi Corporation. All rights reserved. +// Copyright 2022-2024, Pulumi Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. package java @@ -6,7 +18,6 @@ import ( "fmt" "html" "io" - "regexp" "strings" "github.com/pulumi/pulumi/pkg/v3/codegen" @@ -20,93 +31,180 @@ func formatForeignBlockComment(comment, indent string) string { return formatForeignBlockCommentFrom(comment, 0, indent) } -var replaceAtRegexp = regexp.MustCompile("( ?)(@+)") - // formatForeignBlockCommentFrom is like [formatForeignBlockComment], except that it // unconditionally accepts the first idx bytes of comment. func formatForeignBlockCommentFrom(comment string, idx int, indent string) string { comment = codegen.FilterExamples(comment, "java") + comment = comment[:idx] + mapCommentHelper(comment[idx:], escapeCode, escapeNonCode) + return formatBlockComment(comment, indent) +} - // Escape a '@' with a '{@literal @}'. - // - // This inserts a space before the new '@', so we attempt to remove a leading - // space when possible. It not possible, we do it anyway. - escapeAtLiteral := func(s string) string { - return replaceAtRegexp.ReplaceAllString(s, "{@literal $2}") +// Escapes a piece of Java code for use within a Javadoc comment. +// +// - Returned code is wrapped in a Javadoc `{@code ...}` block inside an HTML `
` tag. This provides the best
+//     out-of-the-box experience when it comes to not having to escape a large number of characters (e.g. HTML is fine
+//     as-is).
+//   - Characters that remain a concern are:
+//   - '@', which Javadoc might still try to interpret as a Javadoc tag opener;
+//   - '{' and '}', which _must_ be balanced inside a `@code` block (for this is how Javadoc works out when to
+//     close the block); and
+//   - the combination '*/', which will close the Javadoc comment block prematurely.
+//
+// In general, the only safe way to handle these characters is to exit the `@code` block temporarily, print them out (in
+// some cases still requiring escapes) and then re-enter the code block. This generally harms the readability of the
+// generated comment at the expense of it being renderable by Javadoc and the majority of IDEs that support e.g. docs on
+// hover.
+//
+// In cases where multiple escape-requiring characters appear in a sequence, we "stay outside the `@code` block" as long
+// as possible in order to avoid hampering readability even further.
+func escapeCode(code string) string {
+	var b strings.Builder
+	b.Grow(len(code))
+
+	b.WriteString("
\n{@code")
+
+	previousWasStar := false
+	outsideCode := false
+
+	for _, rune := range code {
+		if rune == '@' {
+			// If we are inside a code block, we need to exit it temporarily to print the '@' character (which still
+			// needs escaping as '{@literal @}').
+			if !outsideCode {
+				b.WriteRune('}')
+				outsideCode = true
+			}
+			b.WriteString("{@literal @}")
+		} else if rune == '{' || rune == '}' {
+			// If we are outside a code block, we can just print the brace as-is. If not, we need to exit the code block
+			// temporarily to print the brace.
+			if outsideCode {
+				b.WriteRune(rune)
+			} else {
+				b.WriteRune('}')
+				b.WriteRune(rune)
+				outsideCode = true
+			}
+		} else if rune == '/' && previousWasStar {
+			// If we are inside a code block, we need to exit it temporarily to print the '*/' sequence (which needs to
+			// be escaped as '*/').
+			if !outsideCode {
+				b.WriteRune('}')
+				outsideCode = true
+			}
+			b.WriteString("/")
+		} else {
+			// For all other characters, we need to re-enter the code block if we've left it.
+			if outsideCode {
+				b.WriteString("{@code")
+				if rune != '\n' {
+					b.WriteRune(' ')
+				}
+				outsideCode = false
+			}
+			b.WriteRune(rune)
+		}
+
+		previousWasStar = rune == '*'
 	}
 
-	comment = comment[:idx] + mapCommentHelper(comment[idx:],
-		// Code
-		func(s string) string {
-			s = strings.TrimPrefix(s, "```java")
-			s = strings.TrimSuffix(s, "```")
-
-			// Javadoc doesn't have a way to escape an arbitrarily code
-			// snippet. The best we have is:
-			//
-			// 
-			// {@code
-			// THE CODE GOES HERE:
-			// - '@' needs to be escaped.
-			// - '&', '<', '>' does not need to be escaped.
-			// }
-			// 
- var b strings.Builder - b.WriteString("
\n{@code")
-			b.WriteString(escapeAtLiteral(s))
-			b.WriteString("}\n
") - return b.String() - }, - // Non-code - func(s string) string { return html.EscapeString(escapeAtLiteral(s)) }, - ) + b.WriteString("}\n
") + return b.String() +} - return formatBlockComment(comment, indent) +// Escapes a piece of text for using within a Javadoc comment. +// +// * '@' characters are escaped as '{@literal @}'. +// * The pair '*/' is escaped as '*/' (the slash being HTML-encoded). +// * All other characters are HTML-encoded as-is. +func escapeNonCode(nonCode string) string { + var b strings.Builder + b.Grow(len(nonCode)) + + previousWasStar := false + insideLiteral := false + + for _, rune := range nonCode { + if rune == '@' { + if insideLiteral { + b.WriteRune('@') + } else { + b.WriteString("{@literal @") + insideLiteral = true + } + } else if rune == '/' && previousWasStar { + // insideLiteral can never be true here, since the previous character was a '*' which would have set + // insideLiteral to false. Thus we don't need to worry about printing the escape sequence literally. + b.WriteString("/") + } else { + if insideLiteral { + b.WriteRune('}') + insideLiteral = false + } + b.WriteString(html.EscapeString(string(rune))) + } + + previousWasStar = rune == '*' + } + + return b.String() } -// mapCommentHelper maps 2 functions over `comment`. It maps `code` over the section of -// `comment` that is made up of Java code (as defined by markdown code fences) and -// `nonCode` over all other parts of `comment`. -func mapCommentHelper(comment string, code, nonCode func(string) string) string { +// mapCommentHelper maps 2 functions over `comment`. It maps `codeF` over the section of `comment` that is made up of +// Java code (as defined by markdown code fences) and `nonCodeF` over all other parts of `comment`. +func mapCommentHelper(comment string, codeF, nonCodeF func(string) string) string { var dst strings.Builder dst.Grow(len(comment)) for { + // At any given point in this loop, we are looking for the next code block in the comment. We do this by hunting + // for the start marker and from there an end marker, using these to calculate the placement of the code within: + // + // ... ```java\npublic class C{}\n``` ... + // ^ ^ ^ ^ + // | | | | + // codeStartMarkerIndex | codeEndIndex + // | | + // codeStartIndex codeEndMarkerIndex + // + // The code inside (trimmed of the markers) is passed to `codeF` for processing. Once we are done, we move to + // the point after the code block (`codeEndIndex`) and go around for another iteration. Any non-code that + // appears before the block (when the `codeStartMarkerIndex` is not zero) or after the last block (outside the + // loop) is passed to `nonCodeF`. + const ( codeStartMarker = "```java" codeEndMarker = "```" ) - codeStart := strings.Index(comment, codeStartMarker) - if codeStart == -1 { + codeStartMarkerIndex := strings.Index(comment, codeStartMarker) + if codeStartMarkerIndex == -1 { break } - codeStartOffset := codeStart + len(codeStartMarker) + codeStartIndex := codeStartMarkerIndex + len(codeStartMarker) - codeEnd := strings.Index(comment[codeStartOffset:], codeEndMarker) - if codeEnd == -1 { + codeEndMarkerIndex := strings.Index(comment[codeStartIndex:], codeEndMarker) + if codeEndMarkerIndex == -1 { break } - codeEnd += codeStartOffset // Make codeEnd relative to comment - codeEndOffset := codeEnd + len(codeEndMarker) - // We have now found a code block: + codeEndMarkerIndex += codeStartIndex + codeEndIndex := codeEndMarkerIndex + len(codeEndMarker) + + code := comment[codeStartIndex:codeEndMarkerIndex] - // Write all non-copied text proceeding the code block - if codeStart != 0 { - dst.WriteString(nonCode(comment[:codeStart])) + if codeStartMarkerIndex != 0 { + dst.WriteString(nonCodeF(comment[:codeStartMarkerIndex])) } - // Then write the code block itself - dst.WriteString(code(comment[codeStart:codeEndOffset])) - // Then adjust copiedTo to start after the code block - comment = comment[codeEndOffset:] + dst.WriteString(codeF(code)) + comment = comment[codeEndIndex:] } // Copy any remaining non-code into the dst buffer. if comment != "" { - dst.WriteString(nonCode(comment)) + dst.WriteString(nonCodeF(comment)) } return dst.String() @@ -114,7 +212,6 @@ func mapCommentHelper(comment string, code, nonCode func(string) string) string func formatBlockComment(comment, indent string) string { prefix := fmt.Sprintf("%s * ", indent) - comment = strings.ReplaceAll(comment, "*/", "*{@literal /}") lines := strings.Split(comment, "\n") if nth := len(lines) - 1; nth >= 0 && lines[nth] != "" { diff --git a/pkg/codegen/java/utilities_test.go b/pkg/codegen/java/utilities_test.go index 83965c0eda0..09b1f1376cd 100644 --- a/pkg/codegen/java/utilities_test.go +++ b/pkg/codegen/java/utilities_test.go @@ -1,5 +1,18 @@ -// Copyright 2022, Pulumi Corporation. All rights reserved. +// Copyright 2022-2024, Pulumi Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//nolint:goconst package java import ( @@ -10,7 +23,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestFormatForignComments(t *testing.T) { +func TestFormatForeignComments(t *testing.T) { tests := []struct { name string input string @@ -52,18 +65,18 @@ func TestFormatForignComments(t *testing.T) { name: "@ escapes", input: `@foo ` + "```java" + ` -@ does not need to be escaped -& must be escaped -< and > must be escaped +@ needs to be escaped +& doesn't need to be escaped +< and > don't need to be escaped ` + "```" + ` @foo `, expected: autogold.Expect(` * {@literal @}foo *
  * {@code
- * {@literal @} does not need to be escaped
- * & must be escaped
- * < and > must be escaped
+ * }{@literal @}{@code  needs to be escaped
+ * & doesn't need to be escaped
+ * < and > don't need to be escaped
  * }
  * 
* {@literal @}foo @@ -80,14 +93,141 @@ func TestFormatForignComments(t *testing.T) { { name: "escape @ variations", input: ` --@foo- -- @foo- The space here should go away since it will *always* be inserted by the {@literal} escape -- @@@foo - +- @Foo @ bar +- Foo @ bar +- Foo bar @@ `, - //nolint:lll - expected: autogold.Expect(` * -{@literal @}foo- - * -{@literal @}foo- The space here should go away since it will *always* be inserted by the {{@literal @}literal} escape - * -{@literal @@@}foo - + expected: autogold.Expect(` * - {@literal @}Foo {@literal @} bar + * - Foo {@literal @} bar + * - Foo bar {@literal @@} + * `), + }, + { + name: "balanced braces text", + input: "This is some text with {balanced} braces", + expected: autogold.Expect(` * This is some text with {balanced} braces + * `), + }, + { + name: "unbalanced braces text", + input: "This is some text with {unbalanced braces", + expected: autogold.Expect(` * This is some text with {unbalanced braces + * `), + }, + { + name: "balanced braces code", + input: "```java" + ` +This is some code with {balanced} braces +` + "```", + expected: autogold.Expect(` *
+ * {@code
+ * This is some code with }{{@code balanced}}{@code  braces
+ * }
+ * 
+ * `), + }, + { + name: "unbalanced braces code", + input: "```java" + ` +This is some code with {unbalanced braces +` + "```", + expected: autogold.Expect(` *
+ * {@code
+ * This is some code with }{{@code unbalanced braces
+ * }
+ * 
+ * `), + }, + { + name: "balanced braces code with @", + input: "```java" + ` +This is some code with {balanced} braces and @ +` + "```", + expected: autogold.Expect(` *
+ * {@code
+ * This is some code with }{{@code balanced}}{@code  braces and }{@literal @}{@code
+ * }
+ * 
+ * `), + }, + { + name: "unbalanced braces code with @", + input: "```java" + ` +This is some code with {unbalanced braces and @ +` + "```", + expected: autogold.Expect(` *
+ * {@code
+ * This is some code with }{{@code unbalanced braces and }{@literal @}{@code
+ * }
+ * 
+ * `), + }, + { + name: "code with Javadoc-looking contents", + input: "```java" + ` +This is some code with {@code stuff} in it +` + "```", + expected: autogold.Expect(` *
+ * {@code
+ * This is some code with }{{@literal @}{@code code stuff}}{@code  in it
+ * }
+ * 
+ * `), + }, + { + name: "code with comment terminators", + input: "```java" + ` +This is some code with */ in it, as well as {@code stuff} +` + "```", + expected: autogold.Expect(` *
+ * {@code
+ * This is some code with *}/{@code  in it, as well as }{{@literal @}{@code code stuff}}{@code
+ * }
+ * 
+ * `), + }, + { + name: "messy code and text", + input: ` +Foo {} @@bar */ + +This is */ @*/ + +Should be escaped + +` + "```java" + ` +This is some @code with */*/ in it, as well as {{@code stuff} + +xx @SuppressWarnings +public static void main(String[] args) {} + +public static bar(String s) { + baz(s, s, s); +} + +x @ foo +`, + // nolint:lll + expected: autogold.Expect(` * Foo {} {@literal @@}bar */ + * + * This is */ {@literal @}*/ + * + * <html>Should be escaped</html> + * + *
+ * {@code
+ * This is some }{@literal @}{@code code with *}/{@code *}/{@code  in it, as well as }{{{@literal @}{@code code stuff}}{@code
+ *
+ * xx }{@literal @}{@code SuppressWarnings
+ * public static void main(String[] args) }{}{@code
+ *
+ * public static bar(String s) }{{@code
+ *   baz(s, s, s);
+ * }}{@code
+ *
+ * x }{@literal @}{@code  foo
+ * }
+ * 
* `), }, }