microsoft / kiota

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

1.5.0 generates broken CLI request builder #3088

Closed jasonjoh closed 1 year ago

jasonjoh commented 1 year ago

In 1.5, kiota generates a request builder for the quickstart that will not compile.

\kiota-samples\get-started\quickstart\cli\src\Client\Posts\PostsRequestBuilder.cs(24,63): error CS0161: 'Posts
RequestBuilder.this[string].get': not all code paths return a value [C:\Source\Repos\kiota-samples\get-started\quickstart\cli
\src\KiotaPostsCLI.csproj]

Offending code:

[Obsolete("This indexer is deprecated and will be removed in the next major version. Use the one with the typed parameter instead.")]
public PostItemRequestBuilder this[string position] { get {
    var urlTplParams = new Dictionary<string, object>(PathParameters);
    if (!string.IsNullOrWhiteSpace(position)) urlTplParams.Add("post%2Did", position);
} }

This method wasn't present at all in the 1.4 generated client, and I verified that removing it fixes the issue (and the client still works).

baywet commented 1 year ago

This seems to be a side effect of #3058. We should probably add a refiner method for CLI that removes those backward compatible indexers. @calebkiage do you need indexers at all in the CLI generation?

baywet commented 1 year ago

put together #3091

calebkiage commented 1 year ago

Hey @bawet, I do use indexers but in a different way than C#. I create a list of commands whenever I encounter an indexer. See https://github.com/microsoft/kiota/blob/main/src/Kiota.Builder/Writers/CLI/CliCodeMethodWriter.cs#L65

calebkiage commented 1 year ago

Are these indexers duplicates?

baywet commented 1 year ago

Thanks for the additional context. Yes we now have 2 indexers in some cases to avoid breaking binary compatibility in CSharp. One with the specific type, and another still with string and tagged as obsolete.