Add support for Nullable build setting

[jcouv]

This is a tracking work item related to the "nullable reference types" feature in C# 8.0 (for dev16). We'll add design notes here as it crystallizes (is it a property or some other form of API?) Tagging @davkean

[jcouv]

The design for turning on the nullable feature changed since our last discussion. Previously, we would have [NonNullTypes] (for class- or method-level control) or [module:NonNullTypes] (for module/project-level). Now, we'll have #nullable ... for scoped control in source, and a compiler flag for project-level control. That compiler flag is exposed via MSBuild property NullableReferenceTypes, as part of dev16 preview1.

Based on this, there are two scenarios of interest:

1. exposing a checkbox in Build options ("Enable Nullable Reference Types")
2. allowing a codefixer to change this project property (like UpgradeProject does for LangVersion)

@davkean I assume that those scenarios translate into three work items:

1. some common work in project-system (object model or API change)
2. IDE work to add Build option UI (or is this also owned by project-system)
3. codefixer work (IDE/myself)

@davkean @Pilchie Do you think the API work (1) could be reasonably scheduled as part of preview 2? If yes, I would do the codefixer (3) in preview 2 as well.

[jcouv]

Tagging @tmeschter per email discussion about preview2 goals. Thanks

[etbyrd]

I could get a PR out for the checkbox in the Build property page - do we have an idea of where on the page we would want it?

[jcouv]

For adding the checkbox to the Build property page, I think putting it under "Check for arithmetic overflow/underflow" would make sense. @davkean Does that sounds ok?

[tmeschter]

I wouldn't want to add the checkbox without adding the underlying property support to csproj/msvbprj, and that's the expensive part.

[chucker]

In my testing, it looks like the <NullableReferenceTypes> property only works with new-style projects. Are there plans to make it work with the old project system (e.g., for an ASP.NET project which can't be migrated due to #2670)?

[jcouv]

@chucker Yes, this property will be made to work in old project system too, before C# 8 ships as RTM, but likely after dev16.0 (where C# 8.0 language features are in beta). The present issue tracks this remaining work.

[jcouv]

@davkean We had to modify the options to add a third one (true/false were not enough) and changed the name of the option in the process. I assume that the change of name affects project system, but the change of values doesn't. Let me know if that's not the case, as one of the options might still change. Sorry for the randomization.

Through msbuild the context could be set by supplying an argument for a "NullableContextOptions" parameter of Csc build task. Accepted values are "enable", "disable", "safeonly", or null (for the default nullable context according to the compiler). The Microsoft.CSharp.Core.targets passes value of msbuild property named "NullableContextOptions" for that parameter.

https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-reference-types.md#setting-project-level-nullable-context

[Tragetaschen]

Given how I spent the last thirty minutes, it might be worth going through the existing blogs and docs for C# 8 and modify the samples on how to set up that feature. That property name change and the new option names took quite some time to figure out.

Some examples are https://docs.microsoft.com/en-us/dotnet/csharp/tutorials/nullable-reference-types or https://blogs.msdn.microsoft.com/dotnet/2018/12/05/take-c-8-0-for-a-spin/

[jjmew]

This issue is tracked internally by https://dev.azure.com/devdiv/DevDiv/_workitems/edit/823504

[jcouv]

From LDM discussion today (5/1/2019) including Phillip, we concluded that "Nullable" would be a better name than either "NullableReferenceTypes" or "NullableContextOptions". It should remain a string (to accept "enable", "disable" and a couple of other values).

Relates to compiler work item https://github.com/dotnet/roslyn/issues/35432

[jnm2]

Moved bug report to separate issue: https://github.com/dotnet/project-system/issues/5551

[jcouv]

@drewnoakes @chucker @tmeschter @davkean I noticed this old issue. I assume that we still want to expose this checkbox. What release should we aim for?

[drewnoakes]

@jcouv I took a look at this but hit a block that would require help from @davkean or someone else who knows the details of the property pages better than me.

In short, adding the UI was straightforward, however we only want to enable that UI when LangVersion is 8 or greater. Sniffing the current version using the APIs available in the property pages (that must work with both legacy and CPS projects) gave me some trouble.

[chucker]

@drewnoakes regarding legacy projects, I take from https://github.com/dotnet/project-system/issues/5551#issuecomment-563249987 that this is no longer planned, so the UI can focus on CPS.

(I'd love to be wrong, though.)

[drewnoakes]

PR #5807 is up to fix this.