Experiment: C# 8 and nullable reference types

[bruno-garcia]

This is again a proposal to start a discussion.
While doing this I found a bug on GetHashCode in Trace/Status.cs. Since Description can be null, it would throw.

Would you be interested in adding C# 8 and moving the code towards nullability checks?

If so, I'd be happy to keep going with this change.

Also worth noting that in the process I believe a few questions will arise, for example should we use more Array.Empty<> instead of null for parameters taking IEnumerable or not.

[SergeyKanzhelev]

I see no problems enabling it as long as you plan to move it forward.

[lmolkova]

Please note that some of c# 8 features aren't and won't be available on .net framework and prior to .net core 2.1. I think this is a real blocker for c# 8 usage here.

https://devblogs.microsoft.com/dotnet/building-c-8-0/
https://stackoverflow.com/questions/56651472/does-c-sharp-8-support-the-net-framework

[SergeyKanzhelev]

@lmolkova I understand we will catch unsuopported features during compilation though. No risk for runtime, just inability to use something cool. Correct?

[lmolkova]

@lmolkova I understand we will catch unsuopported features during compilation though. No risk for runtime, just inability to use something cool. Correct?

Yes, but doesn't it defeat the purpose and introduces additional collaboration issues? I mean it's confusing that you cannot use feature even though C# 8 is enabled. Also if we want someone to be able to build OTel in VS2017 we need to add some extra references (see SO post above).

So is there any benefit of doing it?

[bruno-garcia]

Personally I see a benefit in introducing it.
VS 2019 Community, and VS Code can be downloaded and used on this project by anyone, for free so limiting the code-base in this way in case someone wants to contribute but only has VS 2017 is not worth IMHO.

I do agree that it's not ideal that some C# 8 features are not available. But I believe this will be the case to most repos that need to keep compatibility with anything prior to .NET Core 3 and that'll be the vast majority of OSS projects.

Some examples already using nullable reference types are Dapper, MySqlConnector, NodaTime, SimpleInjector, CliFx and CoreFX

The blog post you linkeded from Stuart say:

While Nullable Reference Types as a feature is "not supported" in lower TFMs such as netstandard2.0, it is encouraged to use this feature, and you can do it with little hassle.

If you all agree, I'd be happy to take this task since it doesn't interfere with the spec implementation and can be done 1 project per PR, in isolation.

[bruno-garcia]

Adding @discostu105 and @austinlparker to get more input since this is a big decision.
/cc @z1c0

[bruno-garcia]

@SergeyKanzhelev, @lmolkova, @MikeGoldsmith, @cijothomas

I was hoping we could make a decision on this one.
If we agree to move forward I can start opening PRs, perhaps one per project.
Otherwise we should close this.

Thoughts?

[MikeGoldsmith]

I'm pro adding C# 8 support. There are some unsupported features in .NET Framework but violations will be caught by the compiler so it won't affect development too much. If there was a particular feature we wanted to include for non-Framework implementations, we could always use #if directives.

I also think adding a section to the README regarding VS2017 requiring additional packages should be sufficient. VS2019 and other IDEs (eg VSCode & Rider) automatically add the packages as required.

[SergeyKanzhelev]

@lmolkova do you feel strongly against trying it out? It looks like @bruno-garcia is quite passionate about trying it out, we will have value out of null validations, and we can always revert it back.

[lmolkova]

@lmolkova do you feel strongly against trying it out? It looks like @bruno-garcia is quite passionate about trying it out, we will have value out of null validations, and we can always revert it back.

Sure, let's do this. I don't have a strong opinion. We should just make sure we have all necessary dependencies to make VS 2017 work - it should be easy and worth trying.
@bruno-garcia if you don't have VS 2017 to try it out, please create an issue so we don't forget it.

[bruno-garcia]

Thanks @lmolkova !

WRT VS 2017: I'm currently working on a Mac. I do have a VM but it's borderline useless

I'll get a PR done for one of the projects, and perhaps someone can try it out on VS 2017? Are there really people using it? Otherwise I'll install VS 2017 on an external drive and try to load it from the VM.

[lmolkova]

@bruno-garcia if you are working on Mac, feel free to just create issue to follow up on VS 2017. I don't have any telemetry to prove, but I know quite a few people who still use VS 2017/

[SergeyKanzhelev]

Please re-open when you are ready to pick it back up