fix: support json_name containing hyphen on all field types (#521)

stephenh · Feb 21, 2022 · 8d9e78e · 8d9e78e
1 parent 63d0e0d
commit 8d9e78e
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 11 deletions.
diff --git a/integration/simple-json-name/simple.bin b/integration/simple-json-name/simple.bin
diff --git a/integration/simple-json-name/simple.proto b/integration/simple-json-name/simple.proto
@@ -12,4 +12,5 @@ message Simple {
   string spaces = 4 [json_name = "name with spaces"];
   string dollarStart = 5 [json_name = "$dollar"];
   string dollarEnd = 6 [json_name = "dollar$"];
+  repeated string hyphenList = 7 [json_name = "hyphen-list"];
 }
diff --git a/integration/simple-json-name/simple.ts b/integration/simple-json-name/simple.ts
@@ -13,10 +13,20 @@ export interface Simple {
   spaces: string;
   dollarStart: string;
   dollarEnd: string;
+  hyphenList: string[];
 }
 
 function createBaseSimple(): Simple {
-  return { name: '', age: undefined, createdAt: undefined, hyphen: '', spaces: '', dollarStart: '', dollarEnd: '' };
+  return {
+    name: '',
+    age: undefined,
+    createdAt: undefined,
+    hyphen: '',
+    spaces: '',
+    dollarStart: '',
+    dollarEnd: '',
+    hyphenList: [],
+  };
 }
 
 export const Simple = {
@@ -42,6 +52,9 @@ export const Simple = {
     if (message.dollarEnd !== '') {
       writer.uint32(50).string(message.dollarEnd);
     }
+    for (const v of message.hyphenList) {
+      writer.uint32(58).string(v!);
+    }
     return writer;
   },
 
@@ -73,6 +86,9 @@ export const Simple = {
         case 6:
           message.dollarEnd = reader.string();
           break;
+        case 7:
+          message.hyphenList.push(reader.string());
+          break;
         default:
           reader.skipType(tag & 7);
           break;
@@ -90,6 +106,7 @@ export const Simple = {
       spaces: isSet(object['name with spaces']) ? String(object['name with spaces']) : '',
       dollarStart: isSet(object.$dollar) ? String(object.$dollar) : '',
       dollarEnd: isSet(object.dollar$) ? String(object.dollar$) : '',
+      hyphenList: Array.isArray(object?.['hyphen-list']) ? object['hyphen-list'].map((e: any) => String(e)) : [],
     };
   },
 
@@ -102,6 +119,11 @@ export const Simple = {
     message.spaces !== undefined && (obj['name with spaces'] = message.spaces);
     message.dollarStart !== undefined && (obj.$dollar = message.dollarStart);
     message.dollarEnd !== undefined && (obj.dollar$ = message.dollarEnd);
+    if (message.hyphenList) {
+      obj['hyphen-list'] = message.hyphenList.map((e) => e);
+    } else {
+      obj['hyphen-list'] = [];
+    }
     return obj;
   },
 
@@ -114,6 +136,7 @@ export const Simple = {
     message.spaces = object.spaces ?? '';
     message.dollarStart = object.dollarStart ?? '';
     message.dollarEnd = object.dollarEnd ?? '';
+    message.hyphenList = object.hyphenList?.map((e) => e) || [];
     return message;
   },
 };

diff --git a/src/main.ts b/src/main.ts
@@ -1117,6 +1117,7 @@ function generateFromJson(ctx: Context, fullName: string, messageDesc: Descripto
     const fieldName = maybeSnakeToCamel(field.name, options);
     const jsonName = getFieldJsonName(field, options);
     const jsonProperty = getPropertyAccessor('object', jsonName);
+    const jsonPropertyOptional = getPropertyAccessor('object', jsonName, true);
 
     // get code that extracts value from incoming object
     const readSnippet = (from: string): Code => {
@@ -1224,11 +1225,11 @@ function generateFromJson(ctx: Context, fullName: string, messageDesc: Descripto
       } else {
         const readValueSnippet = readSnippet('e');
         if (readValueSnippet.toString() === code`e`.toString()) {
-          chunks.push(code`${fieldName}: Array.isArray(object?.${jsonName}) ? [...${jsonProperty}] : [],`);
+          chunks.push(code`${fieldName}: Array.isArray(${jsonPropertyOptional}) ? [...${jsonProperty}] : [],`);
         } else {
           // Explicit `any` type required to make TS with noImplicitAny happy. `object` is also `any` here.
           chunks.push(code`
-            ${fieldName}: Array.isArray(object?.${jsonName}) ? ${jsonProperty}.map((e: any) => ${readValueSnippet}): [],
+            ${fieldName}: Array.isArray(${jsonPropertyOptional}) ? ${jsonProperty}.map((e: any) => ${readValueSnippet}): [],
           `);
         }
       }
@@ -1250,7 +1251,7 @@ function generateFromJson(ctx: Context, fullName: string, messageDesc: Descripto
         chunks.push(code`undefined,`);
       }
     } else if (isAnyValueType(field)) {
-      chunks.push(code`${fieldName}: ${ctx.utils.isSet}(object?.${jsonName})
+      chunks.push(code`${fieldName}: ${ctx.utils.isSet}(${jsonPropertyOptional})
         ? ${readSnippet(`${jsonProperty}`)}
         : undefined,
       `);
@@ -1365,20 +1366,20 @@ function generateToJson(ctx: Context, fullName: string, messageDesc: DescriptorP
     if (isMapType(ctx, messageDesc, field)) {
       // Maps might need their values transformed, i.e. bytes --> base64
       chunks.push(code`
-        obj.${jsonName} = {};
+        ${jsonProperty} = {};
         if (message.${fieldName}) {
           Object.entries(message.${fieldName}).forEach(([k, v]) => {
-            obj.${jsonName}[k] = ${readSnippet('v')};
+            ${jsonProperty}[k] = ${readSnippet('v')};
           });
         }
       `);
     } else if (isRepeated(field)) {
       // Arrays might need their elements transformed
       chunks.push(code`
         if (message.${fieldName}) {
-          obj.${jsonName} = message.${fieldName}.map(e => ${readSnippet('e')});
+          ${jsonProperty} = message.${fieldName}.map(e => ${readSnippet('e')});
         } else {
-          obj.${jsonName} = [];
+          ${jsonProperty} = [];
         }
       `);
     } else if (isWithinOneOfThatShouldBeUnion(options, field)) {

diff --git a/src/utils.ts b/src/utils.ts
@@ -189,12 +189,13 @@ export function getFieldJsonName(field: FieldDescriptorProto, options: Options):
  * For simplicity, we don't match the ECMA 5/6 rules for valid identifiers exactly, and return array syntax liberally.
  * @param objectName
  * @param propertyName
+ * @param optional
  */
-export function getPropertyAccessor(objectName: string, propertyName: string): string {
+export function getPropertyAccessor(objectName: string, propertyName: string, optional: boolean = false): string {
   let validIdentifier = /^[a-zA-Z_$][\w$]*$/;
   return validIdentifier.test(propertyName)
-    ? `${objectName}.${propertyName}`
-    : `${objectName}[${JSON.stringify(propertyName)}]`;
+    ? `${objectName}${optional ? '?' : ''}.${propertyName}`
+    : `${objectName}${optional ? '?.' : ''}[${JSON.stringify(propertyName)}]`;
 }
 
 export function impProto(options: Options, module: string, type: string): Import {