Skip to content

Escape /* and */ in KDocs #6805

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.

Sign up for GitHub

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

Merged
BoD merged 1 commit into from
Dec 17, 2025
Merged

Escape /* and */ in KDocs #6805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions ...compiler/src/main/kotlin/com/apollographql/apollo/compiler/codegen/kotlin/helpers/KDoc.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.apollographql.apollo.compiler.codegen.kotlin.helpers

import com.apollographql.apollo.compiler.internal.applyIf
import com.apollographql.apollo.compiler.codegen.kotlin.KotlinResolver
import com.apollographql.apollo.compiler.codegen.kotlin.KotlinSymbols
import com.apollographql.apollo.compiler.internal.applyIf
import com.apollographql.apollo.compiler.ir.IrEnum
import com.squareup.kotlinpoet.Annotatable
import com.squareup.kotlinpoet.AnnotationSpec
Expand All @@ -18,33 +18,39 @@ internal fun TypeSpec.Builder.maybeAddDescription(description: String?): TypeSpe
return this
}

return addKdoc("%L", description.replace(' ', '♢'))
return addKdoc("%L", description.sanitized())
}

internal fun PropertySpec.Builder.maybeAddDescription(description: String?): PropertySpec.Builder {
if (description.isNullOrBlank()) {
return this
}

return addKdoc("%L", description.replace(' ', '♢'))
return addKdoc("%L", description.sanitized())
}

internal fun ParameterSpec.Builder.maybeAddDescription(description: String?): ParameterSpec.Builder {
if (description.isNullOrBlank()) {
return this
}

return addKdoc("%L", description.replace(' ', '♢'))
return addKdoc("%L", description.sanitized())
}

internal fun FunSpec.Builder.maybeAddDescription(description: String?): FunSpec.Builder {
if (description.isNullOrBlank()) {
return this
}

return addKdoc("%L", description.replace(' ', '♢'))
return addKdoc("%L", description.sanitized())
}

private fun String.sanitized(): String {
return replace(' ', '♢')
// See https://github.com/square/kotlinpoet/issues/887
.replace("/*", "/*")
.replace("*/", "*/")
Comment on lines +51 to +52
Copy link
Contributor

@martinbonnin martinbonnin Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means code blocks containing /* will probably not be rendered correctly, right? It's better than code not compiling I guess.

Copy link
Contributor Author

@BoD BoD Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No actually I checked, and that's rendered correctly in the IDE and in the generated KDoc

Copy link
Contributor

@martinbonnin martinbonnin Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this string:

```
*
```

Is rendered as this codeblock:

/*

Then how does one displays * in a codeblock?

Copy link
Contributor Author

@BoD BoD Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does one displays * in a codeblock?

You have to escape the &:

/**
 * /*
 *
 * *
 */

shows like:

Screenshot 2025-12-08 at 18 49 48

Copy link
Contributor

@martinbonnin martinbonnin Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this?

/**
 * generates some foo!
 * example:
 * 
 * ```kotlin
 * /* this is how you do it! */
 * val foo = doFoo()
 * ```
 */
fun doFoo() = TODO()

Copy link
Contributor Author

@BoD BoD Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh yes indeed it doesn't work inside code blocks :(

Screenshot 2025-12-08 at 18 58 03

Copy link
Contributor

@martinbonnin martinbonnin Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine by me, especially if the current state is to generate non-compiling code but probably means the "good" fix needs to be in Kotlin somewhere?

Copy link
Contributor Author

@BoD BoD Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

I've just opened https://youtrack.jetbrains.com/issue/KT-83005/

}

internal fun TypeSpec.Builder.maybeAddDeprecation(deprecationReason: String?): TypeSpec.Builder {
if (deprecationReason == null) {
Expand Down Expand Up @@ -120,7 +126,7 @@ internal fun requiresOptInAnnotation(annotation: ClassName): AnnotationSpec {
.build()
}

internal fun <T: Annotatable.Builder<*>> T.maybeAddOptIn(
internal fun <T : Annotatable.Builder<*>> T.maybeAddOptIn(
resolver: KotlinResolver,
enumValues: List<IrEnum.Value>,
): T = applyIf(enumValues.any { it.optInFeature != null }) {
Expand All @@ -132,10 +138,10 @@ internal fun <T: Annotatable.Builder<*>> T.maybeAddOptIn(
* Add suppressions for generated code.
* This is code the user has no control over and it should not generate warnings
*/
internal fun <T: Annotatable.Builder<*>> T.addSuppressions(
internal fun <T : Annotatable.Builder<*>> T.addSuppressions(
deprecation: Boolean = false,
optInUsage: Boolean = false,
unusedParameter: Boolean = false
unusedParameter: Boolean = false,
): T = apply {
if (!deprecation && !optInUsage && !unusedParameter) {
return@apply
Expand Down
5 changes: 5 additions & 0 deletions libraries/apollo-compiler/src/test/graphql/com/example/escaped_kdocs/TestOperation.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
query TestQuery {
someType {
someField
}
}
248 changes: 248 additions & 0 deletions ...aphql/com/example/escaped_kdocs/java/operationBased/escaped_kdocs/TestQuery.java.expected
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading