C# nullability

[NN---]

Steps:

1. Add ? and ! where appropriate
2. Add framework attributes in addition to JetBrains Annotations. We must not remove the annotations for older frameworks and VS support.

[NN---]

Thanks. The PR is not ready yet. There is some work to do with annotating precisely code to reduce amount of warnings. IMO if we are going with nullable , the best way is to set all nullable warnings as errors.

It is correct that attributes are not dependent on framework. I added for the old frameworks in the library itself. Need to check whether it works the same way as JB. I do think we must keep both since there could be enough people with ReSharper 2019 without good nullable support. ReSharper got good support for C#nullable only starting 2020.2 and still it doesn't work 100% as expected .

[ig-sinicyn]

I do think we must keep both since there could be enough people with ReSharper 2019 without good nullable support.

Nice argument and I'm going to strongly agree with you.

Can I help in the warnings battle? We can split the work, if you wish.

[NN---]

Sure, you can help 👍 First I am going to make tests pass and then will go back to nullable.

You can commit all your suggestions directly to the branch. Just make small fixup commits and regular pull so we don't get out of sync.

[ig-sinicyn]

I've added some missing nullable annotations.

I'm going to start fixing nullability warnings on evenings next week. It looks like a pretty good amount of work so it'll take some time.

[NN---]

What do you say about errors for all nullable warnings ? I do this in my project and this is good but sometimes annoying :)

[ig-sinicyn]

I'd fix all warnings at first. Current analysers (both c# and ReSharper ones) may produce false positives sometimes.

As example,

class Helpers<T> 
{
   public void DefaultArg([MaybeNull] T arg) {}
   public void Call() => DefaultArg(default); // < warning here
}

[NN---]

I use convention. JBNotNull and FWNotNull. Do you think a better convention is to use fully qualified name instead ?

[ig-sinicyn]

I use convention. JBNotNull and FWNotNull.

I'm fine with it and I've used it in my commits. The only thing, I've renamed FWNotNull to FwNotNull, as by .net naming guidelines. We always can do find-replace if there will be any objections:)

[NN---]

MS doesn’t follow own convention which says 2 letters is okay for acronym. So you have System.IO and IOException and DBNull but then they introduce ClientId:)

My point is whatever looks better we shall use, the convention contradicts itself with too many use cases.

[NN---]

My opinion that JB looks better than Jb , same for FW. Also maybe one letter can be good enough for us?

[ig-sinicyn]

My opinion that JB looks better than Jb , same for FW.

Okay, let's use FW then. I'm not going to commit anything until tomorrow so feel free to rename it back:)

[andrewvk]

@NN--- , have you seen branch /feature/nullable_v3?

[NN---]

Yes. I saw. I will take a closer look later when I have time. There are several changes which are not good like removing JetBrains attributes. They must be preserved.

Do you suggest to take your branch as base one ?

[andrewvk]

I think that supporting old R# attributes is a double job, and I don't think that there are many people forced to use a Resharper versions from years ago. As for the branch, there are 620 warnings in the old one and 3700 warnings in the current, so with the first work should be done less.

[NN---]

Not everyone is updated to the latest VS2019 16.8 and there are developers even not using VS2017. I don't see a big problem supporting both. Happily in current job we update every release and can easily use newer .NET Core version, but in the previous one, for instance, we couldn't afford that.

[andrewvk]

NRT support does not require a VS 16.8, as NRT has been added in C# 8.0/VS 16.3, not 9.0. And NRT works well even with FW 2.0 target (with Theraot, of course, to add System.Diagnostic.Annotation attributes). The only reason to use old attributes is to support old versions of R#/Rider that do not support C# 8.

[NN---]

There were issues with ReSharper even 2020.1 with NRT. Only the upcoming 2020.3 has solved issues I have discovered so far.