Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for comments on union fields in generateOneofProperty #1136

Merged
merged 4 commits into from
Nov 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions integration/oneof-unions-snake/google/protobuf/struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,26 @@ export interface Struct_FieldsEntry {
* The JSON representation for `Value` is JSON value.
*/
export interface Value {
/** The kind of value. */
kind?:
| { $case: "null_value"; null_value: NullValue }
| { $case: "number_value"; number_value: number }
| { $case: "string_value"; string_value: string }
| { $case: "bool_value"; bool_value: boolean }
| { $case: "struct_value"; struct_value: { [key: string]: any } | undefined }
| { $case: "list_value"; list_value: Array<any> | undefined }
| //
/** Represents a null value. */
{ $case: "null_value"; null_value: NullValue }
| //
/** Represents a double value. */
{ $case: "number_value"; number_value: number }
| //
/** Represents a string value. */
{ $case: "string_value"; string_value: string }
| //
/** Represents a boolean value. */
{ $case: "bool_value"; bool_value: boolean }
| //
/** Represents a structured value. */
{ $case: "struct_value"; struct_value: { [key: string]: any } | undefined }
| //
/** Represents a repeated `Value`. */
{ $case: "list_value"; list_value: Array<any> | undefined }
| undefined;
}

Expand Down
25 changes: 19 additions & 6 deletions integration/oneof-unions-value/google/protobuf/struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,26 @@ export interface Struct_FieldsEntry {
* The JSON representation for `Value` is JSON value.
*/
export interface Value {
/** The kind of value. */
kind?:
| { $case: "nullValue"; value: NullValue }
| { $case: "numberValue"; value: number }
| { $case: "stringValue"; value: string }
| { $case: "boolValue"; value: boolean }
| { $case: "structValue"; value: { [key: string]: any } | undefined }
| { $case: "listValue"; value: Array<any> | undefined }
| //
/** Represents a null value. */
{ $case: "nullValue"; value: NullValue }
| //
/** Represents a double value. */
{ $case: "numberValue"; value: number }
| //
/** Represents a string value. */
{ $case: "stringValue"; value: string }
| //
/** Represents a boolean value. */
{ $case: "boolValue"; value: boolean }
| //
/** Represents a structured value. */
{ $case: "structValue"; value: { [key: string]: any } | undefined }
| //
/** Represents a repeated `Value`. */
{ $case: "listValue"; value: Array<any> | undefined }
| undefined;
}

Expand Down
46 changes: 36 additions & 10 deletions integration/oneof-unions-value/oneof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,45 @@ export const protobufPackage = "oneof";

export interface PleaseChoose {
name: string;
/**
* Please to be choosing one of the fields within this oneof clause.
* This text exists to ensure we transpose comments correctly.
*/
choice?:
| { $case: "aNumber"; value: number }
| { $case: "aString"; value: string }
| { $case: "aMessage"; value: PleaseChoose_Submessage }
| { $case: "aBool"; value: boolean }
| { $case: "bunchaBytes"; value: Uint8Array }
| { $case: "anEnum"; value: PleaseChoose_StateEnum }
| //
/**
* Use this if you want a number. Numbers are great. Who doesn't
* like them?
*/
{ $case: "aNumber"; value: number }
| //
/**
* Use this if you want a string. Strings are also nice. Not as
* nice as numbers, but what are you going to do...
*/
{ $case: "aString"; value: string }
| //
{ $case: "aMessage"; value: PleaseChoose_Submessage }
| //
/**
* We also added a bool option! This was added after the 'age'
* field, so it has a higher number.
*/
{ $case: "aBool"; value: boolean }
| //
{ $case: "bunchaBytes"; value: Uint8Array }
| //
{ $case: "anEnum"; value: PleaseChoose_StateEnum }
| undefined;
age: number;
eitherOr?: { $case: "either"; value: string } | { $case: "or"; value: string } | {
$case: "thirdOption";
value: string;
} | undefined;
eitherOr?:
| //
{ $case: "either"; value: string }
| //
{ $case: "or"; value: string }
| //
{ $case: "thirdOption"; value: string }
| undefined;
signature: Uint8Array;
value: any | undefined;
}
Expand Down
25 changes: 19 additions & 6 deletions integration/oneof-unions/google/protobuf/struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,26 @@ export interface Struct_FieldsEntry {
* The JSON representation for `Value` is JSON value.
*/
export interface Value {
/** The kind of value. */
kind?:
| { $case: "nullValue"; nullValue: NullValue }
| { $case: "numberValue"; numberValue: number }
| { $case: "stringValue"; stringValue: string }
| { $case: "boolValue"; boolValue: boolean }
| { $case: "structValue"; structValue: { [key: string]: any } | undefined }
| { $case: "listValue"; listValue: Array<any> | undefined }
| //
/** Represents a null value. */
{ $case: "nullValue"; nullValue: NullValue }
| //
/** Represents a double value. */
{ $case: "numberValue"; numberValue: number }
| //
/** Represents a string value. */
{ $case: "stringValue"; stringValue: string }
| //
/** Represents a boolean value. */
{ $case: "boolValue"; boolValue: boolean }
| //
/** Represents a structured value. */
{ $case: "structValue"; structValue: { [key: string]: any } | undefined }
| //
/** Represents a repeated `Value`. */
{ $case: "listValue"; listValue: Array<any> | undefined }
| undefined;
}

Expand Down
46 changes: 36 additions & 10 deletions integration/oneof-unions/oneof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,45 @@ export const protobufPackage = "oneof";

export interface PleaseChoose {
name: string;
/**
* Please to be choosing one of the fields within this oneof clause.
* This text exists to ensure we transpose comments correctly.
*/
choice?:
| { $case: "aNumber"; aNumber: number }
| { $case: "aString"; aString: string }
| { $case: "aMessage"; aMessage: PleaseChoose_Submessage }
| { $case: "aBool"; aBool: boolean }
| { $case: "bunchaBytes"; bunchaBytes: Uint8Array }
| { $case: "anEnum"; anEnum: PleaseChoose_StateEnum }
| //
/**
* Use this if you want a number. Numbers are great. Who doesn't
* like them?
*/
{ $case: "aNumber"; aNumber: number }
| //
/**
* Use this if you want a string. Strings are also nice. Not as
* nice as numbers, but what are you going to do...
*/
{ $case: "aString"; aString: string }
| //
{ $case: "aMessage"; aMessage: PleaseChoose_Submessage }
| //
/**
* We also added a bool option! This was added after the 'age'
* field, so it has a higher number.
*/
{ $case: "aBool"; aBool: boolean }
| //
{ $case: "bunchaBytes"; bunchaBytes: Uint8Array }
| //
{ $case: "anEnum"; anEnum: PleaseChoose_StateEnum }
| undefined;
age: number;
eitherOr?: { $case: "either"; either: string } | { $case: "or"; or: string } | {
$case: "thirdOption";
thirdOption: string;
} | undefined;
eitherOr?:
| //
{ $case: "either"; either: string }
| //
{ $case: "or"; or: string }
| //
{ $case: "thirdOption"; thirdOption: string }
| undefined;
signature: Uint8Array;
value: any | undefined;
}
Expand Down
25 changes: 19 additions & 6 deletions integration/use-readonly-types/google/protobuf/struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,26 @@ export interface Struct_FieldsEntry {
* The JSON representation for `Value` is JSON value.
*/
export interface Value {
/** The kind of value. */
readonly kind?:
| { readonly $case: "nullValue"; readonly nullValue: NullValue }
| { readonly $case: "numberValue"; readonly numberValue: number }
| { readonly $case: "stringValue"; readonly stringValue: string }
| { readonly $case: "boolValue"; readonly boolValue: boolean }
| { readonly $case: "structValue"; readonly structValue: { readonly [key: string]: any } | undefined }
| { readonly $case: "listValue"; readonly listValue: ReadonlyArray<any> | undefined }
| //
/** Represents a null value. */
{ readonly $case: "nullValue"; readonly nullValue: NullValue }
| //
/** Represents a double value. */
{ readonly $case: "numberValue"; readonly numberValue: number }
| //
/** Represents a string value. */
{ readonly $case: "stringValue"; readonly stringValue: string }
| //
/** Represents a boolean value. */
{ readonly $case: "boolValue"; readonly boolValue: boolean }
| //
/** Represents a structured value. */
{ readonly $case: "structValue"; readonly structValue: { readonly [key: string]: any } | undefined }
| //
/** Represents a repeated `Value`. */
{ readonly $case: "listValue"; readonly listValue: ReadonlyArray<any> | undefined }
| undefined;
}

Expand Down
10 changes: 6 additions & 4 deletions integration/use-readonly-types/use-readonly-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ export interface Entity {
readonly fieldMask: readonly string[] | undefined;
readonly listValue: ReadonlyArray<any> | undefined;
readonly structValue: { readonly [key: string]: any } | undefined;
readonly oneOfValue?: { readonly $case: "theStringValue"; readonly theStringValue: string } | {
readonly $case: "theIntValue";
readonly theIntValue: number;
} | undefined;
readonly oneOfValue?:
| //
{ readonly $case: "theStringValue"; readonly theStringValue: string }
| //
{ readonly $case: "theIntValue"; readonly theIntValue: number }
| undefined;
}

export interface SubEntity {
Expand Down
46 changes: 19 additions & 27 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1174,41 +1174,33 @@ function generateOneofProperty(
sourceInfo: SourceInfo,
): Code {
const { options } = ctx;
const fields = messageDesc.field.filter((field) => isWithinOneOf(field) && field.oneofIndex === oneofIndex);
const fields = messageDesc.field
.map((field, index) => ({ index, field }))
.filter((item) => isWithinOneOf(item.field) && item.field.oneofIndex === oneofIndex);

const mbReadonly = maybeReadonly(options);
const info = sourceInfo.lookup(Fields.message.oneof_decl, oneofIndex);
let outerComments: Code[] = [];
maybeAddComment(options, info, outerComments);

const unionType = joinCode(
fields.map((f) => {
let fieldName = maybeSnakeToCamel(f.name, options);
let typeName = toTypeName(ctx, messageDesc, f);
fields.flatMap((f) => {
const fieldInfo = sourceInfo.lookup(Fields.message.field, f.index);
let fieldName = maybeSnakeToCamel(f.field.name, options);
let typeName = toTypeName(ctx, messageDesc, f.field);
let valueName = oneofValueName(fieldName, options);
return code`{ ${mbReadonly}$case: '${fieldName}', ${mbReadonly}${valueName}: ${typeName} }`;
let fieldComments: Code[] = [];
maybeAddComment(options, fieldInfo, fieldComments);

const combinedComments = fieldComments.join("\n");
return code`| // \n ${combinedComments} { ${mbReadonly}$case: '${fieldName}', ${mbReadonly}${valueName}: ${typeName} }`;
}),
{ on: " | " },
);

const name = maybeSnakeToCamel(messageDesc.oneofDecl[oneofIndex].name, options);
return code`${mbReadonly}${name}?: ${unionType} | ${nullOrUndefined(options)},`;

/*
// Ideally we'd put the comments for each oneof field next to the anonymous
// type we've created in the type union above, but ts-poet currently lacks
// that ability. For now just concatenate all comments into one big one.
let comments: Array<string> = [];
const info = sourceInfo.lookup(Fields.message.oneof_decl, oneofIndex);
maybeAddComment(options, info, (text) => comments.push(text));
messageDesc.field.forEach((field, index) => {
if (!isWithinOneOf(field) || field.oneofIndex !== oneofIndex) {
return;
}
const info = sourceInfo.lookup(Fields.message.field, index);
const name = maybeSnakeToCamel(field.name, options);
maybeAddComment(options, info, (text) => comments.push(name + '\n' + text));
return joinCode([...outerComments, code`${mbReadonly}${name}?:`, unionType, code`| ${nullOrUndefined(options)},`], {
on: "\n",
});
if (comments.length) {
prop = prop.addJavadoc(comments.join('\n'));
}
return prop;
*/
}

// Create a function that constructs 'base' instance with default values for decode to use as a prototype
Expand Down
Loading