Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.11.0 generates invalid javadoc for Azure Native #1363

Closed
thomas11 opened this issue May 21, 2024 · 3 comments · Fixed by #1397
Closed

v0.11.0 generates invalid javadoc for Azure Native #1363

thomas11 opened this issue May 21, 2024 · 3 comments · Fixed by #1397
Assignees
Labels
area/codegen Code generation impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Milestone

Comments

@thomas11
Copy link
Contributor

The latest az-native release fails reproducibly for the Java publishing step. There are two errors:

/home/runner/work/pulumi-azure-native/pulumi-azure-native/sdk/java/src/main/java/com/pulumi/azurenative/media/StreamingPolicy.java:258: error: unterminated inline tag
 * {@code

/home/runner/work/pulumi-azure-native/pulumi-azure-native/sdk/java/src/main/java/com/pulumi/azurenative/media/StreamingPolicy.java:257: error: element not closed: pre
 * <pre>

This blocks Azure Native releases for Java.

Can be reproduced locally in a pulumi-azure-native checkout via:

$ make generate_java

$ cd sdk/java && gradle javadoc

If .pulumi-java-gen.version is v0.10.0, gradle succeeds; if it's v0.11.0, it fails.

@iwahbe iwahbe added the area/codegen Code generation label May 21, 2024
thomas11 added a commit to pulumi/pulumi-azure-native that referenced this issue May 22, 2024
Due to pulumi/pulumi-java#1363, malformed Javadoc prevents us from
publishing the provider SDK for Java. Pin to the working previous
version of pulumi-java.

Tested locally:
```
$ make generate_java

$ cd sdk/java && gradle javadoc
```

If `.pulumi-java-gen.version` is v0.10.0, gradle succeeds; if it's
v0.11.0, it fails.
@iwahbe iwahbe added the impact/regression Something that used to work, but is now broken label May 24, 2024
@iwahbe iwahbe removed their assignment May 24, 2024
@mikhailshilkov mikhailshilkov added the kind/bug Some behavior is incorrect or out of spec label May 28, 2024
thomas11 added a commit to pulumi/pulumi-azure-native that referenced this issue Jun 24, 2024
Doing it manually because the automated job also upgrades p-java which
we cannot do at the moment due to pulumi/pulumi-java#1363.
@mjeffryes mjeffryes added the p1 A bug severe enough to be the next item assigned to an engineer label Jul 17, 2024
thomas11 added a commit to pulumi/pulumi-azure-native that referenced this issue Jul 23, 2024
- **Upgrade pulumi to 3.126**
- **regenerate**

Done manually because the automated upgrade pulls in the latest p-java
which is broken for us (pulumi/pulumi-java#1363).
@justinvp justinvp added this to the 0.108 milestone Jul 23, 2024
@justinvp
Copy link
Member

Do we know the root cause of the regression?

@thomas11
Copy link
Contributor Author

Do we know the root cause of the regression?

Pretty sure it's #1356 but haven't dug deeper.

@lunaris
Copy link
Contributor

lunaris commented Jul 29, 2024

It is #1356, yes. In that we move to using {@code ...}. This provides a lot of flexibility we didn't have before, but it requires that braces inside it be balanced so that Javadoc can work out when to terminate the code block. The offending resource that triggers this bug has a snippet with an unclosed brace, which causes an error. I'm working on a fix presently.

lunaris added a commit that referenced this issue Jul 30, 2024
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. The test suite for Javadoc processing has been
bulked out so that hopefully this does not bite again (famous last words!).

Fixes #1363
lunaris added a commit that referenced this issue Jul 30, 2024
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. The test suite for Javadoc processing has been
bulked out so that hopefully this does not bite again (famous last words!).

Fixes #1363
lunaris added a commit that referenced this issue Jul 30, 2024
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. The test suite for Javadoc processing has been
bulked out so that hopefully this does not bite again (famous last words!).

Fixes #1363
lunaris added a commit that referenced this issue Jul 30, 2024
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. The test suite for Javadoc processing has been
bulked out so that hopefully this does not bite again (famous last words!).

Fixes #1363
lunaris added a commit that referenced this issue Jul 30, 2024
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. The test suite for Javadoc processing has been
bulked out so that hopefully this does not bite again (famous last words!).

Fixes #1363
lunaris added a commit that referenced this issue Jul 30, 2024
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. The test suite for Javadoc processing has been
bulked out so that hopefully this does not bite again (famous last words!).

Fixes #1363
lunaris added a commit that referenced this issue Jul 31, 2024
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
lunaris added a commit that referenced this issue Jul 31, 2024
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
@lunaris lunaris closed this as completed in 210c886 Aug 1, 2024
@pulumi-bot pulumi-bot added resolution/fixed This issue was fixed labels Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codegen Code generation impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants