RicoSuter / NSwag

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

Client generated code does not compile #3895

Open cabadam opened 2 years ago

cabadam commented 2 years ago

I am attempting to generate a client for the OpenAPI specification you can download from the top of this page: https://api.mattermost.com/

I have tried multiple different ways of generating:

  1. Visual Studio 2022 adding a "Connected Service"
  2. NSwagStudio
  3. Using the NSwag.Msbuild

All more or less produce the same code and the same problem. There is one easy to work-around issue where there is a property called in the json file called "" and Nswag tries to generate a C# property of the same name. Naturally that fails. Easy enough to change the json to remove the <> characters.

However, after that I still get a variety of issues. Some methods of generating will put this line just inside the namespace: using System = global::System;

When compiling, that gives many of:

Namespace 'MattermostTest' contains a definition conflicting with alias 'System'

If I comment out that line in the generated code, then the rest of the file throws compilation errors as if it has no idea what exists under "System." For example, the first class gets a GeneratedCode attribute (typename fully qualified): [System.CodeDom.Compiler.GeneratedCode("NSwag", "13.15.9.0 (NJsonSchema v10.6.8.0 (Newtonsoft.Json v13.0.0.0))")]

That gives compilation error:

error CS0426: The type name 'CodeDom' does not exist in the type 'System'

Dumping the json file into editor.swagger.io does give a few errors. Stripping out the relevant pieces of the spec however yields no change in the errors.

I've tried it with multiple different versions of .NET (from .NET Framework 4.8, .NET Core 3.1, and .NET 6 which was my intended target). I also had a different person try generating the client code, no luck.

What are we missing?

Thanks

ovska commented 2 years ago

I looked into it and there seem to be multiple things going wrong:

Manually fixing the client takes quite a bit of work, but I can't see a way around all the issues otherwise (for now).

cabadam commented 2 years ago

The definition has a type called System, which causes the compilation problems you're encountering (note the "does not exist in the type 'System'"). There isn't a way to rename a generated type that I know of, so hard to say what to do about this one.

Interesting catch... I wonder if nswag caught it and that is what the "using System = global::System;" line was trying to do, re-point "System" to be the namespace, not the type?

lahma commented 2 years ago

using System = global::System; is part of the default template. I guess one could create a workaround by creating a custom IClientGenerator:

https://github.com/RicoSuter/NSwag/blob/5cfa197cbb6442f989a1e7be7135c8e895218363/src/NSwag.CodeGeneration/IClientGenerator.cs#L13-L26

The rabbit hole goes deeper if you check the default implementation and how it delegates forward. Just an idea.

AndersHogqvist commented 8 months ago

I had the exact same issue when trying out the SpaceTraders API. I managed to solve it by changing using System = global::System; to using gSystem = global::System; and then manually search & replace all references to to global System with gSystem

Pilchard123 commented 8 months ago

@AndersHogqvist And I just had exactly the same problem with exactly the same API. I don't have the slightest idea how hard this would be to do, but I wonder if it would be possible to detect the use of "System" as a generated type name and do something similar to what you did manually.

nycdotnet commented 6 months ago

Also attempting to get this working with SpaceTraders. I dug into this a bit.

In src\NSwag.CodeGeneration.CSharp\CSharpGeneratorBaseSettings.cs I added a new string property GlobalSystemNamespaceAlias defaulting to "System".

I also had to add a new getter to CSharpFileTemplateModel.cs:

public string GlobalSystemNamespaceAlias => _settings.GlobalSystemNamespaceAlias;

Then I changed line 19 of File.liquid to this:

 using {{ GlobalSystemNamespaceAlias }} = global::System;

With this change, I was able to create a new test in src\NSwag.CodeGeneration.CSharp.Tests\FileTests.cs and set the new property on the CSharpGeneratorSettings object and the generated code had the newly provided value. So I'm somewhat convinced this is possible.

I imagine there will be a LOT of other changes needed to make this work in real life. I wonder if @RicoSuter would be willing to take a change that big. There are 166 uses of System in .liquid files and there may be more in code.

TLDR; right now if your OpenAPI contains a model named "System" (such as SpaceTraders.io where star systems are modeled with the entity name "System"), you can't easily use NSwag for it because the "System" model conflicts with the "System" .NET namespace alias.

nycdotnet commented 6 months ago

FYI - this change I made on my fork still has all tests pass. It's not usable yet I don't think from outside - I haven't yet learned how to integrate the settings - but it does allow renaming the namespace for the main file only.

https://github.com/nycdotnet/NSwag/commit/eba3a55c379dd8bf1a9bc3bd25e4fc1f225702a8

nycdotnet commented 5 months ago

FYI I am making progress on my local repository for this and I have the command-line app (which is what the MSBuild NuGet uses) working to generate code where the System namespace is renamed if an alias you provide on the command line is provided. It's not complete yet but it's getting there. One thing i know my be a challenge is how to reconcile things like selecting System.Text.Json if you're renaming System - it will need to respect it consistently and that's a bit tricky to make work consistently.