Consider enabling strict roslyn setting

[aspnet-hello]

From @pakrym on Sunday, July 29, 2018 7:59:29 PM

See: dotnet/corefx#31457

cc @natemcmaster @davidfowl @Eilon

Copied from original issue: aspnet/Universe#1285

[aspnet-hello]

From @Eilon on Sunday, July 29, 2018 9:41:27 PM

@pakrym - is the main place of value in Kestrel? If so, can you go ahead and run with strict temporarily just to see if there are any issues? (We'd need to fix the issues anyway if/when we enable this globally.)

[aspnet-hello]

From @davidfowl on Monday, July 30, 2018 9:04:07 AM

We use structs all over, there’s probably code comparing a StringValues to null elsewhere.

[aspnet-hello]

From @Eilon on Monday, July 30, 2018 11:41:24 AM

OK. We can still preempt a lot of the fallout by fixing up areas where we know it'll be a disaster.

[natemcmaster]

Triage team: let's give this a try. If there are a crazy amount of compiler errors, let's chat first before you spend too much time correcting all of them.

[divega]

@natemcmaster @pakrym is there a summary somewhere of the benefits of C# strict mode? Is this something we should consider enabling for the EF Core codebase as well?

cc @bricelam @ajcvickers

[pakrym]

It enables Roslyn diagnostics that would be a breaking change to enable by default. I wasn't able to find the exact list of diagnostics but the most useful from my experience is struct to a null comparison check.

Is this something we should consider enabling for the EF Core codebase as well?

Yes, corefx, roslyn has had it enabled for some time and it caught a couple minor bugs in AspNetCore.

You can get a rough idea which diagnostics are enabled by looking through roslyns source: https://github.com/dotnet/roslyn/search?l=C%23&q=FeatureStrictEnabled&type=

[divega]

Thanks @pakrym.

[pakrym]

Done.