Turn on nullability for all src projects

[pranavkm]

No description provided.

[JamesNK]

Is the plan something like:

1. Default to globally on
2. Fix the remaining projects to remove their disables
3. Remove the <nullable>enables</nullable> everywhere

I think there are a couple of projects that already have <nullable>disable</nullable>. Perhaps add a comment with the new disables to they're easy to tell apart.

[pranavkm]

Yup, that's the plan.

I think there are a couple of projects that already have disable. Perhaps add a comment with the new disables to they're easy to tell apart.

From the looks of it, they have comments and the ones that don't should be updated to have nullability enabled: https://github.com/dotnet/aspnetcore/search?l=XML&q=nullable%3Edisable

[JamesNK]

Add comments to the existing nullable disables in Microsoft.AspNetCore.Authentication.AzureAD.UI.csproj and Microsoft.AspNetCore.Authentication.AzureADB2C.UI.csproj that these projects are obsolete.

[dougbu]

This rhymes with

Suggested change

	<Nullable Condition="'$(Nullable)' == '' AND !('$(IsTestProject)' == 'true' OR
	'$(IsTestAssetProject)' == 'true' OR
	'$(ISBenchmarkProject)' == 'true' OR
	'$(IsSampleProject)' == 'true' OR
	'$(IsMicrobenchmarksProject)' == 'true')">enable</Nullable>
	<Nullable Condition=" '$(Nullable)' == '' AND (
	'$(IsImplementationProject)' == 'true' OR
	'$(IsAnalyzersProject)' == true OR
	'$(IsSpecificationTestProject)' == 'true') ">enable</Nullable>

If that's what you meant, the suggestion is easier to read. Even better if analyzers and / or specification tests don't need nullability enabled.

[dougbu]

Separately, may be good to add bits like AND ! $(RepoRelativeProjectDir.Contains('Tools') rather than enshrine exclusions we'll never remove elsewhere in the repo.

[pranavkm]

I didn't quite follow the second suggestion. Is this to exclude the eng tools?

[dougbu]

The second suggestion would remove the need for src/Tools/Directory.Build.props because $(RepoRelativeProjectDir) is the relative path from the repo root to the project folder.

/fyi src/ProjectTemplates/testassets/DotNetToolsInstaller is the only not-exactly-Tools folder the Contains(...) would match. I suspect a repetition w/ .Contains("tools") would be needed if you want to automatically exclude projects under src/Middleware/tools and the other 10 or so other tools folders.

[pranavkm]

I really want to get to the tools too (it validates the nullability of shared sources for e.g). Just not making it a priority for 6.0. Having it be an explicit nullable disabled makes it easier to find these exclusions. Let me know if you feel strongly about it.

[captainsafia]

Are we tracking the disabled ones in an issue somewhere?

[JamesNK]

#5680