Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.5.0 generates broken CLI request builder #3088

Closed
jasonjoh opened this issue Aug 8, 2023 · 5 comments · Fixed by #3091
Closed

1.5.0 generates broken CLI request builder #3088

jasonjoh opened this issue Aug 8, 2023 · 5 comments · Fixed by #3091
Assignees
Labels
CLI Work to support generating CLIs with Kiota type:bug A broken experience WIP
Milestone

Comments

@jasonjoh
Copy link
Member

jasonjoh commented Aug 8, 2023

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 baywet self-assigned this Aug 8, 2023
@baywet baywet added type:bug A broken experience CLI Work to support generating CLIs with Kiota labels Aug 8, 2023
@baywet baywet added this to Kiota Aug 8, 2023
@baywet baywet added this to the Kiota v1.6 milestone Aug 8, 2023
@github-project-automation github-project-automation bot moved this to Todo in Kiota Aug 8, 2023
@baywet
Copy link
Member

baywet commented Aug 8, 2023

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
Copy link
Member

baywet commented Aug 8, 2023

put together #3091

@calebkiage
Copy link
Contributor

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
Copy link
Contributor

Are these indexers duplicates?

@baywet baywet moved this from Todo to In Progress in Kiota Aug 8, 2023
@baywet
Copy link
Member

baywet commented Aug 8, 2023

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.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Kiota Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Work to support generating CLIs with Kiota type:bug A broken experience WIP
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants