Skip to content

Commit

Permalink
Making repeated deprecated fields also optional
Browse files Browse the repository at this point in the history
  • Loading branch information
@tufandevrim
tufandevrim committed Mar 23, 2024
1 parent 7bbe9ab commit 1252fd1
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 59 deletions.
6 changes: 3 additions & 3 deletions integration/use-optionals-deprecated-only/optionals-test.ts
View file
Expand Up @@ -5,9 +5,8 @@ describe('useOptionals=deprecatedOnly', () => {
const test: OptionalsTest = {
id: 1,
long: 10,

repId: [1, 2],
repState: [StateEnum.ON, StateEnum.OFF],
repStateV2: [StateEnum.ON, StateEnum.OFF],
repLong: [11, 12],
repTruth: [true, false],
repDescription: ["hello", "world"],
Expand Down Expand Up @@ -35,7 +34,8 @@ describe('useOptionals=deprecatedOnly', () => {
data: new Uint8Array(0),

repId: [1, 2],
repState: [StateEnum.ON, StateEnum.OFF],
repState: [],
repStateV2: [StateEnum.ON, StateEnum.OFF],
repLong: [11, 12],
repTruth: [true, false],
repDescription: ["hello", "world"],
Expand Down
Binary file modified integration/use-optionals-deprecated-only/test.bin
View file
Binary file not shown.
23 changes: 12 additions & 11 deletions integration/use-optionals-deprecated-only/test.proto
View file
Expand Up @@ -11,19 +11,20 @@ message OptionalsTest {

repeated int32 rep_id = 7;
repeated StateEnum rep_state = 8 [deprecated = true];
repeated int64 rep_long = 9;
repeated bool rep_truth = 10;
repeated string rep_description = 11;
repeated bytes rep_data = 12;
repeated StateEnum rep_state_v2 = 9;
repeated int64 rep_long = 10;
repeated bool rep_truth = 11;
repeated string rep_description = 12;
repeated bytes rep_data = 13;

optional int32 opt_id = 13;
optional StateEnum opt_state = 14 [deprecated = true];
optional int64 opt_long = 15;
optional bool opt_truth = 16;
optional string opt_description = 17;
optional bytes opt_data = 18;
optional int32 opt_id = 14;
optional StateEnum opt_state = 15 [deprecated = true];
optional int64 opt_long = 16;
optional bool opt_truth = 17;
optional string opt_description = 18;
optional bytes opt_data = 19;

map<string, string> translations = 19;
map<string, string> translations = 20;
}

enum StateEnum {
Expand Down
113 changes: 73 additions & 40 deletions integration/use-optionals-deprecated-only/test.ts
View file
Expand Up @@ -61,7 +61,8 @@ export interface OptionalsTest {
data?: Uint8Array | undefined;
repId: number[];
/** @deprecated */
repState: StateEnum[];
repState?: StateEnum[] | undefined;
repStateV2: StateEnum[];
repLong: number[];
repTruth: boolean[];
repDescription: string[];
Expand Down Expand Up @@ -93,6 +94,7 @@ function createBaseOptionalsTest(): OptionalsTest {
data: new Uint8Array(0),
repId: [],
repState: [],
repStateV2: [],
repLong: [],
repTruth: [],
repDescription: [],
Expand All @@ -118,7 +120,7 @@ export const OptionalsTest = {
if (message.long !== 0) {
writer.uint32(24).int64(message.long);
}
if (message.truth === true) {
if (message.truth !== undefined && message.truth !== false) {
writer.uint32(32).bool(message.truth);
}
if (message.description !== undefined && message.description !== "") {
Expand All @@ -132,47 +134,54 @@ export const OptionalsTest = {
writer.int32(v);
}
writer.ldelim();
writer.uint32(66).fork();
for (const v of message.repState) {
if (message.repState !== undefined && message.repState.length !== 0) {
writer.uint32(66).fork();
for (const v of message.repState) {
writer.int32(v);
}
writer.ldelim();
}
writer.uint32(74).fork();
for (const v of message.repStateV2) {
writer.int32(v);
}
writer.ldelim();
writer.uint32(74).fork();
writer.uint32(82).fork();
for (const v of message.repLong) {
writer.int64(v);
}
writer.ldelim();
writer.uint32(82).fork();
writer.uint32(90).fork();
for (const v of message.repTruth) {
writer.bool(v);
}
writer.ldelim();
for (const v of message.repDescription) {
writer.uint32(90).string(v!);
writer.uint32(98).string(v!);
}
for (const v of message.repData) {
writer.uint32(98).bytes(v!);
writer.uint32(106).bytes(v!);
}
if (message.optId !== undefined) {
writer.uint32(104).int32(message.optId);
writer.uint32(112).int32(message.optId);
}
if (message.optState !== undefined) {
writer.uint32(112).int32(message.optState);
writer.uint32(120).int32(message.optState);
}
if (message.optLong !== undefined) {
writer.uint32(120).int64(message.optLong);
writer.uint32(128).int64(message.optLong);
}
if (message.optTruth !== undefined) {
writer.uint32(128).bool(message.optTruth);
writer.uint32(136).bool(message.optTruth);
}
if (message.optDescription !== undefined) {
writer.uint32(138).string(message.optDescription);
writer.uint32(146).string(message.optDescription);
}
if (message.optData !== undefined) {
writer.uint32(146).bytes(message.optData);
writer.uint32(154).bytes(message.optData);
}
Object.entries(message.translations).forEach(([key, value]) => {
OptionalsTest_TranslationsEntry.encode({ key: key as any, value }, writer.uint32(154).fork()).ldelim();
OptionalsTest_TranslationsEntry.encode({ key: key as any, value }, writer.uint32(162).fork()).ldelim();
});
return writer;
},
Expand Down Expand Up @@ -228,15 +237,15 @@ export const OptionalsTest = {
continue;
case 7:
if (tag === 56) {
message.repId.push(reader.int32());
message.repId!.push(reader.int32());

continue;
}

if (tag === 58) {
const end2 = reader.uint32() + reader.pos;
while (reader.pos < end2) {
message.repId.push(reader.int32());
message.repId!.push(reader.int32());
}

continue;
Expand All @@ -245,15 +254,15 @@ export const OptionalsTest = {
break;
case 8:
if (tag === 64) {
message.repState.push(reader.int32() as any);
message.repState!.push(reader.int32() as any);

continue;
}

if (tag === 66) {
const end2 = reader.uint32() + reader.pos;
while (reader.pos < end2) {
message.repState.push(reader.int32() as any);
message.repState!.push(reader.int32() as any);
}

continue;
Expand All @@ -262,15 +271,15 @@ export const OptionalsTest = {
break;
case 9:
if (tag === 72) {
message.repLong.push(longToNumber(reader.int64() as Long));
message.repStateV2!.push(reader.int32() as any);

continue;
}

if (tag === 74) {
const end2 = reader.uint32() + reader.pos;
while (reader.pos < end2) {
message.repLong.push(longToNumber(reader.int64() as Long));
message.repStateV2!.push(reader.int32() as any);
}

continue;
Expand All @@ -279,85 +288,102 @@ export const OptionalsTest = {
break;
case 10:
if (tag === 80) {
message.repTruth.push(reader.bool());
message.repLong!.push(longToNumber(reader.int64() as Long));

continue;
}

if (tag === 82) {
const end2 = reader.uint32() + reader.pos;
while (reader.pos < end2) {
message.repTruth.push(reader.bool());
message.repLong!.push(longToNumber(reader.int64() as Long));
}

continue;
}

break;
case 11:
if (tag !== 90) {
break;
if (tag === 88) {
message.repTruth!.push(reader.bool());

continue;
}

message.repDescription.push(reader.string());
continue;
if (tag === 90) {
const end2 = reader.uint32() + reader.pos;
while (reader.pos < end2) {
message.repTruth!.push(reader.bool());
}

continue;
}

break;
case 12:
if (tag !== 98) {
break;
}

message.repData.push(reader.bytes());
message.repDescription!.push(reader.string());
continue;
case 13:
if (tag !== 104) {
if (tag !== 106) {
break;
}

message.optId = reader.int32();
message.repData!.push(reader.bytes());
continue;
case 14:
if (tag !== 112) {
break;
}

message.optState = reader.int32() as any;
message.optId = reader.int32();
continue;
case 15:
if (tag !== 120) {
break;
}

message.optLong = longToNumber(reader.int64() as Long);
message.optState = reader.int32() as any;
continue;
case 16:
if (tag !== 128) {
break;
}

message.optTruth = reader.bool();
message.optLong = longToNumber(reader.int64() as Long);
continue;
case 17:
if (tag !== 138) {
if (tag !== 136) {
break;
}

message.optDescription = reader.string();
message.optTruth = reader.bool();
continue;
case 18:
if (tag !== 146) {
break;
}

message.optData = reader.bytes();
message.optDescription = reader.string();
continue;
case 19:
if (tag !== 154) {
break;
}

const entry19 = OptionalsTest_TranslationsEntry.decode(reader, reader.uint32());
if (entry19.value !== undefined) {
message.translations[entry19.key] = entry19.value;
message.optData = reader.bytes();
continue;
case 20:
if (tag !== 162) {
break;
}

const entry20 = OptionalsTest_TranslationsEntry.decode(reader, reader.uint32());
if (entry20.value !== undefined) {
message.translations![entry20.key] = entry20.value;
}
continue;
}
Expand All @@ -379,6 +405,9 @@ export const OptionalsTest = {
data: isSet(object.data) ? bytesFromBase64(object.data) : new Uint8Array(0),
repId: globalThis.Array.isArray(object?.repId) ? object.repId.map((e: any) => globalThis.Number(e)) : [],
repState: globalThis.Array.isArray(object?.repState) ? object.repState.map((e: any) => stateEnumFromJSON(e)) : [],
repStateV2: globalThis.Array.isArray(object?.repStateV2)
? object.repStateV2.map((e: any) => stateEnumFromJSON(e))
: [],
repLong: globalThis.Array.isArray(object?.repLong) ? object.repLong.map((e: any) => globalThis.Number(e)) : [],
repTruth: globalThis.Array.isArray(object?.repTruth)
? object.repTruth.map((e: any) => globalThis.Boolean(e))
Expand Down Expand Up @@ -413,7 +442,7 @@ export const OptionalsTest = {
if (message.long !== 0) {
obj.long = Math.round(message.long);
}
if (message.truth === true) {
if (message.truth !== undefined && message.truth !== false) {
obj.truth = message.truth;
}
if (message.description !== undefined && message.description !== "") {
Expand All @@ -428,6 +457,9 @@ export const OptionalsTest = {
if (message.repState?.length) {
obj.repState = message.repState.map((e) => stateEnumToJSON(e));
}
if (message.repStateV2?.length) {
obj.repStateV2 = message.repStateV2.map((e) => stateEnumToJSON(e));
}
if (message.repLong?.length) {
obj.repLong = message.repLong.map((e) => Math.round(e));
}
Expand Down Expand Up @@ -483,6 +515,7 @@ export const OptionalsTest = {
message.data = object.data ?? new Uint8Array(0);
message.repId = object.repId?.map((e) => e) || [];
message.repState = object.repState?.map((e) => e) || [];
message.repStateV2 = object.repStateV2?.map((e) => e) || [];
message.repLong = object.repLong?.map((e) => e) || [];
message.repTruth = object.repTruth?.map((e) => e) || [];
message.repDescription = object.repDescription?.map((e) => e) || [];
Expand Down
2 changes: 1 addition & 1 deletion src/main.ts
View file
Expand Up @@ -1212,7 +1212,7 @@ function generateDecode(ctx: Context, fullName: string, messageDesc: DescriptorP
isOptionalProperty(field, messageDesc.options, options, currentFile.isProto3Syntax);

if (isRepeated(field)) {
const maybeNonNullAssertion = ctx.options.useOptionals === "all" ? "!" : "";
const maybeNonNullAssertion = ctx.options.useOptionals === "all" || ctx.options.useOptionals === "deprecatedOnly" ? "!" : "";

const mapType = detectMapType(ctx, messageDesc, field);
if (mapType) {
Expand Down
10 changes: 6 additions & 4 deletions src/types.ts
View file
Expand Up @@ -352,9 +352,12 @@ export function isScalar(field: FieldDescriptorProto): boolean {
return scalarTypes.includes(field.type);
}

// When useOptionals='deprecatedOnly', all deprecated fields are translated into optional
// When useOptionals='messages', non-scalar fields are translated into optional
// properties. When useOptionals='all', all fields are translated into
// properties.
// When useOptionals='all', all fields are translated into
// optional properties, with the exception of map Entry key/values, which must
// always be present.
// When useOptionals='deprecatedOnly', all deprecated fields are translated into
// optional properties, with the exception of map Entry key/values, which must
// always be present.
// OneOf fields are always optional, whenever oneof=unions option not in use.
Expand All @@ -371,13 +374,12 @@ export function isOptionalProperty(

return (
(optionalMessages && isMessage(field) && !isRepeated(field)) ||
(optionalAll && !messageOptions?.mapEntry) ||
((optionalAll || deprecatedOnly) && !messageOptions?.mapEntry) ||
// file is proto2, we have enabled proto2 optionals, and the field itself is optional
(!isProto3Syntax &&
field.label === FieldDescriptorProto_Label.LABEL_OPTIONAL &&
!messageOptions?.mapEntry &&
!options.disableProto2Optionals) ||
(deprecatedOnly && !isMessage(field) && !isRepeated(field)) ||
// don't bother verifying that oneof is not union. union oneofs generate their own properties.
isWithinOneOf(field) ||
field.proto3Optional
Expand Down

0 comments on commit 1252fd1

Please sign in to comment.