Skip to content

chore(dotnet): allow nullable channels #36114

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.

Sign up for GitHub

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

Merged
mxschmitt merged 1 commit into from
May 28, 2025
Merged

chore(dotnet): allow nullable channels #36114

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion utils/generate_dotnet_channels.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ function properties(properties, indent, onlyOptional, parentName, level) {
continue;
ts.push('');
ts.push(`${indent}[JsonPropertyName("${name}")]`);
ts.push(`${indent}public ${inner.ts}${nullableSuffix(inner)} ${toTitleCase(name)} { get; set; }`);
let suffix = ''
if (!['bool', 'int', 'System.Text.Json.JsonElement'].includes(inner.ts))
suffix = ' = null!;'
ts.push(`${indent}public ${inner.ts}${nullableSuffix(inner)} ${toTitleCase(name)} { get; set; }${suffix}`);
Copy link
Contributor

@dgozman dgozman May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this forces everything to be non-null, instead of allowing nullable channels? Could you please explain better what we are trying to achieve?

Copy link
Contributor Author

@mxschmitt mxschmitt May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When enabling nullability in the repository .NET complains that these classes we generate can be created with null values which is correct. In order to avoid that there are three ways:

a) constructor which forces to initialise these values - this doesn't apply to JSON serialises, since these classes are never constructed via us, they are always constructed via the underlying json library. So thats not an option.
b) use the required attribute - this won't work since we have netstandard2.0 but it was introduced in netstandard2.1. So thats not an option.
c) use = null!; - its the verbose way of skipping this null check, internally it was null anyways. In the future we could change it to the required attribute once we move to a more recent TFM.

const wrapped = inner.optional ? `tOptional(${inner.scheme})` : inner.scheme;
scheme.push(`${indent}${name}: ${wrapped},`);
}
Expand Down
Loading