ProtoParser fidelity PR 1: split leading/trailing comments, fix blank-doc-line rendering

wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/EnumConstant.kt

@@ -28,7 +28,13 @@ data class EnumConstant(
   val isDeprecated: Boolean
     get() = "true" == options.get(DEPRECATED)
 
-  internal fun toElement() = EnumConstantElement(location, name, tag, documentation, options.elements)
+  internal fun toElement() = EnumConstantElement(
+    location = location,
+    name = name,
+    tag = tag,
+    documentation = documentation,
+    options = options.elements,
+  )
 
   internal fun linkOptions(linker: Linker, validate: Boolean) {
     @Suppress("NAME_SHADOWING")

wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/Reserved.kt

@@ -34,6 +34,12 @@ data class Reserved(
     fun fromElements(elements: List<ReservedElement>) = elements.map { Reserved(it.location, it.documentation, it.values) }
 
     @JvmStatic
-    fun toElements(reserveds: List<Reserved>) = reserveds.map { ReservedElement(it.location, it.documentation, it.values) }
+    fun toElements(reserveds: List<Reserved>) = reserveds.map {
+      ReservedElement(
+        location = it.location,
+        documentation = it.documentation,
+        values = it.values,
+      )
+    }
   }
 }

wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/Util.kt

@@ -39,9 +39,45 @@ fun StringBuilder.appendDocumentation(
     lines = lines.dropLast(1)
   }
   for (line in lines) {
-    append("// ")
-      .append(line)
-      .append('\n')
+    if (line.isEmpty()) {
+      append("//\n")
+    } else {
+      append("// ").append(line).append('\n')
+    }
+  }
+}
+
+/**
+ * Extracts the leading-only portion of a declaration's documentation, given
+ * the merged form produced by the parser (which stores leading and trailing
+ * joined by `\n`) and the trailing piece reported separately.
+ */
+// TODO internal and friend for wire-java-generator: https://youtrack.jetbrains.com/issue/KT-34102
+fun leadingDocumentation(
+  documentation: String,
+  trailingDocumentation: String,
+): String {
+  if (trailingDocumentation.isEmpty()) return documentation
+  if (documentation == trailingDocumentation) return ""
+  return documentation.removeSuffix("\n$trailingDocumentation")
+}
+
+/**
+ * Appends a same-line trailing comment. Emits `// text` for single-line content
+ * and `/* text */` for multi-line content. The caller must ensure
+ * [trailingDocumentation] does not contain `*` followed by `/`; parser-produced
+ * values are always safe, but manually-constructed elements must not violate
+ * this precondition.
+ */
+fun StringBuilder.appendTrailingDocumentation(
+  trailingDocumentation: String,
+) {
+  if (trailingDocumentation.isEmpty()) return
+  append(' ')
+  if (trailingDocumentation.contains('\n')) {
+    append("/* ").append(trailingDocumentation).append(" */")
+  } else {
+    append("// ").append(trailingDocumentation)
   }
 }
 

wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/EnumConstantElement.kt

@@ -18,23 +18,28 @@ package com.squareup.wire.schema.internal.parser
 import com.squareup.wire.schema.Location
 import com.squareup.wire.schema.internal.appendDocumentation
 import com.squareup.wire.schema.internal.appendOptions
+import com.squareup.wire.schema.internal.appendTrailingDocumentation
+import com.squareup.wire.schema.internal.leadingDocumentation
 
 data class EnumConstantElement(
   val location: Location,
   val name: String,
   val tag: Int,
   val documentation: String = "",
+  val trailingDocumentation: String = "",
   val options: List<OptionElement> = emptyList(),
 ) {
 
   fun toSchema() = buildString {
-    appendDocumentation(documentation)
+    appendDocumentation(leadingDocumentation(documentation, trailingDocumentation))
     append("$name = $tag")
 
     if (options.isNotEmpty()) {
       append(" ")
       appendOptions(options)
     }
-    append(";\n")
+    append(';')
+    appendTrailingDocumentation(trailingDocumentation)
+    append('\n')
   }
 }

wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/FieldElement.kt

@@ -20,6 +20,8 @@ import com.squareup.wire.schema.Location
 import com.squareup.wire.schema.ProtoType
 import com.squareup.wire.schema.internal.appendDocumentation
 import com.squareup.wire.schema.internal.appendOptions
+import com.squareup.wire.schema.internal.appendTrailingDocumentation
+import com.squareup.wire.schema.internal.leadingDocumentation
 import com.squareup.wire.schema.internal.toEnglishLowerCase
 
 data class FieldElement(
@@ -31,10 +33,11 @@ data class FieldElement(
   val jsonName: String? = null,
   val tag: Int = 0,
   val documentation: String = "",
+  val trailingDocumentation: String = "",
   val options: List<OptionElement> = emptyList(),
 ) {
   fun toSchema() = buildString {
-    appendDocumentation(documentation)
+    appendDocumentation(leadingDocumentation(documentation, trailingDocumentation))
 
     if (label != null) {
       append("${label.name.toEnglishLowerCase()} ")
@@ -47,7 +50,9 @@ data class FieldElement(
       appendOptions(optionsWithDefault)
     }
 
-    append(";\n")
+    append(';')
+    appendTrailingDocumentation(trailingDocumentation)
+    append('\n')
   }
 
   /**

wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/ProtoParser.kt

@@ -377,8 +377,7 @@ class ProtoParser internal constructor(
     val jsonName = stripJsonName(options)
     reader.require(';')
 
-    @Suppress("NAME_SHADOWING")
-    val documentation = reader.tryAppendTrailingDocumentation(documentation)
+    val withTrailing = reader.tryAppendTrailingDocumentation(documentation)
 
     return FieldElement(
       location = location,
@@ -388,7 +387,8 @@ class ProtoParser internal constructor(
       defaultValue = defaultValue,
       jsonName = jsonName,
       tag = tag,
-      documentation = documentation,
+      documentation = withTrailing.merged,
+      trailingDocumentation = withTrailing.trailing,
       options = options.toList(),
     )
   }
@@ -517,12 +517,12 @@ class ProtoParser internal constructor(
       "'reserved' must have at least one field name or tag"
     }
 
-    @Suppress("NAME_SHADOWING")
-    val documentation = reader.tryAppendTrailingDocumentation(documentation)
+    val withTrailing = reader.tryAppendTrailingDocumentation(documentation)
 
     return ReservedElement(
       location = location,
-      documentation = documentation,
+      documentation = withTrailing.merged,
+      trailingDocumentation = withTrailing.trailing,
       values = values,
     )
   }
@@ -580,14 +580,14 @@ class ProtoParser internal constructor(
     val options = OptionReader(reader).readOptions()
     reader.require(';')
 
-    @Suppress("NAME_SHADOWING")
-    val documentation = reader.tryAppendTrailingDocumentation(documentation)
+    val withTrailing = reader.tryAppendTrailingDocumentation(documentation)
 
     return EnumConstantElement(
       location = location,
       name = label,
       tag = tag,
-      documentation = documentation,
+      documentation = withTrailing.merged,
+      trailingDocumentation = withTrailing.trailing,
       options = options,
     )
   }

wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/ReservedElement.kt

@@ -18,15 +18,18 @@ package com.squareup.wire.schema.internal.parser
 import com.squareup.wire.schema.Location
 import com.squareup.wire.schema.internal.MAX_TAG_VALUE
 import com.squareup.wire.schema.internal.appendDocumentation
+import com.squareup.wire.schema.internal.appendTrailingDocumentation
+import com.squareup.wire.schema.internal.leadingDocumentation
 
 data class ReservedElement(
   val location: Location,
   val documentation: String = "",
+  val trailingDocumentation: String = "",
   /** A [String] name or [Int] or [IntRange] tag. */
   val values: List<Any>,
 ) {
   fun toSchema() = buildString {
-    appendDocumentation(documentation)
+    appendDocumentation(leadingDocumentation(documentation, trailingDocumentation))
     append("reserved ")
 
     values.forEachIndexed { index, value ->
@@ -46,6 +49,8 @@ data class ReservedElement(
         else -> throw AssertionError()
       }
     }
-    append(";\n")
+    append(';')
+    appendTrailingDocumentation(trailingDocumentation)
+    append('\n')
   }
 }

wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt

@@ -306,7 +306,7 @@ class SyntaxReader(
     }
   }
 
-  fun tryAppendTrailingDocumentation(documentation: String): String {
+  internal fun tryAppendTrailingDocumentation(documentation: String): DocumentationWithTrailing {
     // Search for a '/' character ignoring spaces and tabs.
     loop@ while (pos < data.size) {
       when (data[pos]) {
@@ -318,7 +318,7 @@ class SyntaxReader(
         }
 
         // Not a whitespace or comment-starting character. Return original documentation.
-        else -> return documentation
+        else -> return DocumentationWithTrailing(documentation, "")
       }
     }
 
@@ -381,11 +381,11 @@ class SyntaxReader(
       end--
     }
 
-    if (end == start) return documentation
+    if (end == start) return DocumentationWithTrailing(documentation, "")
 
     val trailingDocumentation = data.concatToString(start, end + 1)
-    if (documentation.isEmpty()) return trailingDocumentation
-    return "$documentation\n$trailingDocumentation"
+    if (documentation.isEmpty()) return DocumentationWithTrailing(trailingDocumentation, trailingDocumentation)
+    return DocumentationWithTrailing("$documentation\n$trailingDocumentation", trailingDocumentation)
   }
 
   /**
@@ -425,3 +425,10 @@ class SyntaxReader(
     location: Location? = location(),
   ): RuntimeException = throw IllegalStateException("Syntax error in $location: $message")
 }
+
+internal data class DocumentationWithTrailing(
+  /** The merged documentation (leading + trailing), matching the pre-refactor return value. */
+  val merged: String,
+  /** The trailing (same-line) comment only, or "" if no trailing comment was found. */
+  val trailing: String,
+)

wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/EnumElementTest.kt

@@ -208,4 +208,49 @@ class EnumElementTest {
     """.trimMargin()
     assertThat(value.toSchema()).isEqualTo(expected)
   }
+
+  @Test
+  fun enumConstantWithTrailingOnlyDocumentation() {
+    val constant = EnumConstantElement(
+      location = location,
+      name = "FOO",
+      tag = 1,
+      documentation = "inline doc",
+      trailingDocumentation = "inline doc",
+    )
+
+    assertThat(constant.toSchema()).isEqualTo("FOO = 1; // inline doc\n")
+  }
+
+  @Test
+  fun enumConstantWithLeadingAndTrailingDocumentation() {
+    val constant = EnumConstantElement(
+      location = location,
+      name = "FOO",
+      tag = 1,
+      documentation = "above\ninline",
+      trailingDocumentation = "inline",
+    )
+
+    assertThat(constant.toSchema()).isEqualTo(
+      """
+        |// above
+        |FOO = 1; // inline
+        |
+      """.trimMargin(),
+    )
+  }
+
+  @Test
+  fun enumConstantWithMultilineTrailingUsesBlockComment() {
+    val constant = EnumConstantElement(
+      location = location,
+      name = "FOO",
+      tag = 1,
+      documentation = "line one\nline two",
+      trailingDocumentation = "line one\nline two",
+    )
+
+    assertThat(constant.toSchema()).isEqualTo("FOO = 1; /* line one\nline two */\n")
+  }
 }