Update oneof representation for v3 (#262)

MANUAL.md

@@ -636,7 +636,7 @@ idiosyncrasies.
 message OneofExample {
  // Only one of (or none of) the fields can be set
  oneof result { 
-   int32 value = 1;
+   int32 int = 1;
    string error = 2;
  }
 }
@@ -645,13 +645,13 @@ Compiles the `oneof` group to a union type that ensures that only one member
 field is set:
 ```ts   
 interface OneofExample {
- result: { oneofKind: "value"; value: number; } 
-       | { oneofKind: "error"; error: string; } 
-       | { oneofKind: undefined; };
+ result: { kind: "int"; value: number; } 
+       | { kind: "error"; value: string; } 
+       | { kind: undefined; };
 }
 
 let message: OneofExample;
-if (message.result.oneofKind === "value") {
+if (message.result.kind === "int") {
  message.result.value // the union has been narrowed down
 }
 ```

packages/example-chat-system/browser-client/index.ts

@@ -43,16 +43,14 @@ joinPanel.startCallback = async (username) => {
 
     // print all chat events
     call.responses.onMessage(message => {
-        switch (message.event.oneofKind) {
+        switch (message.event.kind) {
             case "joined":
-                chatPanel.addOther(message.event.joined);
-                break;
             case "left":
-                chatPanel.addOther(message.event.left);
+                chatPanel.addOther(message.event.value);
                 break;
             case "message":
-                chatPanel.addMessage(message.username, message.event.message);
-                console.log(`${message.username}: ${message.event.message}`);
+                chatPanel.addMessage(message.username, message.event.value);
+                console.log(`${message.username}: ${message.event.value}`);
                 break;
         }
     });

packages/example-chat-system/browser-client/service-chat.ts

@@ -24,25 +24,26 @@ export interface ChatEvent {
      * @generated from protobuf oneof: event
      */
     event: {
-        oneofKind: "joined";
+        kind: "joined";
         /**
          * @generated from protobuf field: string joined = 2;
          */
-        joined: string;
+        value: string;
     } | {
-        oneofKind: "message";
+        kind: "message";
         /**
          * @generated from protobuf field: string message = 3;
          */
-        message: string;
+        value: string;
     } | {
-        oneofKind: "left";
+        kind: "left";
         /**
          * @generated from protobuf field: string left = 4;
          */
-        left: string;
+        value: string;
     } | {
-        oneofKind: undefined;
+        kind: undefined;
+        value?: never;
     };
 }
 /**

packages/example-chat-system/server/server.ts

@@ -26,8 +26,8 @@ class ChatService implements IChatService {
         await this.broadcast({
             username: request.username,
             event: {
-                oneofKind: "joined",
-                joined: `${request.username} joined the chat`
+                kind: "joined",
+                value: `${request.username} joined the chat`
             }
         });
 
@@ -37,8 +37,8 @@ class ChatService implements IChatService {
         await this.broadcast({
             username: request.username,
             event: {
-                oneofKind: "left",
-                left: `${request.username} left the chat`
+                kind: "left",
+                value: `${request.username} left the chat`
             }
         });
 
@@ -54,8 +54,8 @@ class ChatService implements IChatService {
         await this.broadcast({
             username: user.username,
             event: {
-                oneofKind: 'message',
-                message: request.message
+                kind: 'message',
+                value: request.message
             }
         });
 

packages/example-chat-system/server/service-chat.ts

@@ -30,25 +30,26 @@ export interface ChatEvent {
      * @generated from protobuf oneof: event
      */
     event: {
-        oneofKind: "joined";
+        kind: "joined";
         /**
          * @generated from protobuf field: string joined = 2;
          */
-        joined: string;
+        value: string;
     } | {
-        oneofKind: "message";
+        kind: "message";
         /**
          * @generated from protobuf field: string message = 3;
          */
-        message: string;
+        value: string;
     } | {
-        oneofKind: "left";
+        kind: "left";
         /**
          * @generated from protobuf field: string left = 4;
          */
-        left: string;
+        value: string;
     } | {
-        oneofKind: undefined;
+        kind: undefined;
+        value?: never;
     };
 }
 /**
@@ -125,20 +126,20 @@ class ChatEvent$Type extends MessageType<ChatEvent> {
                     break;
                 case /* string joined */ 2:
                     message.event = {
-                        oneofKind: "joined",
-                        joined: reader.string()
+                        kind: "joined",
+                        value: reader.string()
                     };
                     break;
                 case /* string message */ 3:
                     message.event = {
-                        oneofKind: "message",
-                        message: reader.string()
+                        kind: "message",
+                        value: reader.string()
                     };
                     break;
                 case /* string left */ 4:
                     message.event = {
-                        oneofKind: "left",
-                        left: reader.string()
+                        kind: "left",
+                        value: reader.string()
                     };
                     break;
                 default:
@@ -157,14 +158,14 @@ class ChatEvent$Type extends MessageType<ChatEvent> {
         if (message.username !== "")
             writer.tag(1, WireType.LengthDelimited).string(message.username);
         /* string joined = 2; */
-        if (message.event.oneofKind === "joined")
-            writer.tag(2, WireType.LengthDelimited).string(message.event.joined);
+        if (message.event.kind === "joined")
+            writer.tag(2, WireType.LengthDelimited).string(message.event.value);
         /* string message = 3; */
-        if (message.event.oneofKind === "message")
-            writer.tag(3, WireType.LengthDelimited).string(message.event.message);
+        if (message.event.kind === "message")
+            writer.tag(3, WireType.LengthDelimited).string(message.event.value);
         /* string left = 4; */
-        if (message.event.oneofKind === "left")
-            writer.tag(4, WireType.LengthDelimited).string(message.event.left);
+        if (message.event.kind === "left")
+            writer.tag(4, WireType.LengthDelimited).string(message.event.value);
         let u = options.writeUnknownFields;
         if (u !== false)
             (u == true ? UnknownFieldHandler.onWrite : u)(this.typeName, message, writer);

packages/example-chat-system/terminal-client/client.ts

@@ -32,15 +32,13 @@ async function main(term: TerminalIO, client: IChatServiceClient) {
 
     // print all chat events
     call.responses.onMessage(message => {
-        switch (message.event.oneofKind) {
+        switch (message.event.kind) {
             case "joined":
-                term.print(`* ${message.event.joined}`);
-                break;
             case "left":
-                term.print(`* ${message.event.left}`);
+                term.print(`* ${message.event.value}`);
                 break;
             case "message":
-                term.print(`${message.username}: ${message.event.message}`);
+                term.print(`${message.username}: ${message.event.value}`);
                 break;
         }
     });

packages/example-chat-system/terminal-client/service-chat.ts

@@ -24,25 +24,26 @@ export interface ChatEvent {
      * @generated from protobuf oneof: event
      */
     event: {
-        oneofKind: "joined";
+        kind: "joined";
         /**
          * @generated from protobuf field: string joined = 2;
          */
-        joined: string;
+        value: string;
     } | {
-        oneofKind: "message";
+        kind: "message";
         /**
          * @generated from protobuf field: string message = 3;
          */
-        message: string;
+        value: string;
     } | {
-        oneofKind: "left";
+        kind: "left";
         /**
          * @generated from protobuf field: string left = 4;
          */
-        left: string;
+        value: string;
     } | {
-        oneofKind: undefined;
+        kind: undefined;
+        value?: never;
     };
 }
 /**

packages/plugin/spec/interpreter.spec.ts

@@ -1,6 +1,6 @@
 import {getFixtureFileDescriptor} from "./support/helpers";
 import {DescriptorRegistry} from "@protobuf-ts/plugin-framework";
-import {Interpreter} from "../src/interpreter";
+import {Interpreter, reservedObjectProperties, reservedClassProperties} from "../src/interpreter";
 import * as rt from "@protobuf-ts/runtime";
 
 
@@ -12,7 +12,7 @@ describe('interpreter', function () {
             const interpreter = new Interpreter(registry, {
                 normalLongType,
                 synthesizeEnumZeroValue: 'UNSPECIFIED$',
-                oneofKindDiscriminator: 'oneofKind',
+                oneofKindDiscriminator: 'kind',
                 forceExcludeAllOptions: false,
                 keepEnumPrefix: false,
                 useProtoFieldName: false,
@@ -25,8 +25,77 @@ describe('interpreter', function () {
             expectFieldType(messageType, 'fixed64_field_min', rt.LongType.BIGINT);
         });
     });
+
+    describe('name clashes', function () {
+        const registry = DescriptorRegistry.createFrom(getFixtureFileDescriptor("name-clash.proto"));
+        const interpreter = new Interpreter(registry, {
+            normalLongType: rt.LongType.NUMBER,
+            synthesizeEnumZeroValue: 'UNSPECIFIED$',
+            oneofKindDiscriminator: 'kind',
+            forceExcludeAllOptions: false,
+            keepEnumPrefix: false,
+            useProtoFieldName: false,
+        });
+
+        it('ReservedFieldNames is escaped', function () {
+            const messageType = interpreter.getMessageType('spec.ReservedFieldNames');
+            messageType.fields.forEach((field) => {
+                expect(reservedObjectProperties.has(field.localName)).toBeFalse();
+            });
+            expect(getLocalName(messageType, 'kind')).toBe('kind');
+        });
+
+        it('ReservedFieldNamesInOneof is escaped', function () {
+            const messageType = interpreter.getMessageType('spec.ReservedFieldNamesInOneof');
+            messageType.fields.forEach((field) => {
+                expect(reservedObjectProperties.has(field.localName)).toBeFalse();
+            });
+            expect(getLocalName(messageType, 'kind')).toBe('kind');
+        });
+
+        it('NameClashService is escaped', function () {
+            const serviceType = interpreter.getServiceType('spec.NameClashService');
+            serviceType.methods.forEach((field) => {
+                expect(reservedClassProperties.has(field.localName)).toBeFalse();
+            });
+        })
+
+        describe('oneof descriminator kind', function () {
+            it('is escaped in OneofDiscriminatorClash', function () {
+                const messageType = interpreter.getMessageType('spec.OneofDiscriminatorClash');
+                expect(getLocalName(messageType, 'kind')).toBe('kind$');
+            });
+
+            it('is escaped in OneofDiscriminatorClashInOneof', function () {
+                const messageType = interpreter.getMessageType('spec.OneofDiscriminatorClashInOneof');
+                expect(getLocalName(messageType, 'kind')).toBe('kind$');
+            });
+
+            it('is escaped in OneofDiscriminatorClashNumber', function () {
+                const messageType = interpreter.getMessageType('spec.OneofDiscriminatorClashNumber');
+                expect(getLocalName(messageType, 'kind')).toBe('kind$');
+            });
+
+            it('is NOT escaped in NoOneofDiscriminatorClashNumber', function () {
+                const messageType = interpreter.getMessageType('spec.NoOneofDiscriminatorClashNumber');
+                expect(getLocalName(messageType, 'kind')).toBe('kind');
+            });
+
+            it('is NOT escaped in NoOneofDiscriminatorClashRepeated', function () {
+                const messageType = interpreter.getMessageType('spec.NoOneofDiscriminatorClashRepeated');
+                expect(getLocalName(messageType, 'kind')).toBe('kind');
+            });
+        })
+    });
 });
 
+// Expect to find a scalar field `name` of type `type`.
+function getLocalName(messageType: rt.IMessageType<rt.UnknownMessage>, name: string) {
+    const field = messageType.fields.find(f => f.name === name);
+    expect(field).toBeDefined();
+    return field!.localName;
+}
+
 // Expect to find a scalar field `name` of type `type`.
 function expectFieldType(messageType: rt.IMessageType<rt.UnknownMessage>, name: string, type: rt.LongType) {
     const field = messageType.fields.find(f => f.name === name);