Nswag: Support C# 8 nullable reference types

Created on 29 Sep 2019  路  25Comments  路  Source: RicoSuter/NSwag

Since OpenApi allows you to annotate properties as required or not (also for response types) it would be great if client side C# code generation could consider it and create properties and parameters as nullable reference types, if they are not required. https://github.com/swagger-api/swagger-codegen is doing this for typescript for instance already for quite a while.

Thanks (I couldn't find an existing issue regarding this, in case it already exists)

duplicate NJsonSchema.CodeGeneration.CSharp bug

Most helpful comment

With the correct settings this is already done in the generated typescript code, however not for the generated C# code.

We鈥檇 need a setting to generate C# 8 code with NRT.

All 25 comments

With the correct settings this is already done in the generated typescript code, however not for the generated C# code.

We鈥檇 need a setting to generate C# 8 code with NRT.

@RicoSuter What are those settings for the generated TypeScript code? We have non-nullable properties in many of our (output) data classes, but in the TypeScript code they get generated as optional (name?: Type). If either [Required] or [JsonProperty(Required = Required.Always)] is added to the property then the TypeScript code correctly gets non-optional properties. It's annoying having to add those attributes to the source C# code though when C# 8 already conveys those semantics.

This also happened for struct types from C# before C# 8 was a thing (which can never be null, yet the generated TypeScript code treats them as optional).

Example:

Source C# class:

public class Article
{
  public Article(Guid id, string name)
  {
    Id = id;
    Name = name;
  }

  public Guid Id { get; }
  public string Name { get; } // Under C# 8, this is a non-nullable property
}

The generated TypeScript class becomes:

export class Article implements IArticle {
    id?: string;
    name?: string;
}

Additionally, what decides if a property is "optional"? Just the lack of a [Required] or similar attribute?

If I have a class with a ref of another class like this

PContactInfo techContactInfo;

which is not [required], I need to change this to

PContactInfo? techContactInfo;

for C# 8 with #nullable enabled.

However, the swagger json code changes a lot. Why is this extra OneOf added? It is not understood by several openapi-generators like the one for ELM, and it doesn't add anything semantically.

So, I have now learned, I should not use C#8 nullable when defining classes to be visible in openapi. :-(

           "techContactInfo": {

-            "nullable": true,
-
-            "oneOf": [
-
-              {
-
-                "$ref": "#/components/schemas/PContactInfo"
-
-              }
-
-            ]
+            "$ref": "#/components/schemas/PContactInfo"

           },

When I look at the specification of OneOf at https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/ it seems to me that adding a OneOf with just one option is useless, and only there to confuse the reader. Or what am I missing?

The schema PContactInfo itself is not (always) nullable but the property techContactInfo is, there might be properties with the same schema but which are not nullable. $ref must be alone in an object (no other props are allowed) that鈥檚 why it is wrapped in a oneOf

Hi Rico, Plz do not ask me why the OneOf is there. It has been added by NSwag.

I have an aspnet dotnet-core-3 application, and this is the swagger.json I get from nswag.

"x-generator": "NSwag v13.1.3.0 (NJsonSchema v10.0.27.0 (Newtonsoft.Json v12.0.0.0))",
"openapi": "3.0.0",
"info": {
"title": "SpreadsheetConverter Submit Server",
"version": "1.0.0"
},

Hi @RicoSuter , you mentioned at the top that this was possible for the generated TS code. Does that mean that there are existing settings we could use that would generate the correct nullable TS code for a web api 2 nullable response (NRT)?

For example:

// that this C#
public Foo? GetFoo() { return null; }

// would generate this TS
getFoo(): Observable<Foo | null> { ... }

I've tried with different settings, my default ref type handling is set to NonNull, also added [CanBeNull] to the response, but the generated TS is always getFoo(): Observable<Foo>. Or will this not be supported until this issue/RicoSuter/NJsonSchema#1069 is addressed?

Thanks in advance, just didn't want to keep trying things if it just wasn't possible at the moment.

The issue https://github.com/RicoSuter/NJsonSchema/issues/1069 is for generating C# code with NRT annotations - what you are looking for is reading NRT annotations when generating the spec and then generate the TypeScript. Is the spec correct? Maybe this screenshot helps:

image

Hi Rico, many thanks for your quick response. Apologies it took so long to respond, it just took me some time to update everything (made sure I was on the latest version of C#, core, using OpenApi, and as close to the screenshot as I thought was needed). I've been using the command line nswag to generate everything but I also added the package to the project and setup the service like you showed just in case.

The good news is I was able to figure out how to get a nullable response. The only shortcoming is I was not able to do it solely with an NRT. The only way I was able to get the response to be nullable was to annotate it with [return: CanBeNull], that would then set the response as "nullable": true in the generated spec. Without that attribute the "nullable" was omitted completely. I also made sure to be explicit about the default handling of reference and response types to NotNull in both the service and in the command line.

I can also confirm that it does work as expected with NRT properties, it's just the response types I can't seem to get it to work unless I annotate with the [return: CanBeNull]. Is there anything you think I could be missing?

Thanks again for the quick response and that screenshot, was super helpful.

Edit: versions I'm using:
npm nswag package: 13.2.2
netcore: 3.1
NJsonSchema: 10.1.4

Maybe you net to set this to Null
https://github.com/RicoSuter/NSwag/blob/master/src/NSwag.Generation/OpenApiDocumentGeneratorSettings.cs#L28

Maybe it makes sense to create a wiki page with NRT config info

Maybe you net to set this to Null
https://github.com/RicoSuter/NSwag/blob/master/src/NSwag.Generation/OpenApiDocumentGeneratorSettings.cs#L28
And if NRT says not nullable this setting is ignored.

Maybe it makes sense to create a wiki page with NRT config info. I currently did not update my projects so I dont know yet the optimal settings for NRT. Maybe we also need to change the defaults and/or add convenience methods to the settings class to easily enable the best settings.

I just tried with setting the DefaultResponseReferenceTypeNullHandling to Null on both the command line and in the service, but the generated spec still didn't specify "nullable": true on the response NRT.

The good thing is that there's still a way to get the nullable in the generated spec via the [return: CanBeNull], but yea a config/settings to make it work without that would be awesome.

Thanks again for the responses and awesome lib Rico.

Can you post the full signature of the operation method? Maybe i find time to look into this. Maybe its a bug how responses are processed (ie NRT is ignored or something)

Yep, the return type is coming from apiResponse.Type

https://github.com/RicoSuter/NSwag/blob/85ae862fd6d68173a201a79e0ad06e0be2ec5de1/src/NSwag.Generation.AspNetCore/Processors/OperationResponseProcessor.cs#L83

Which cannot carry NRT information (it鈥檚 a regular Type without context). So it cannot work at the moment I think. It must be directly loaded from the method object... need to check if this is possible somehow.

Thanks Rico, here's a strip down of what I was using to test in case it helps:

namespace Test {

    public class Foo { }

    [Route("api/[controller]")]
    public class FooController {

        public FooController() { }

        // this one will generate '"nullable": true'
        [return: CanBeNull]
        [HttpGet("{id}")]
        public Foo? Get(long id) => null;

        // this one does not generate '"nullable": true'
        [HttpGet("alt/{id}")]
        public Foo? GetAlt(long id) => null;
    }
}

Hi! Is there a way to make the async result null-able?

Something like:

// This C#...
public async Task<Foo?> GetFoo() { await Bar(); return null; }

// ...would generate this TypeScript.
export async function getFoo(): Promise<Foo | null> { ... }

I have tried [return: CanBeNull] but to no avail.

The schema PContactInfo itself is not (always) nullable but the property techContactInfo is, there might be properties with the same schema but which are not nullable. $ref must be alone in an object (no other props are allowed) that鈥檚 why it is wrapped in a oneOf

@RicoSuter
This behaviour is slightly confusing for the Redoc UI. In case when there is only one possible ref object, which might be nullable. The nullable flag already points out that the ref object might be nullable, and the oneOf brings more ambiguity.

Non-nullable string arguments (for actions) are being generated as nullable, even or typescript generator

e.g.

msbuild

    <Nullable>enable</Nullable>
ActionResult SomeAction([FromQuery, BindRequired]string code)

generates

            "type": "string",
            "name": "code",
            "in": "query",
            "required": true,
            "x-nullable": true

Is it generated as nullable?

I think the spec looks correct, did you try other typescript nullability settings?

@RicoSuter this is about generating the correct openapi json, not about generating typescript; "aspnetcore2openapi" sets "x-nullable" to true even though the "string code" is not nullable

Looks like a bug and needs more investigation - maybe the parameter context is not passed correctly or nullability information is missing somehow.

CSharp 8 nullable reference types code generation has landed in 13.6.0 with #2912 and is being improved by #2959
It is hidden behind the opt-in "generateNullableReferenceTypes": true, option.

Closed as implemented and released, please create new issues if there are bugs/problems with the current implementation.

Was this page helpful?
0 / 5 - 0 ratings