RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.78k stars 1.29k forks source link

`string` should be `string?` for optional properties if `"generateOptionalPropertiesAsNullable": true` #4280

Open SymboLinker opened 1 year ago

SymboLinker commented 1 year ago

NSwagStudio v13.16.1.0

Runtime: Net60 Outputs: CSharpClient Settings:

"generateOptionalPropertiesAsNullable": true,
"generateNullableReferenceTypes": true,

According to its docs, the Newtonsoft.Json.Required.DisallowNull enum value means:

The property is not required but it cannot be a null value.

so then properties that are marked that way are "optional".

For (EDIT: some) value type properties, the generated outputs is:

[Newtonsoft.Json.JsonProperty("repaymentType", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
[System.ComponentModel.DataAnnotations.StringLength(3)]
public string RepaymentType { get; set; } = default!;

[Newtonsoft.Json.JsonProperty("numberOfInstallments", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
[System.ComponentModel.DataAnnotations.Range(1, 99999)]
public int NumberOfInstallments { get; set; } = default!;

This is wrong: string should be string? and int should be int? for these optional properties.

For nullable reference types and enums it always looks OK:

[Newtonsoft.Json.JsonProperty("installmentAmount", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
public MoneyAmount? InstallmentAmount { get; set; } = default!;

EDIT: wow, weird. At some places in the code, string? IS used correctly:

/// <summary>
/// The name of the institution that provides the income.
/// </summary>
[Newtonsoft.Json.JsonProperty("institutionName", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
[System.ComponentModel.DataAnnotations.StringLength(30)]
public string? InstitutionName { get; set; } = default!;

Help for finding were to fix it:

I have more than 15 cases of both and without exception!

markm77 commented 1 year ago

Yeah, I noticed this for strings and DateTimeOffset values. Any chance of a fix @RicoSuter?

RicoSuter commented 5 months ago

Might be fixed with the next version of NJS/NSwag.