feat: Support json names containing non-alphanumeric characters

integration/simple-json-name/simple.proto

@@ -5,7 +5,11 @@ import "google/protobuf/timestamp.proto";
 package simple;
 
 message Simple {
-  string name = 1 [ json_name = "other_name" ];
-  optional int32 age = 2 [ json_name = "other_age" ];
-  optional google.protobuf.Timestamp created_at = 9 [ json_name="createdAt" ];
+  string name = 1 [json_name = "other_name"];
+  optional int32 age = 2 [json_name = "other_age"];
+  optional google.protobuf.Timestamp created_at = 9 [json_name = "createdAt"];
+  string hyphen = 3 [json_name = "hyphened-name"];
+  string spaces = 4 [json_name = "name with spaces"];
+  string dollarStart = 5 [json_name = "$dollar"];
+  string dollarEnd = 6 [json_name = "dollar$"];
 }

integration/simple-json-name/simple.ts

@@ -9,10 +9,14 @@ export interface Simple {
   name: string;
   age?: number | undefined;
   createdAt?: Date | undefined;
+  hyphen: string;
+  spaces: string;
+  dollarStart: string;
+  dollarEnd: string;
 }
 
 function createBaseSimple(): Simple {
-  return { name: '', age: undefined, createdAt: undefined };
+  return { name: '', age: undefined, createdAt: undefined, hyphen: '', spaces: '', dollarStart: '', dollarEnd: '' };
 }
 
 export const Simple = {
@@ -26,6 +30,18 @@ export const Simple = {
     if (message.createdAt !== undefined) {
       Timestamp.encode(toTimestamp(message.createdAt), writer.uint32(74).fork()).ldelim();
     }
+    if (message.hyphen !== '') {
+      writer.uint32(26).string(message.hyphen);
+    }
+    if (message.spaces !== '') {
+      writer.uint32(34).string(message.spaces);
+    }
+    if (message.dollarStart !== '') {
+      writer.uint32(42).string(message.dollarStart);
+    }
+    if (message.dollarEnd !== '') {
+      writer.uint32(50).string(message.dollarEnd);
+    }
     return writer;
   },
 
@@ -45,6 +61,18 @@ export const Simple = {
         case 9:
           message.createdAt = fromTimestamp(Timestamp.decode(reader, reader.uint32()));
           break;
+        case 3:
+          message.hyphen = reader.string();
+          break;
+        case 4:
+          message.spaces = reader.string();
+          break;
+        case 5:
+          message.dollarStart = reader.string();
+          break;
+        case 6:
+          message.dollarEnd = reader.string();
+          break;
         default:
           reader.skipType(tag & 7);
           break;
@@ -58,6 +86,10 @@ export const Simple = {
       name: isSet(object.other_name) ? String(object.other_name) : '',
       age: isSet(object.other_age) ? Number(object.other_age) : undefined,
       createdAt: isSet(object.createdAt) ? fromJsonTimestamp(object.createdAt) : undefined,
+      hyphen: isSet(object['hyphened-name']) ? String(object['hyphened-name']) : '',
+      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$) : '',
     };
   },
 
@@ -66,6 +98,10 @@ export const Simple = {
     message.name !== undefined && (obj.other_name = message.name);
     message.age !== undefined && (obj.other_age = Math.round(message.age));
     message.createdAt !== undefined && (obj.createdAt = message.createdAt.toISOString());
+    message.hyphen !== undefined && (obj['hyphened-name'] = message.hyphen);
+    message.spaces !== undefined && (obj['name with spaces'] = message.spaces);
+    message.dollarStart !== undefined && (obj.$dollar = message.dollarStart);
+    message.dollarEnd !== undefined && (obj.dollar$ = message.dollarEnd);
     return obj;
   },
 
@@ -74,6 +110,10 @@ export const Simple = {
     message.name = object.name ?? '';
     message.age = object.age ?? undefined;
     message.createdAt = object.createdAt ?? undefined;
+    message.hyphen = object.hyphen ?? '';
+    message.spaces = object.spaces ?? '';
+    message.dollarStart = object.dollarStart ?? '';
+    message.dollarEnd = object.dollarEnd ?? '';
     return message;
   },
 };

integration/watch.ts

@@ -112,7 +112,7 @@ function yarnRun(yarn: string, task: string, header: string, taskArgument?: stri
 }
 
 function srcHandler(yarn: string, task: string, tests: string[]) {
-  return async (event: FsEvent, triggerPath: string) => {
+  return async (triggerPath: string) => {
     triggerPath = triggerPath.replace(/\\/g, '/');
     if (tests.length === 0) {
       const notice = `Plugin modified! Press [enter] to regenerate all integration tests or run 'yarn watch [TEST, ...]'. See 'yarn watch --help'.`;

src/main.ts

@@ -40,11 +40,12 @@ import {
 import SourceInfo, { Fields } from './sourceInfo';
 import {
   assertInstanceOf,
-  determineFieldJsonName,
+  getFieldJsonName,
   FormattedMethodDescriptor,
   impProto,
   maybeAddComment,
   maybePrefixPackage,
+  getPropertyAccessor,
 } from './utils';
 import { camelToSnake, capitalize, maybeSnakeToCamel } from './case';
 import {
@@ -1114,7 +1115,8 @@ function generateFromJson(ctx: Context, fullName: string, messageDesc: Descripto
   // add a check for each incoming field
   messageDesc.field.forEach((field) => {
     const fieldName = maybeSnakeToCamel(field.name, options);
-    const jsonName = determineFieldJsonName(field, options);
+    const jsonName = getFieldJsonName(field, options);
+    const jsonProperty = getPropertyAccessor('object', jsonName);
 
     // get code that extracts value from incoming object
     const readSnippet = (from: string): Code => {
@@ -1212,8 +1214,8 @@ function generateFromJson(ctx: Context, fullName: string, messageDesc: Descripto
         const fieldType = toTypeName(ctx, messageDesc, field);
         const i = maybeCastToNumber(ctx, messageDesc, field, 'key');
         chunks.push(code`
-          ${fieldName}: ${ctx.utils.isObject}(object.${jsonName})
-            ? Object.entries(object.${jsonName}).reduce<${fieldType}>((acc, [key, value]) => {
+          ${fieldName}: ${ctx.utils.isObject}(${jsonProperty})
+            ? Object.entries(${jsonProperty}).reduce<${fieldType}>((acc, [key, value]) => {
                 acc[${i}] = ${readSnippet('value')};
                 return acc;
               }, {})
@@ -1222,11 +1224,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}) ? [...object.${jsonName}] : [],`);
+          chunks.push(code`${fieldName}: Array.isArray(object?.${jsonName}) ? [...${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}) ? object.${jsonName}.map((e: any) => ${readValueSnippet}): [],
+            ${fieldName}: Array.isArray(object?.${jsonName}) ? ${jsonProperty}.map((e: any) => ${readValueSnippet}): [],
           `);
         }
       }
@@ -1240,35 +1242,35 @@ function generateFromJson(ctx: Context, fullName: string, messageDesc: Descripto
         chunks.push(code`${fieldName}: `);
       }
 
-      const ternaryIf = code`${ctx.utils.isSet}(object.${jsonName})`;
-      const ternaryThen = code`{ $case: '${fieldName}', ${fieldName}: ${readSnippet(`object.${jsonName}`)}`;
+      const ternaryIf = code`${ctx.utils.isSet}(${jsonProperty})`;
+      const ternaryThen = code`{ $case: '${fieldName}', ${fieldName}: ${readSnippet(`${jsonProperty}`)}`;
       chunks.push(code`${ternaryIf} ? ${ternaryThen}} : `);
 
       if (field === lastCase) {
         chunks.push(code`undefined,`);
       }
     } else if (isAnyValueType(field)) {
       chunks.push(code`${fieldName}: ${ctx.utils.isSet}(object?.${jsonName})
-        ? ${readSnippet(`object.${jsonName}`)}
+        ? ${readSnippet(`${jsonProperty}`)}
         : undefined,
       `);
     } else if (isStructType(field)) {
       chunks.push(
-        code`${fieldName}: ${ctx.utils.isObject}(object.${jsonName})
-          ? ${readSnippet(`object.${jsonName}`)}
+        code`${fieldName}: ${ctx.utils.isObject}(${jsonProperty})
+          ? ${readSnippet(`${jsonProperty}`)}
           : undefined,`
       );
     } else if (isListValueType(field)) {
       chunks.push(code`
-        ${fieldName}: Array.isArray(object.${jsonName})
-          ? ${readSnippet(`object.${jsonName}`)}
+        ${fieldName}: Array.isArray(${jsonProperty})
+          ? ${readSnippet(`${jsonProperty}`)}
           : undefined,
       `);
     } else {
       const fallback = isWithinOneOf(field) ? 'undefined' : defaultValue(ctx, field);
       chunks.push(code`
-        ${fieldName}: ${ctx.utils.isSet}(object.${jsonName})
-          ? ${readSnippet(`object.${jsonName}`)}
+        ${fieldName}: ${ctx.utils.isSet}(${jsonProperty})
+          ? ${readSnippet(`${jsonProperty}`)}
           : ${fallback},
       `);
     }
@@ -1292,7 +1294,8 @@ function generateToJson(ctx: Context, fullName: string, messageDesc: DescriptorP
   // then add a case for each field
   messageDesc.field.forEach((field) => {
     const fieldName = maybeSnakeToCamel(field.name, options);
-    const jsonName = determineFieldJsonName(field, options);
+    const jsonName = getFieldJsonName(field, options);
+    const jsonProperty = getPropertyAccessor('obj', jsonName);
 
     const readSnippet = (from: string): Code => {
       if (isEnum(field)) {
@@ -1382,10 +1385,10 @@ function generateToJson(ctx: Context, fullName: string, messageDesc: DescriptorP
       // oneofs in a union are only output as `oneof name = ...`
       const oneofName = maybeSnakeToCamel(messageDesc.oneofDecl[field.oneofIndex].name, options);
       const v = readSnippet(`message.${oneofName}?.${fieldName}`);
-      chunks.push(code`message.${oneofName}?.$case === '${fieldName}' && (obj.${jsonName} = ${v});`);
+      chunks.push(code`message.${oneofName}?.$case === '${fieldName}' && (${jsonProperty} = ${v});`);
     } else {
       const v = readSnippet(`message.${fieldName}`);
-      chunks.push(code`message.${fieldName} !== undefined && (obj.${jsonName} = ${v});`);
+      chunks.push(code`message.${fieldName} !== undefined && (${jsonProperty} = ${v});`);
     }
   });
   chunks.push(code`return obj;`);

src/utils.ts

@@ -174,7 +174,7 @@ export class FormattedMethodDescriptor implements MethodDescriptorProto {
   }
 }
 
-export function determineFieldJsonName(field: FieldDescriptorProto, options: Options): string {
+export function getFieldJsonName(field: FieldDescriptorProto, options: Options): string {
   // jsonName will be camelCased by the protocol compiler, plus can be overridden by the user,
   // so just use that instead of our own maybeSnakeToCamel
   if (options.snakeToCamel.includes('json')) {
@@ -184,6 +184,19 @@ export function determineFieldJsonName(field: FieldDescriptorProto, options: Opt
   }
 }
 
+/**
+ * Returns a snippet for reading an object's property, such as `foo.bar`, or `foo['bar']` if the property name contains unusual characters.
+ * For simplicity, we don't match the ECMA 5/6 rules for valid identifiers exactly, and return array syntax liberally.
+ * @param objectName
+ * @param propertyName
+ */
+export function getPropertyAccessor(objectName: string, propertyName: string): string {
+  let validIdentifier = /^[a-zA-Z_$][\w$]*$/;
+  return validIdentifier.test(propertyName)
+    ? `${objectName}.${propertyName}`
+    : `${objectName}[${JSON.stringify(propertyName)}]`;
+}
+
 export function impProto(options: Options, module: string, type: string): Import {
   return imp(`${type}@./${module}${options.fileSuffix}`);
 }