microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
3.01k stars 210 forks source link

CSharp - Path Parameters should keep their name and descriptions #2978

Closed pillowfication closed 1 year ago

pillowfication commented 1 year ago

Using the OpenAPI 3.0.0 petstore sample

kiota generate -d "https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/examples/v3.0/petstore.json" -l CSharp

The part of interest is

"paths": {
  "/pets/{petId}": {
    "get": {
      "parameters": [
        {
          "name": "petId",
          "in": "path",
          "required": true,
          "description": "The id of the pet to retrieve",
          "schema": {
            "type": "string"
          }
        }
      ]
    }
  }
}

Current Behavior

The following block of code is created for the indexer

/// <summary>Gets an item from the ApiSdk.pets.item collection</summary>
public WithPetItemRequestBuilder this[string position] { get {
    var urlTplParams = new Dictionary<string, object>(PathParameters);
    if (!string.IsNullOrWhiteSpace(position)) urlTplParams.Add("petId", position);
    return new WithPetItemRequestBuilder(urlTplParams, RequestAdapter);
} }

current behavior

Expected Behavior

I would like for the position variable to be named the same as the path parameter petId. I would also like for path parameter's description to be kept and placed in a corresponding <param/> documentation.

/// <summary>Gets an item from the ApiSdk.pets.item collection</summary>
/// <param name="petId">The id of the pet to retrieve</param>
public WithPetItemRequestBuilder this[string petId] { get {
    var urlTplParams = new Dictionary<string, object>(PathParameters);
    if (!string.IsNullOrWhiteSpace(petId)) urlTplParams.Add("petId", petId);
    return new WithPetItemRequestBuilder(urlTplParams, RequestAdapter);
} }

expected behavior

baywet commented 1 year ago

Thanks for your interest in kiota and for reporting this.

This is something that could be easily fixed by adding a ParameterDescription property on the code indexer DOM element and populating that property.

https://github.com/microsoft/kiota/blob/9f3c55fca68a0ec94abab7edcde0cc45463a9c67/src/Kiota.Builder/KiotaBuilder.cs#L973

https://github.com/microsoft/kiota/blob/9f3c55fca68a0ec94abab7edcde0cc45463a9c67/src/Kiota.Builder/CodeDOM/CodeIndexer.cs#L4

Then we need to make sure we carry the information when we convert the indexer to a method in other languages

https://github.com/microsoft/kiota/blob/9f3c55fca68a0ec94abab7edcde0cc45463a9c67/src/Kiota.Builder/CodeDOM/CodeMethod.cs#L83

And lastly we need to make sure we write the information in dotnet (the other languages will rely on the code parameter information already)

https://github.com/microsoft/kiota/blob/9f3c55fca68a0ec94abab7edcde0cc45463a9c67/src/Kiota.Builder/Writers/CSharp/CodeIndexerWriter.cs#L15

Is this something you'd be willing to submit a pull request for?

sebastienlevert commented 1 year ago

@baywet Please review based on the 1.5 release improvements. Thanks!

baywet commented 1 year ago

The 1.5 improvements didn't impact this issue much: We now support type specific indexer parameters (not just string anymore, but also int etc...). #2594 We can't change the parameter name, it'd be a breaking change even though it's unlikely anybody has a direct reference to it. I created #3070 to follow up on this during v2. We should still carry on the description of the parameter for a better developer experience. I'll work on that for the next release.