From ce44668b8fe01b14f50ac3c5c950f73db769fa76 Mon Sep 17 00:00:00 2001 From: Bouke Versteegh Date: Mon, 21 Feb 2022 15:39:52 +0100 Subject: [PATCH] feat: Support json names containing non-alphanumeric characters (#520) * Fix yarn watch source event handler signature * Fix issue where json_names with hyphens would generate invalid TypeScript --- integration/simple-json-name/simple.bin | Bin 6974 -> 7485 bytes integration/simple-json-name/simple.proto | 10 ++++-- integration/simple-json-name/simple.ts | 42 +++++++++++++++++++++- integration/watch.ts | 2 +- src/main.ts | 39 ++++++++++---------- src/utils.ts | 15 +++++++- 6 files changed, 84 insertions(+), 24 deletions(-) diff --git a/integration/simple-json-name/simple.bin b/integration/simple-json-name/simple.bin index 6f4c9ddf57bfa53a302ab5f6034409253ea124e7..98d2a04860da195ea7bf87330542ea1c7081c80b 100644 GIT binary patch delta 696 zcmYk2K~BOz6o$*pbZB2&Gy_Pf8XzIWfCz}FaimG2A;qR zs7tTl-gpmR=?r1l|9}7czj^&U{kVAlb;*wr{kiAll|-dt7>a%1$WDfx zR=&HbDqTsyN}H&s=qsg8&=FvpIa~|lpQ$6kF1WHqM}l2cuhx-Zdo4AKRjnlnTu_Io t5l4cvK7k{_F@;>?2ypT{N+G~09IaGIBOE&&x2cA)sy6TVd@&1V-G7BeOUM8K delta 197 zcmdmMwa;wBV+p34ER&x~STIdvoGdR{ujs_a#S)*Gp6bNQ#T}nql$uzQni8K_;`NV- zi%p1yfq`qXm6Vn=7b6!J7c&b3qYyJ22Ok$V7Yhpm6N3;lJCpw8dMR@)JuV(DR*(=E zP)tMznWvxwRSwk4!DKr5ft0JP2~+_iP&tRR5i(ERXtI-Zyo8xIRFDa%gww%0iLIb0 Iza-xn0DvhT-T(jq diff --git a/integration/simple-json-name/simple.proto b/integration/simple-json-name/simple.proto index 48bf36f0c..67ecd6b33 100644 --- a/integration/simple-json-name/simple.proto +++ b/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$"]; } diff --git a/integration/simple-json-name/simple.ts b/integration/simple-json-name/simple.ts index 2b5118d05..7709acca3 100644 --- a/integration/simple-json-name/simple.ts +++ b/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; }, }; diff --git a/integration/watch.ts b/integration/watch.ts index 5a074d20e..b1803fdd5 100644 --- a/integration/watch.ts +++ b/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'.`; diff --git a/src/main.ts b/src/main.ts index 687e12be3..49fced731 100644 --- a/src/main.ts +++ b/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,8 +1242,8 @@ 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) { @@ -1249,26 +1251,26 @@ function generateFromJson(ctx: Context, fullName: string, messageDesc: Descripto } } 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;`); diff --git a/src/utils.ts b/src/utils.ts index ed868a5fc..55cd35f1d 100644 --- a/src/utils.ts +++ b/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}`); }