Repository: stephenh/ts-proto (ts)

Issue:
How to generate camel_case with json_name option?
I tried 4 different options in `snakeToCamel`, but I cound not found for my requirement.
- camel case
- use json_name if exist.
- It works at `fromJSON` & `toJSON`

with below [proto](https://github.com/kanziw/ts-proto/blob/f3c64f2fcfd4ee9c3d8e3b307461771dd84742ad/services/echo/v1/echo.proto),
```proto
message EchoRequest {
  string msg = 1;
  string long_message = 2 [json_name = "long"];
}

message EchoResponse {
  string want_to_camel_case = 1;
}
```

with [buf](https://github.com/kanziw/ts-proto/blob/f3c64f2fcf/buf.gen.yaml),

### 1. `snakeToCamel=false`

Use camel_case (in proto), I expected. [sample](https://github.com/kanziw/ts-proto/blob/f3c64f2fcfd4ee9c3d8e3b307461771dd84742ad/gen/snake-case-false/echo/v1/echo.ts)


### 2. `snakeToCamel=json`

Use json_name, but also use camelCase too. [sample](https://github.com/kanziw/ts-proto/blob/f3c64f2fcf/gen/snake-case-json/echo/v1/echo.ts)


### 3. `snakeToCamel=keys`

Use snakeCase (with json_name), I expected. [sample](https://github.com/kanziw/ts-proto/blob/f3c64f2fcf/gen/snake-case-keys/echo/v1/echo.ts)


### 4. `snakeToCamel=json,keys`

It equals `snakeToCamel=json` option?! [sample](https://github.com/kanziw/ts-proto/blob/f3c64f2fcf/gen/snake-case-json-keys/echo/v1/echo.ts)


---

I think there is a bug in situation 2.

Pull request description:
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

Patch:
diff --git a/src/case.ts b/src/case.ts
index ec6d5a32..820a6035 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 9803bcf1..ff175206 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;
   }
 }
 


Test patch:
diff --git a/tests/case-test.ts b/tests/case-test.ts
index 37dd06b7..aa2bda57 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');
+    });
+  });
 });