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

Enable Nullable Reference Types feature #22

Closed
m-wild opened this issue Oct 2, 2020 · 7 comments
Closed

Enable Nullable Reference Types feature #22

m-wild opened this issue Oct 2, 2020 · 7 comments
Assignees
Labels
💡 enhancement New feature or request

Comments

@m-wild
Copy link
Collaborator

m-wild commented Oct 2, 2020

We should use the C# 8.0 feature Nullable Reference Types to prevent Null Reference Exceptions.

https://docs.microsoft.com/en-us/dotnet/csharp/nullable-references

@m-wild m-wild added 💡 enhancement New feature or request help wanted labels Oct 2, 2020
@domca25101
Copy link

Hi, I'd like to dig into this issue. Could you give me a bit of guidance and possibly assign this to me?

@m-wild
Copy link
Collaborator Author

m-wild commented Oct 3, 2020

My advice for this one would be to

  • first enable nullable reference types <Nullable>enable</Nullable> in the project file.
  • set the language version to latest <LangVersion>latest</LangVersion>
  • I would then begin looking through the AsyncAPI specification (https://www.asyncapi.com/docs/specifications/2.0.0) at each type and for any property which is OPTIONAL add the ? to allow it to be null.
  • For non-optional properties, I would add a constructor which checks for null.

e.g. If we look at the Info object
https://www.asyncapi.com/docs/specifications/2.0.0#infoObject
Title and Version are required. All others can be null.

public class Info
{
    public Info(string title, string version)
    {
        Title = title ?? throw new ArgumentNullException(nameof(title));
        Version = version ?? throw new ArgumentNullException(nameof(version));
    }

    [JsonPropertyName("title")]
    public string Title { get; }

    [JsonPropertyName("version")]
    public string Version { get; }

    [JsonPropertyName("description")]
    public string? Description { get; set; }

    [JsonPropertyName("termsOfService")]
    public string? TermsOfService { get; set; }

    [JsonPropertyName("contact")]
    public Contact? Contact { get; set; }

    [JsonPropertyName("license")]
    public License? License { get; set; }
}

@dgnaegi
Copy link

dgnaegi commented Oct 10, 2020

Hi @domca25101 , are you still working on this one? I would be interested as well

@domca25101
Copy link

Hi @dgnaegi, yes I am working on it.

@domca25101
Copy link

Hi, I have encountered a couple issues that I need a bit of help with:

  1. As you described in the comment above, I need to add required properties to the constructor, which checks for nulls. In case of AsyncApi object, however, Info property is required, but adding it as a required parameter of constructor necessitates changes to all its instantiations and I'm having trouble figuring some of them out.

  2. There are some cases, where presence of null check is not necessary from context, but if null check is omitted, there will be warning asking for one. Should I use arbitrary null check in this case or just suppress the warning?

@m-wild
Copy link
Collaborator Author

m-wild commented Nov 27, 2020

For (2), I would add the check.

For (1), I will review the code when I am back from holiday and reply.

@m-wild
Copy link
Collaborator Author

m-wild commented Jul 19, 2021

Closing as not needed for now.

@m-wild m-wild closed this as completed Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants