[C#] API model name conflicts with namespace

[peterwurzinger]

When generating a C# client for an API, where a model has the same name as a part of its namespace , the compiler would raise CS0118 ("'XY is a namespace but is used like a type'") on various locations in the generated code.
E.g.
Response model "Subscription" generated as part of an API client in namespace "Whatever.Application.Subscription.Client"

As a workaround one can use the fully qualified type name to correct the generated code. That has the drawback that it would get overwritten permanently when the clients are being regenerated though.

Kiota Version 1.13.0

[andrueastman]

Thanks for raising this @peterwurzinger

The generator does make attempts at type disambiguation in various scenarios to use the fully qualified name of the model/type. It may be that this is not a scenario captured by the current logic here. And we need to capture for a case where the model name matches the namespace name.

kiota/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs

Line 205 in 5314d63

if (currentType.TypeDefinition != null &&

Any chance you'd be willing to share an openApi description to help understand better the scenario that is not captured?

[peterwurzinger]

Hey there @andrueastman, thanks for investigating

Sure, absolutely. I created a repository at https://github.com/peterwurzinger/KiotaRepro that should contain a minimal reproduction of that issue.

I used the PayPal subscriptions API available at https://github.com/paypal/paypal-rest-api-specifications/blob/main/openapi/billing_subscriptions_v1.json, and since it's a public API I guess they wouldn't be too pissed about being used in a bug repro :)

The Kiota command used for creating the client was

dotnet kiota generate -l CSharp -c PayPalSubscriptionsClient -n KiotaRepro.Subscriptions.PayPal.Client -d https://raw.githubusercontent.com/paypal/paypal-rest-api-specifications/main/openapi/billing_subscriptions_v1.json -o ./Client

[baywet]

I can get the repro to work by applying the following change here.
https://github.com/peterwurzinger/KiotaRepro/blob/97e5fa3bf7ea7af6bbde9058e7fc144ce6196615/SubscriptionsClient.cs#L1

-namespace KiotaRepro.Subscription.PayPal;
+namespace KiotaRepro.Subscriptions.PayPal;

I believe it's only a matter of misalignment between how the dotnet project was initialized and the namespace argument passed to kiota. Can you confirm please?

[peterwurzinger]

Hey @baywet
Hm, yeah I see, my example probably wasn't good, using just one namespace oversimplifies the problem. I just pushed some changes to reflect a real world scenario that I faced last week, maybe now it's clearer how that could happen.

Obviously renaming application namespaces to not contain names of the generated client models would resolve the problem, but that's not really something I would consider doing in an application larger than the one shown in the repro.

[baywet]

Thanks for the additional information.
With this update, if I remove this block I'm able to compile the application properly.
While I understand this is a probable use case, kiota cannot know about this additional namespace during generation as it doesn't "plug into the project" or run any kind of compiler service/language server/etc... to parse the existing code. And this is something we're unlikely to add anytime soon as it'd require tons of integration work with the diverse project configurations, compiler technologies, etc...
Also there are workarounds to that: you could generate the client into a separate library project and refer to that library project in your "main" project.

Let us know if you have further questions/concerns.

[peterwurzinger]

Yes, I'm aware that, just as renaming them, removing entire namespaces from the existing code would eliminate any type disambiguation with those in the generated code. I would argue, that they exist for a specific reason and not just for fun (or a repro), and in the best of cases people spend quite some time in naming them properly to reduce cognitive load when working within complex application ecosystems.

In my naive view of how Kiota generates C# clients, those specific problems could be avoided by using the FQN when referencing generated types. So if Kiota e.g. replaced this expression with

-<Subscription?>
+<KiotaRepro.Application.Features.Payment.Providers.PayPal.Client.Subscriptions.Models.Subscription?>

then the application would compile properly as well. (I know that in fact it would not, since there are 4 occurences of this compiler error, but they would be resolved using the same fix.)

The thing is, I obviously don't have any deeper knowledge about how Kiota manages the abstract representation of APIs, or how the C# client builder emits the requested code, but I don't see where using the FQN instead of just the generated types name would require deeper integration work in diverse compiler technologies.

[baywet]

Thanks for articulating this further.
We could imagine changing the code to ALWAYS emit FQNs in CSharp rather that only when collisions with the namespaces (kiota knows about) might occur. The generated code would become uglier, but it'd be full proof. Thoughts?
@andrueastman your opinion as well?

[peterwurzinger]

As far as I know using the FQN everywhere is quite similar to the behavior of NSwag, the current default tooling for generating Connected Services clients in VS. There are some conceptual differences, as everything is being emitted into one file and one namespace, therefore avoiding some collisions in advance, but those clients contain constructs as e.g.

private System.Lazy<Newtonsoft.Json.JsonSerializerSettings> _settings;

or

var headers_ = System.Linq.Enumerable.ToDictionary(response_.Headers, h_ => h_.Key, h_ => h_.Value);

I would agree that the readability is somewhat more limited, yes. But even if I'm just a single reference, I couldn't remember needing to read the code of a generated service client for other purposes than debugging them every now and then.

[baywet]

Sorry, I realized that I never came back to that last answer. The last few weeks have been a bit hectic to say the least.
@andrueastman any opposition to FQDN everywhere? (besides the degraded readability) any better alternative?
@peterwurzinger is this something you'd be willing to submit a pull request for?

[peterwurzinger]

@baywet Sorry, last week was a bit rough.

Yeah, sure, I started looking into it. From what I could see in the codebase, there are some issues that I want to point out:

• There is quite some effort taken to make the code readable during the generation process. Refer to e.g. https://github.com/microsoft/kiota/blob/main/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs So in this field there would be quite a lot of changes.
• De facto primitives like List, Guid, DateTime/Only, CancellationToken,... are inserted during the generation process extensively, so it would need a lot of tiny changes to be made all around the C#/CLR part of the renderers and refiners
• Since Kiota is a tool that generates strings, emitting different ones will very likely break a lot of tests, so those would have to be adapted as well.

So the overall surface of necessary changes would be quite big with only little improvement to the codebase after all, and potentially opening the door for bugs to be introduced within those changes.

Speaking of tests: I think there would be a need for robust regression tests, that ensure that the generated code uses FQN for type references everywhere - even for code that is committed afterwards. I just can't see how these could be implemented with reasonable effort

That a ll being said, in my opinion it would be a good idea to discuss a compromise, that would still resolve the raised issue, but without introducing huge changes. My proposal would be to use the FQN only for types generated by Kiota. That would resolve the issue, leaving edge cases for openApi schema names like JsonSerializationWriterFactory or ApiClientBuilder - but honestly speaking that just does not sound like a realistic scenario to happen.
What are your thoughts on that proposal?

[0xced]

I'm a bit disappointed that #4751 was merged. One thing that I like in kiota is the tidy code which is generated, exactly like if it was written manually (except for the conditional compilation around nullable types, which is why I keep rebasing #3945 onto the main branch).

The type can't use the same name as the namespace is annoying but I always prefer to solve it by tweaking the namespace to avoid collisions.

I just tried to generate a new API client with this change merged and it's becoming pretty ugly. 🙁

[Balkoth]

I was updating my generated client this morning with the latest kiota bits and came to a similar conclusion as @0xced. The code now looks ugly.

At first, i thought there was a bug introduced somewhere and went looking for changes.

Examples taken from Graph client (this change causes nearly all files to change):

[baywet]

Thanks for the feedback. Continuing the discussion on #4796