Review impact of nullable reference types on existing rules

[sharwell]

https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-8#nullable-reference-types

[sharwell]

This feature brought in the following new pieces of syntax that need to be accounted for:

• Allowing a ? after types. I suspect the only place that changed is array types, since pointer types cannot be marked nullable and generic types already supported it in the past (e.g. ArraySegment<int>?).
• Allowing class? as a generic constraint
• Allowing notnull as a generic constraint
• Allowing a ! after expressions. This one will be more complicated, but overall I would expect the general rule to be:
  • There should not be any whitespace before the !
  • All other rules should operate as though the ! did not appear in code at all

[alistairjevans]

Regarding the '!' after expressions, might it be worth adding an optional rule to require a comment on the line before a forced statement, to require explanation?

I sort of consider the use of forcing the nullability to need an explanation when you do so, for example:

        public void Include(IRegistrationSource source)
        {
            if (IsInitialized)
            {
                BeginInitialization(new[] { source });
            }
            else if (IsInitializing)
            {
                // _sourcesToQuery can only be non-null here due to the initialization flow.
                _sourcesToQuery!.Enqueue(source);
            }
        }

Not sure it fits every use-case, but thought I'd mention it for discussion.

[sharwell]

@alistairjevans right now I'm not certain that rule would be good. I think we need a bit more time using these features so we can hone in on long-term best practices. Please feel free to create a new issue proposing the rule. With more time it may become favorable.

[alistairjevans]

@sharwell, That's entirely fair.

As I've been going through it, there's actually a number of open roslyn issues under discussion (e.g. dotnet/roslyn#39291) around flow analysis of constructors that may greatly reduce the need for the ! anyway, which may make the requirement to flag the edge case use of the ! more helpful.

[sharwell]

The one I'm eagerly awaiting is dotnet/roslyn#25372

[alistairjevans]

Eeek, yeah, that fix seems extremely necessary.

Can we add something that indicates a property/field is initialized through reflection and cannot be null? (avoiding CS8618)

[sharwell]

@Pedramrezaei That would be a language feature to post in dotnet/csharplang.