From d1b7c080b8b3b84a8f37a24ad21444129c6efccf Mon Sep 17 00:00:00 2001
From: Stephen Haberman <stephen.haberman@gmail.com>
Date: Sun, 14 Aug 2022 11:09:26 -0500
Subject: [PATCH] fix: Use jsonName even with snakeToCamel=false.

Previously we only checked jsonName if snakeToCamel=json was enabled,
b/c protobuf _always_ set jsonName to the camelized key name OR the
jsonName attribute value.

Because of this, we couldn't do logic like "if jsonName is explicitly
set, use that, otherwise keep the field name as-is".

Now we guess about whether jsonName is explicitly set by doing our
own snake-field-name -> camel-json-name, and if our camel-json-name
matches jsonName, then we assume the user has _not_ set jsonName,
and we should keep using snake-field-name.

But if those two values, camel-json-name and jsonName, are different,
then the user has probably manually set jsonName in the proto file,
and we'll use that instead.

Fixes #635
---
 src/case.ts        | 33 +++++++++++++++++----------------
 src/utils.ts       | 13 ++++++++++---
 tests/case-test.ts | 11 +++++++++++
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/src/case.ts b/src/case.ts
index ec6d5a320..820a60350 100644
--- a/src/case.ts
+++ b/src/case.ts
@@ -1,27 +1,28 @@
 import { Options } from './options';
 
-export function maybeSnakeToCamel(s: string, options: Pick<Options, 'snakeToCamel'>): string {
-  if (options.snakeToCamel.includes('keys') && s.includes('_')) {
-    const hasLowerCase = !!s.match(/[a-z]/);
-    return s
-      .split('_')
-      .map((word, i) => {
-        // If the word is already mixed case, leave the exist case as-is
-        word = hasLowerCase ? word : word.toLowerCase();
-        return i === 0 ? word : capitalize(word);
-      })
-      .join('');
+/** Converts `key` to TS/JS camel-case idiom, unless overridden not to. */
+export function maybeSnakeToCamel(key: string, options: Pick<Options, 'snakeToCamel'>): string {
+  if (options.snakeToCamel.includes('keys') && key.includes('_')) {
+    return snakeToCamel(key);
   } else {
-    return s;
+    return key;
   }
 }
 
-export function camelToSnake(s: string): string {
+export function snakeToCamel(s: string): string {
+  const hasLowerCase = !!s.match(/[a-z]/);
   return s
-    .replace(/[\w]([A-Z])/g, function (m) {
-      return m[0] + '_' + m[1];
+    .split('_')
+    .map((word, i) => {
+      // If the word is already mixed case, leave the existing case as-is
+      word = hasLowerCase ? word : word.toLowerCase();
+      return i === 0 ? word : capitalize(word);
     })
-    .toUpperCase();
+    .join('');
+}
+
+export function camelToSnake(s: string): string {
+  return s.replace(/\w([A-Z])/g, (m) => m[0] + '_' + m[1]).toUpperCase();
 }
 
 export function capitalize(s: string): string {
diff --git a/src/utils.ts b/src/utils.ts
index 9803bcf1b..ff1752068 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -9,7 +9,7 @@ import {
 import ReadStream = NodeJS.ReadStream;
 import { SourceDescription } from './sourceInfo';
 import { Options, ServiceOption } from './options';
-import { camelCase } from './case';
+import { camelCase, snakeToCamel } from './case';
 
 export function protoFilesToGenerate(request: CodeGeneratorRequest): FileDescriptorProto[] {
   return request.protoFile.filter((f) => request.fileToGenerate.includes(f.name));
@@ -174,13 +174,20 @@ export class FormattedMethodDescriptor implements MethodDescriptorProto {
   }
 }
 
-export function getFieldJsonName(field: FieldDescriptorProto, options: Options): string {
+export function getFieldJsonName(
+  field: Pick<FieldDescriptorProto, 'name' | 'jsonName'>,
+  options: Pick<Options, 'snakeToCamel'>
+): 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')) {
     return field.jsonName;
   } else {
-    return field.name;
+    // The user wants to keep snake case in the JSON, but we still want to see if the jsonName
+    // attribute is set as an explicit override.
+    const probableJsonName = snakeToCamel(field.name);
+    const isJsonNameSet = probableJsonName !== field.jsonName;
+    return isJsonNameSet ? field.jsonName : field.name;
   }
 }
 
diff --git a/tests/case-test.ts b/tests/case-test.ts
index 37dd06b74..aa2bda572 100644
--- a/tests/case-test.ts
+++ b/tests/case-test.ts
@@ -1,5 +1,6 @@
 import { maybeSnakeToCamel } from '../src/case';
 import { Options, optionsFromParameter } from '../src/options';
+import { getFieldJsonName } from '../src/utils';
 
 const keys = optionsFromParameter('snakeToCamel=keys');
 
@@ -45,4 +46,14 @@ describe('case', () => {
   it('converts snake to camel with first underscore and camelize other', () => {
     expect(maybeSnakeToCamel('_uuid_foo', { snakeToCamel: ['keys'] })).toEqual('UuidFoo');
   });
+
+  describe('getFieldJsonName', () => {
+    it('keeps snake case when jsonName is probably not set', () => {
+      expect(getFieldJsonName({ name: 'foo_bar', jsonName: 'fooBar' }, { snakeToCamel: [] })).toBe('foo_bar');
+    });
+
+    it('uses jsonName when it is set', () => {
+      expect(getFieldJsonName({ name: 'foo_bar', jsonName: 'foo' }, { snakeToCamel: [] })).toBe('foo');
+    });
+  });
 });