feat: Avoid adding empty trailing comments to oneof unions (#1140)

This PR tweaks the output from #1136 slightly to avoid adding the trailing `//` after the union member separator if there isn't a documentation comment for the following oneof field (or if comments are turned off).
stephenh · Nov 25, 2024 · 5359e8d · 5359e8d
1 parent 37d3a39
commit 5359e8d
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 36 deletions.
diff --git a/integration/oneof-unions-value/oneof.ts b/integration/oneof-unions-value/oneof.ts
@@ -26,28 +26,21 @@ export interface PleaseChoose {
      * nice as numbers, but what are you going to do...
      */
     { $case: "aString"; value: string }
-    | //
-    { $case: "aMessage"; value: PleaseChoose_Submessage }
+    | { $case: "aMessage"; value: PleaseChoose_Submessage }
     | //
     /**
      * We also added a bool option! This was added after the 'age'
      * field, so it has a higher number.
      */
     { $case: "aBool"; value: boolean }
-    | //
-    { $case: "bunchaBytes"; value: Uint8Array }
-    | //
-    { $case: "anEnum"; value: PleaseChoose_StateEnum }
+    | { $case: "bunchaBytes"; value: Uint8Array }
+    | { $case: "anEnum"; value: PleaseChoose_StateEnum }
     | undefined;
   age: number;
-  eitherOr?:
-    | //
-    { $case: "either"; value: string }
-    | //
-    { $case: "or"; value: string }
-    | //
-    { $case: "thirdOption"; value: string }
-    | undefined;
+  eitherOr?: { $case: "either"; value: string } | { $case: "or"; value: string } | {
+    $case: "thirdOption";
+    value: string;
+  } | undefined;
   signature: Uint8Array;
   value: any | undefined;
 }

diff --git a/integration/oneof-unions/oneof.ts b/integration/oneof-unions/oneof.ts
@@ -26,28 +26,21 @@ export interface PleaseChoose {
      * nice as numbers, but what are you going to do...
      */
     { $case: "aString"; aString: string }
-    | //
-    { $case: "aMessage"; aMessage: PleaseChoose_Submessage }
+    | { $case: "aMessage"; aMessage: PleaseChoose_Submessage }
     | //
     /**
      * We also added a bool option! This was added after the 'age'
      * field, so it has a higher number.
      */
     { $case: "aBool"; aBool: boolean }
-    | //
-    { $case: "bunchaBytes"; bunchaBytes: Uint8Array }
-    | //
-    { $case: "anEnum"; anEnum: PleaseChoose_StateEnum }
+    | { $case: "bunchaBytes"; bunchaBytes: Uint8Array }
+    | { $case: "anEnum"; anEnum: PleaseChoose_StateEnum }
     | undefined;
   age: number;
-  eitherOr?:
-    | //
-    { $case: "either"; either: string }
-    | //
-    { $case: "or"; or: string }
-    | //
-    { $case: "thirdOption"; thirdOption: string }
-    | undefined;
+  eitherOr?: { $case: "either"; either: string } | { $case: "or"; or: string } | {
+    $case: "thirdOption";
+    thirdOption: string;
+  } | undefined;
   signature: Uint8Array;
   value: any | undefined;
 }

diff --git a/integration/use-readonly-types/use-readonly-types.ts b/integration/use-readonly-types/use-readonly-types.ts
@@ -19,12 +19,10 @@ export interface Entity {
   readonly fieldMask: readonly string[] | undefined;
   readonly listValue: ReadonlyArray<any> | undefined;
   readonly structValue: { readonly [key: string]: any } | undefined;
-  readonly oneOfValue?:
-    | //
-    { readonly $case: "theStringValue"; readonly theStringValue: string }
-    | //
-    { readonly $case: "theIntValue"; readonly theIntValue: number }
-    | undefined;
+  readonly oneOfValue?: { readonly $case: "theStringValue"; readonly theStringValue: string } | {
+    readonly $case: "theIntValue";
+    readonly theIntValue: number;
+  } | undefined;
 }
 
 export interface SubEntity {

diff --git a/src/main.ts b/src/main.ts
@@ -1193,7 +1193,9 @@ function generateOneofProperty(
       maybeAddComment(options, fieldInfo, fieldComments);
 
       const combinedComments = fieldComments.join("\n");
-      return code`| // \n ${combinedComments} { ${mbReadonly}$case: '${fieldName}', ${mbReadonly}${valueName}: ${typeName} }`;
+      return code`|${
+        combinedComments ? " // " : ""
+      }\n ${combinedComments} { ${mbReadonly}$case: '${fieldName}', ${mbReadonly}${valueName}: ${typeName} }`;
     }),
   );
 
@@ -1531,7 +1533,7 @@ function generateDecode(ctx: Context, fullName: string, messageDesc: DescriptorP
 
     chunks.push(code`
       const buf = reader.skip(tag & 7);
-      
+
       ${unknownFieldsInitializerSnippet}
       const list = message._unknownFields${maybeNonNullAssertion}[tag];