{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/csharplang/issues/1939","repository_url":"https://api.github.com/repos/dotnet/csharplang","labels_url":"https://api.github.com/repos/dotnet/csharplang/issues/1939/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/csharplang/issues/1939/comments","events_url":"https://api.github.com/repos/dotnet/csharplang/issues/1939/events","html_url":"https://github.com/dotnet/csharplang/issues/1939","id":371812607,"node_id":"MDU6SXNzdWUzNzE4MTI2MDc=","number":1939,"title":"A design for nullable opt-in","user":{"login":"MadsTorgersen","id":6677272,"node_id":"MDQ6VXNlcjY2NzcyNzI=","avatar_url":"https://avatars2.githubusercontent.com/u/6677272?v=4","gravatar_id":"","url":"https://api.github.com/users/MadsTorgersen","html_url":"https://github.com/MadsTorgersen","followers_url":"https://api.github.com/users/MadsTorgersen/followers","following_url":"https://api.github.com/users/MadsTorgersen/following{/other_user}","gists_url":"https://api.github.com/users/MadsTorgersen/gists{/gist_id}","starred_url":"https://api.github.com/users/MadsTorgersen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MadsTorgersen/subscriptions","organizations_url":"https://api.github.com/users/MadsTorgersen/orgs","repos_url":"https://api.github.com/users/MadsTorgersen/repos","events_url":"https://api.github.com/users/MadsTorgersen/events{/privacy}","received_events_url":"https://api.github.com/users/MadsTorgersen/received_events","type":"User","site_admin":false},"labels":[{"id":537024200,"node_id":"MDU6TGFiZWw1MzcwMjQyMDA=","url":"https://api.github.com/repos/dotnet/csharplang/labels/Discussion","name":"Discussion","color":"c2e0c6","default":false},{"id":513192173,"node_id":"MDU6TGFiZWw1MTMxOTIxNzM=","url":"https://api.github.com/repos/dotnet/csharplang/labels/Proposal","name":"Proposal","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-10-19T05:31:35Z","updated_at":"2018-12-05T12:52:17Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"# A design for nullable opt-in\r\n\r\nAt the C# Language Design Meeting on [Oct 3](https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-10-03.md) we resolved to express source-level opt-in to the nullable reference types feature through the use of compiler directives.\r\n\r\nHere is a proposal for a concrete design that fleshes out more details.\r\n\r\n# Contexts\r\nEvery line of source code has a nullability context, which consists of an annotation context and a warning context.\r\n\r\n## Annotation context\r\nThe annotation context for a given line is either *on* or *off*\r\n* When the annotation context is *on*, unannotated reference types are considered non-nullable, and `?`-annotated reference types are considered nullable.\r\n* When the annotation context is *off*, unannotated reference types are considered oblivious, and `?`-annotated reference types yield a warning and are considered oblivious.\r\n\r\n## Warning context\r\nThe warning context for a given line is either *on* or *off* for each warning id. When a given warning condition occurs on a given line, and the warning context of that warning on that line is *on*, the warning is given. \r\n\r\n# Switches\r\nThere are two levels of switches to control nullable behavior:\r\n\r\n1. Project-level switches provide default behavior for all files in the project, and are also known as \"compiler switches\" *I won’t try to detail here how exactly they are specified, since I don’t know enough about it.*\r\n2. File level switches provide behavior for a certain span of lines in a given source file, and are specified by means of `#`-directives in the source code.\r\n\r\nAt each of the two levels there is a hierarchy of switches:\r\na. Feature switch: sets the annotation context as well as the warning context for all nullable warnings\r\nb. Warning switch: sets the warning context for all nullable warnings\r\nc. Specific switch: sets the warning context for a specific warning (as today)\r\n\r\nSpecific trumps general. Specifically:\r\n\r\n1.\tAt the project level, settings for specific warnings (c) trump those for all nullable warnings (b), and both trump the general feature switch (a).\r\n2.\tWithin a file, a given directive changes the part of the context it controls for subsequent lines, and leaves the rest unchanged from above it\r\n\r\n## File-level directives\r\nThe directives for the three kinds of switches are:\r\n``` c@\r\n#nullable <setting>                // (a) Set annotation context and all nullable warnings\r\n#pragma warning nullable <setting> // (b) Set all nullable warnings\r\n#pragma warning CSxxxx <setting>   // (c) Set a specific warning, as today\r\n```\r\n\r\n# Settings\r\nCurrent `#pragma` warning directives only have disable and restore settings. We can technically make do with those, but there is more expressiveness that could be had by adding more settings.\r\n\r\n## Restoring to default\r\nSince in today’s world all warnings are on by default (*is this correct?*), `restore` simply turns the warning back on. In C# 8.0 it’s going to be common for warnings to be *off* by default (because of project level settings around nullable or warning waves). If directives can only turn *on* and *off*, then the only part of a source file that can adhere to project level settings is the beginning of it, *before* the first directive! If we want to allow returning to project settings later in the file, we need a third setting to do that. I propose reinterpreting `restore` to mean \"restore to project level setting\" (which could be *off*), and then adding `enable` which would be the dual of `disable` and always turn the warning on.\r\n\r\nExample:\r\n\r\n``` c#\r\n// To start out the context is based on project settings\r\npublic class Dictionary<TKey, TValue>\r\n{\r\n    public Dictionary() { }\r\n    public Dictionary(IDictionary<TKey, TValue> dictionary) { }\r\n#nullable enable // annotations and all warnings are on \r\n    public Dictionary(IEnumerable<KeyValuePair<TKey, TValue>> collection) { }\r\n#pragma warning CS4321 restore // revert specific warning to project settings\r\n    public Dictionary(IEqualityComparer<TKey>? comparer) { }\r\n#nullable disable // annotations and all warnings are off\r\n    public Dictionary(int capacity) { }\r\n#pragma warning nullable enable // warnings are on, annotations still off\r\n    public Dictionary(IDictionary<TKey, TValue> dictionary, IEqualityComparer<TKey> comparer) { }\r\n#pragma warning nullable restore // warnings restored to project settings, annotations still off\r\n    public Dictionary(IEnumerable<KeyValuePair<TKey, TValue>> collection, IEqualityComparer<TKey> comparer) { }\r\n#nullable restore // annotations and all warnings restored to project settings \r\n    public Dictionary(int capacity, IEqualityComparer<TKey>? comparer) { }\r\n#pragma warning nullable enable // Warnings are on, annotations remain at project settings\r\n    protected Dictionary(SerializationInfo info, StreamingContext context) { }\r\n}\r\n```\r\n\r\n## W-warnings\r\nFurthermore, we need a way to specify that we want only non-W warnings. (\"W-warnings\" is our completely nondescriptive way of referring to null-warnings that don't directly impact null safety, such as assigning null to a nonnullable local.) I propose a *fourth* setting that only works for nullable, not other kinds of warnings, which could be called `safe`. It means \"*off* for W-warnings but *on* for null-safety-critical warnings\". I propose we allow it *both* for `#pragma warning nullable` specifically, but also for `#nullable` directly. For the latter it means \"turn on annotations, and also warnings, except W-warnings\":\r\n\r\n``` c#\r\n#nullable disable // annotations and all warnings are off\r\n    public Dictionary(int capacity) { }\r\n#pragma warning nullable safe // non-W warnings are on, annotations still off\r\n    public Dictionary(IDictionary<TKey, TValue> dictionary, IEqualityComparer<TKey> comparer) { }\r\n    public Dictionary(IEnumerable<KeyValuePair<TKey, TValue>> collection, IEqualityComparer<TKey> comparer) { }\r\n#nullable safe // annotations and non-W warnings are on\r\n    public Dictionary(int capacity, IEqualityComparer<TKey>? comparer) { }\r\n#pragma warning nullable enable // All warnings are on, annotations remain on\r\n    protected Dictionary(SerializationInfo info, StreamingContext context) { }\r\n```\r\n\r\n## Project-level settings\r\nAt the project level we don’t need an equivalent to the `restore` setting, but we would want an equivalent to the `safe` setting. Again, I don’t have a proposal for what project-level switches look like syntactically.\r\n\r\n# Scenarios\r\nThe above design is proposed with the following prioritized list of scenarios in mind:\r\n\r\n1.\tI upgrade my code to C# 8.0. *It all still just works.*\r\n2.\tI start a new project in C# 8.0. *Nullable just works.*\r\n3.\tScenario 1, plus I want to use nullability everywhere.\r\n4.\tScenario 2, plus I don’t want to use nullability anywhere.\r\n5.\tScenario 1 or 4, plus I want to gradually add nullability annotations and get nullability warnings.\r\n6.\tScenario 2 or 3, plus I want to omit annotations or avoid warnings in some places.\r\n7.\tScenario 1 through 6, plus I want to control warnings separately from annotations.\r\n\r\nThe complexity of getting into each scenario increases only as priority decreases:\r\n\r\n## Scenario 1: Upgrade\r\nThe default setting for the project-level feature switch is off, and no code that compiled in C# 7.x will have the new directives in it, so this happens by default without any explicit effort.\r\n\r\n## Scenario 2: New project\r\nC# 8.0 project templates will have the project-level feature switch `enable`d. This makes annotation and warning contexts (for all nullable warnings) *on* by default in all source files. I don’t have to do anything.\r\n\r\n## Scenario 3: Nullify upgraded project\r\nSet the project level feature switch to `safe` or *on*.\r\n\r\n## Scenario 4: De-nullify new project\r\nRemove the project-level feature switch or set it to *off*.\r\n\r\n## Scenario 5: Nullify within oblivious project\r\nAdd `#nullable safe` or `#nullable enable` directives where applicable.\r\n\r\n## Scenario 6: De-nullify within null-aware project\r\nAdd `#nullable disable` directives where applicable\r\n\r\n## Scenario 7: Control warnings separately\r\nAdd `#pragma warning nullable` directives to deviate locally from warning contexts introduced by `#nullable` or the project settings, or add project warning settings to deviate systematically from project feature settings across all files.\r\n\r\n# Discussion\r\nThe above proposal has some complexity. However I feel like it hits all the scenarios, and makes the complexity “pay for play”, depending on the gnarliness of the user’s situation:\r\n\r\n* Scenario 1 and 2 don’t need any new effort from the developer\r\n* Scenario 3 and 4 require toggling a single project-level switch (which we should make good UI for)\r\n* Scenario 5 and 6 require use of `#nullable` directives to turn the whole feature on and off for parts of the code\r\n* Scenario 7 requires project-level warning settings and/or use of `#pragma warning nullable` directives to handle warnings separately from annotations\r\n\r\nI think the proposed `restore`/`enable` distinction is useful (even for warnings that are not related to nullability), but we can certainly live without it. However, if we want to do it we need to do it now, since it changes the meaning of `restore` for scenarios we are about to introduce.\r\n\r\nI really like using a `safe` setting for disabling W-warnings, because it makes it more first class and enables it to be expressed directly on the feature setting. You don’t *first* have to enable all warnings, and *then* turn some of them back off. However, if we don’t like it we can instead go back to the idea of having a fake diagnostic for the W-warnings, but I think that makes turning the W-warnings off way too obscure.\r\n\r\nI previously advocated that the in-file directives should only \"work\" when the feature was turned on at the project level. I am now convinced that it is important to be able to turn it on per file without churning other files. A specific scenario is generated code that is not annotated: you cannot edit it, and you want to keep it oblivious.\r\n\r\nThe proposal makes it up to the developer in scenario 5 if/when to \"flip\" into a scenario 6 situation – that is, to go from file-level opt-in to project level opt-in with file-level opt-out. I think \"flipping\" is a much less dramatic operation now that we have directives than it would have been with attributes, so I’m not too worried about it.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/dotnet/csharplang/issues/comments/444474579","html_url":"https://github.com/dotnet/csharplang/issues/1939#issuecomment-444474579","issue_url":"https://api.github.com/repos/dotnet/csharplang/issues/1939","id":444474579,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NDQ3NDU3OQ==","user":{"login":"MichaelKetting","id":6972548,"node_id":"MDQ6VXNlcjY5NzI1NDg=","avatar_url":"https://avatars2.githubusercontent.com/u/6972548?v=4","gravatar_id":"","url":"https://api.github.com/users/MichaelKetting","html_url":"https://github.com/MichaelKetting","followers_url":"https://api.github.com/users/MichaelKetting/followers","following_url":"https://api.github.com/users/MichaelKetting/following{/other_user}","gists_url":"https://api.github.com/users/MichaelKetting/gists{/gist_id}","starred_url":"https://api.github.com/users/MichaelKetting/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MichaelKetting/subscriptions","organizations_url":"https://api.github.com/users/MichaelKetting/orgs","repos_url":"https://api.github.com/users/MichaelKetting/repos","events_url":"https://api.github.com/users/MichaelKetting/events{/privacy}","received_events_url":"https://api.github.com/users/MichaelKetting/received_events","type":"User","site_admin":false},"created_at":"2018-12-05T12:52:17Z","updated_at":"2018-12-05T12:52:17Z","author_association":"NONE","body":"I've just tried Visual Studio 2019 Preview 1 to take C#8 for a spin and from what I can surmise, the the project-level setting doesn't exist yet, is that correct? I.e. do I have to apply `#nullable enable` to each individual code file?"}}
{"actor":{"display_login":"MichaelKetting"}}