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

feature: Improve support for nullable #1302

Closed
dannyBies opened this issue Feb 3, 2022 · 8 comments
Closed

feature: Improve support for nullable #1302

dannyBies opened this issue Feb 3, 2022 · 8 comments

Comments

@dannyBies
Copy link
Contributor

Is your feature request related to a problem? Please describe.

In my project I have nullable reference types enabled <Nullable>enable</Nullable>.

I retrieve data like this:

var response = await RestApi.Users.Query.GetUrl("1234");
if (!response.IsSuccessStatusCode)
{
    Logger.LogError(response.Error.Message);
    return;
}
var url = response.Content.Url;

The issue I'm seeing is that I get CS8602 warnings(Dereference of a possible null reference) for response.Content and response.Error due to these being nullable. However I am checking response.IsSuccessStatusCode which I believe make sure that either Content or Error is populated.

Describe the solution you'd like

To not get these warnings.

Describe suggestions on how to achieve the feature

I believe we can add the following attributes to IsSuccessStatusCode to fix this issue.
[MemberNotNullWhen(true, nameof(Content))]
[MemberNotNullWhen(false, nameof(Error))]

@clairernovotny
Copy link
Member

Can you please share a sample interface definition that matches what you're doing? It's not clear what the type of response is here.

@dannyBies
Copy link
Contributor Author

This happens for every call that uses ApiResponse

public T? Content { get; }

An example could be:

[Get("/api/xyz")]
Task<ApiResponse<UrlResponse> GetUrl(CancellationToken cancellationToken = default);

public record UrlResponse(string Url);

@clairernovotny
Copy link
Member

Can you please submit a PR to add the correct attributes?

@dannyBies
Copy link
Contributor Author

I've submitted a PR, thanks!

@clairernovotny
Copy link
Member

Are there other places in the public API that we need to decorate with the nullable attributes to be more accurate?

@dannyBies
Copy link
Contributor Author

I've looked through ApiResponse and ApiException and have added one more property to the list. I haven't looked through the rest of the API as I'm not too familiar (yet!) with the project and I unfortunately don't have the time right now to look through everything.

If you can point me to the exact properties that could also use this change I'd be happy to make another commit.

@clairernovotny
Copy link
Member

Thanks!

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants