Sanitize generated comments

[stefanvanburen]

We're seeing issues with the codegen in which comments containing */ literals aren't being escaped. (An example can be found in google.longrunning.Operations.ListOperations).

KotlinPoet has an open issue for sanitizing comments, so for now, borrow their implementation in Wire.

[stefanvanburen]

(In draft because I'm still testing this 😅)

[timostamm]

The fix looks sensible to me. I'm not sure why whitespace is modified, and I believe @ should be escaped as well, because it has meaning in KDoc, but not in proto.

A test for this would be very nice to have. What about a proto file with a service that has offending comments attached? I think it should be generated automatically when put into protoc-gen-connect-kotlin/proto/buf, and produce a Kotlin client in protoc-gen-connect-kotlin/src/test/java. Then we just need a test case in protoc-gen-connect-kotlin/src/test/kotlin/PluginGenerationTest.kt to make sure that the generated code compiles.

[stefanvanburen]

The fix looks sensible to me. I'm not sure why whitespace is modified, and I believe @ should be escaped as well, because it has meaning in KDoc, but not in proto.

Sounds good - I'm less familiar with this, but looks to me like we should be changing literal @ -> @, in line with https://ascii.cl/htmlcodes.htm.

[stefanvanburen]

ok, added a simple test case in 09cd4bb - happy to extend it in any way that would make us more confident.

[stefanvanburen]

generated code looks like this FWIW, which is valid kotlin:

// Code generated by connect-kotlin. DO NOT EDIT.
//
// Source: buf/evilcomments/v1/evilcomments.proto
//
package buf.evilcomments.v1

import build.buf.connect.Headers
import build.buf.connect.MethodSpec
import build.buf.connect.ProtocolClientInterface
import build.buf.connect.ResponseMessage
import build.buf.connect.http.Cancelable
import kotlin.Unit

public class EvilCommentsServiceClient(
  private val client: ProtocolClientInterface,
) : EvilCommentsServiceClientInterface {
  /**
   *  This comment contains characters that should be escaped.
   *  @ is valid in KDoc, but not in proto.
   *  Comments in KDoc use C-style block comments, so */ and /* should be escaped.
   *  \[ and \] characters should also be escaped.
   */
  public override suspend fun evilComments(request: Evilcomments.EvilCommentsRequest,
      headers: Headers): ResponseMessage<Evilcomments.EvilCommentsResponse> = client.unary(
    request,
    headers,
    MethodSpec(
    "buf.evilcomments.v1.EvilCommentsService/EvilComments",
      buf.evilcomments.v1.Evilcomments.EvilCommentsRequest::class,
      buf.evilcomments.v1.Evilcomments.EvilCommentsResponse::class
    ),
  )


  /**
   *  This comment contains characters that should be escaped.
   *  &#64; is valid in KDoc, but not in proto.
   *  Comments in KDoc use C-style block comments, so &#42;/ and /&#42; should be escaped.
   *  \[ and \] characters should also be escaped.
   */
  public override fun evilComments(
    request: Evilcomments.EvilCommentsRequest,
    headers: Headers,
    onResult: (ResponseMessage<Evilcomments.EvilCommentsResponse>) -> Unit,
  ): Cancelable = client.unary(
    request,
    headers,
    MethodSpec(
    "buf.evilcomments.v1.EvilCommentsService/EvilComments",
      buf.evilcomments.v1.Evilcomments.EvilCommentsRequest::class,
      buf.evilcomments.v1.Evilcomments.EvilCommentsResponse::class
    ),
    onResult
  )

}

[pkwarren]

Are we sure about these two? When I look at the generated source in IDEA, this is how the doc is rendered:

[pkwarren]

I think we may want to do:

.replace("""[""", "[")
.replace("""]""", "]")

instead.

[timostamm]

Should fix the much more pressing issue with */ and /*, thank you for adding the tests!

Not sure about escaping [] though, see comment below.