Nullable Reference Types for main library

.editorconfig

@@ -27,71 +27,123 @@ indent_size = 2
 # Dotnet code style settings:
 [*.{cs,vb}]
 # Sort using and Import directives with System.* appearing first
-dotnet_sort_system_directives_first = true
+dotnet_sort_system_directives_first = true:warning
 # Avoid "this." and "Me." if not necessary
-dotnet_style_qualification_for_field = false:suggestion
-dotnet_style_qualification_for_property = false:suggestion
-dotnet_style_qualification_for_method = false:suggestion
-dotnet_style_qualification_for_event = false:suggestion
+dotnet_style_qualification_for_field = false:warning
+dotnet_style_qualification_for_property = false:warning
+dotnet_style_qualification_for_method = false:warning
+dotnet_style_qualification_for_event = false:warning
+
+# Modifiers
+dotnet_style_require_accessibility_modifiers = for_non_interface_members:warning
+dotnet_style_readonly_field = true:warning
 
 # Use language keywords instead of framework type names for type references
-dotnet_style_predefined_type_for_locals_parameters_members = true:suggestion
-dotnet_style_predefined_type_for_member_access = true:suggestion
+dotnet_style_predefined_type_for_locals_parameters_members = true:warning
+dotnet_style_predefined_type_for_member_access = true:warning
 
 # Suggest more modern language features when available
-dotnet_style_object_initializer = true:suggestion
-dotnet_style_collection_initializer = true:suggestion
-dotnet_style_coalesce_expression = true:suggestion
-dotnet_style_null_propagation = true:suggestion
-dotnet_style_explicit_tuple_names = true:suggestion
+dotnet_style_object_initializer = true:warning
+dotnet_style_collection_initializer = true:warning
+dotnet_style_explicit_tuple_names = true:warning
+dotnet_style_null_propagation = true:warning
+dotnet_style_coalesce_expression = true:warning
+dotnet_style_prefer_is_null_check_over_reference_equality_method = true:warning
+dotnet_style_prefer_auto_properties = true:suggestion
 
 # Ignore silly if statements
-dotnet_style_prefer_conditional_expression_over_return = false:none
+dotnet_style_prefer_conditional_expression_over_assignment = true:suggestion
+dotnet_style_prefer_conditional_expression_over_return = true:suggestion
 
 # Don't warn on things that actually need suppressing
 dotnet_remove_unnecessary_suppression_exclusions = CA1009,CA1063,CA1069,CA1416,CA1816,CA1822,CA2202,CS0618,IDE0060,IDE0062,RCS1047,RCS1085,RCS1090,RCS1194,RCS1231
 
+# Style Definitions
+dotnet_naming_style.pascal_case_style.capitalization = pascal_case
+# Use PascalCase for constant fields  
+dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = suggestion
+dotnet_naming_rule.constant_fields_should_be_pascal_case.symbols = constant_fields
+dotnet_naming_rule.constant_fields_should_be_pascal_case.style = pascal_case_style
+dotnet_naming_symbols.constant_fields.applicable_kinds = field
+dotnet_naming_symbols.constant_fields.applicable_accessibilities = *
+dotnet_naming_symbols.constant_fields.required_modifiers = const
+
 # CSharp code style settings:
 [*.cs]
 # Prefer method-like constructs to have a expression-body
-csharp_style_expression_bodied_methods = true:none
-csharp_style_expression_bodied_constructors = true:none
-csharp_style_expression_bodied_operators = true:none
+csharp_style_expression_bodied_constructors = true:silent
+csharp_style_expression_bodied_methods = true:silent
+csharp_style_expression_bodied_operators = true:warning
 
 # Prefer property-like constructs to have an expression-body
-csharp_style_expression_bodied_properties = true:none
-csharp_style_expression_bodied_indexers = true:none
-csharp_style_expression_bodied_accessors = true:none
-
-# Suggest more modern language features when available
-csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion
-csharp_style_pattern_matching_over_as_with_null_check = true:suggestion
-csharp_style_inlined_variable_declaration = true:suggestion
-csharp_style_throw_expression = true:suggestion
-csharp_style_conditional_delegate_call = true:suggestion
+csharp_style_expression_bodied_accessors = true:warning
+csharp_style_expression_bodied_indexers = true:warning
+csharp_style_expression_bodied_properties = true:warning
+csharp_style_expression_bodied_lambdas = true:warning
+csharp_style_expression_bodied_local_functions = true:silent
+
+# Pattern matching preferences
+csharp_style_pattern_matching_over_is_with_cast_check = true:warning
+csharp_style_pattern_matching_over_as_with_null_check = true:warning
+
+# Null-checking preferences
+csharp_style_throw_expression = true:warning
+csharp_style_conditional_delegate_call = true:warning
+
+# Modifier preferences
+csharp_preferred_modifier_order = public,private,protected,internal,static,extern,new,virtual,abstract,sealed,override,readonly,volatile,async:suggestion
+
+# Expression-level preferences
+csharp_prefer_braces = true:silent
+csharp_style_deconstructed_variable_declaration = true:suggestion
+csharp_prefer_simple_default_expression = true:silent
+csharp_style_pattern_local_over_anonymous_function = true:suggestion
+csharp_style_inlined_variable_declaration = true:warning
 csharp_prefer_simple_using_statement = true:silent
+csharp_style_prefer_not_pattern = true:warning
+csharp_style_prefer_switch_expression = true:warning
 
 # Disable range operator suggestions
 csharp_style_prefer_range_operator = false:none
 csharp_style_prefer_index_operator = false:none
 
-# Newline settings
+# New line preferences
 csharp_new_line_before_open_brace = all
 csharp_new_line_before_else = true
 csharp_new_line_before_catch = true
 csharp_new_line_before_finally = true
 csharp_new_line_before_members_in_object_initializers = true
 csharp_new_line_before_members_in_anonymous_types = true
-
-# Space settings
-csharp_space_after_keywords_in_control_flow_statements = true:suggestion
-
-# Language settings
-csharp_prefer_simple_default_expression = false:none
+csharp_new_line_between_query_expression_clauses = true
+
+# Indentation preferences
+csharp_indent_case_contents = true
+csharp_indent_switch_labels = true
+csharp_indent_labels = flush_left
+
+# Space preferences
+csharp_space_after_cast = false
+csharp_space_after_keywords_in_control_flow_statements = true:warning
+csharp_space_between_method_call_parameter_list_parentheses = false
+csharp_space_between_method_declaration_parameter_list_parentheses = false
+csharp_space_between_parentheses = false
+csharp_space_before_colon_in_inheritance_clause = true
+csharp_space_after_colon_in_inheritance_clause = true
+csharp_space_around_binary_operators = before_and_after
+csharp_space_between_method_declaration_empty_parameter_list_parentheses = false
+csharp_space_between_method_call_name_and_opening_parenthesis = false
+csharp_space_between_method_call_empty_parameter_list_parentheses = false
+
+# Wrapping preferences
+csharp_preserve_single_line_statements = true
+csharp_preserve_single_line_blocks = true
 
 # IDE0090: Use 'new(...)'
 dotnet_diagnostic.IDE0090.severity = silent
 
+# RCS1037: Remove trailing white-space.
+dotnet_diagnostic.RCS1037.severity = error
+
 # RCS1098: Constant values should be placed on right side of comparisons.
 dotnet_diagnostic.RCS1098.severity = none
 
@@ -105,4 +157,4 @@ dotnet_diagnostic.RCS1229.severity = none
 dotnet_diagnostic.RCS1233.severity = none
 
 # RCS1234: Duplicate enum value.
-dotnet_diagnostic.RCS1234.severity = none
+dotnet_diagnostic.RCS1234.severity = none

docs/ReleaseNotes.md

@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+- Adds: [Nullable reference type](https://docs.microsoft.com/en-us/dotnet/csharp/nullable-references) annotations ([#2041 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2041))
+  - Adds annotations themselves for nullability to everything in the library
+  - Fixes a few internal edge cases that will now throw proper errors (rather than a downstream null reference)
+  - Fixes inconsistencies with `null` vs. empty array returns (preferring an not-null empty array in those edge cases)
+  - Note: does *not* increment a major version (as these are warnings to consumers), because: they're warnings (errors are opt-in), removing obsolete types with a 3.0 rev _would_ be binary breaking (this isn't), and reving to 3.0 would cause binding redirect pain for consumers. Bumping from 2.5 to 2.6 only for this change.
 
 ## 2.5.61
 

src/StackExchange.Redis/ChannelMessageQueue.cs

@@ -17,14 +17,15 @@ namespace StackExchange.Redis
         /// <summary>
         /// The Channel:Message string representation.
         /// </summary>
-        public override string ToString() => ((string)Channel) + ":" + ((string)Message);
+        public override string ToString() => ((string?)Channel) + ":" + ((string?)Message);
 
         /// <inheritdoc/>
         public override int GetHashCode() => Channel.GetHashCode() ^ Message.GetHashCode();
 
         /// <inheritdoc/>
-        public override bool Equals(object obj) => obj is ChannelMessage cm
+        public override bool Equals(object? obj) => obj is ChannelMessage cm
             && cm.Channel == Channel && cm.Message == Message;
+
         internal ChannelMessage(ChannelMessageQueue queue, in RedisChannel channel, in RedisValue value)
         {
             _queue = queue;
@@ -72,12 +73,12 @@ public sealed class ChannelMessageQueue
         /// The Channel that was subscribed for this queue.
         /// </summary>
         public RedisChannel Channel { get; }
-        private RedisSubscriber _parent;
+        private RedisSubscriber? _parent;
 
         /// <summary>
         /// The string representation of this channel.
         /// </summary>
-        public override string ToString() => (string)Channel;
+        public override string? ToString() => (string?)Channel;
 
         /// <summary>
         /// An awaitable task the indicates completion of the queue (including drain of data).
@@ -127,9 +128,9 @@ public bool TryGetCount(out int count)
             try
             {
                 var prop = _queue.GetType().GetProperty("ItemsCountForDebugger", BindingFlags.Instance | BindingFlags.NonPublic);
-                if (prop != null)
+                if (prop is not null)
                 {
-                    count = (int)prop.GetValue(_queue);
+                    count = (int)prop.GetValue(_queue)!;
                     return true;
                 }
             }
@@ -138,7 +139,7 @@ public bool TryGetCount(out int count)
             return false;
         }
 
-        private Delegate _onMessageHandler;
+        private Delegate? _onMessageHandler;
         private void AssertOnMessage(Delegate handler)
         {
             if (handler == null) throw new ArgumentNullException(nameof(handler));
@@ -155,34 +156,34 @@ public void OnMessage(Action<ChannelMessage> handler)
             AssertOnMessage(handler);
 
             ThreadPool.QueueUserWorkItem(
-                state => ((ChannelMessageQueue)state).OnMessageSyncImpl().RedisFireAndForget(), this);
+                state => ((ChannelMessageQueue)state!).OnMessageSyncImpl().RedisFireAndForget(), this);
         }
 
         private async Task OnMessageSyncImpl()
         {
-            var handler = (Action<ChannelMessage>)_onMessageHandler;
+            var handler = (Action<ChannelMessage>?)_onMessageHandler;
             while (!Completion.IsCompleted)
             {
                 ChannelMessage next;
                 try { if (!TryRead(out next)) next = await ReadAsync().ForAwait(); }
                 catch (ChannelClosedException) { break; } // expected
                 catch (Exception ex)
                 {
-                    _parent.multiplexer?.OnInternalError(ex);
+                    _parent?.multiplexer?.OnInternalError(ex);
                     break;
                 }
 
-                try { handler(next); }
+                try { handler?.Invoke(next); }
                 catch { } // matches MessageCompletable
             }
         }
 
-        internal static void Combine(ref ChannelMessageQueue head, ChannelMessageQueue queue)
+        internal static void Combine(ref ChannelMessageQueue? head, ChannelMessageQueue queue)
         {
             if (queue != null)
             {
                 // insert at the start of the linked-list
-                ChannelMessageQueue old;
+                ChannelMessageQueue? old;
                 do
                 {
                     old = Volatile.Read(ref head);
@@ -200,12 +201,15 @@ public void OnMessage(Func<ChannelMessage, Task> handler)
             AssertOnMessage(handler);
 
             ThreadPool.QueueUserWorkItem(
-                state => ((ChannelMessageQueue)state).OnMessageAsyncImpl().RedisFireAndForget(), this);
+                state => ((ChannelMessageQueue)state!).OnMessageAsyncImpl().RedisFireAndForget(), this);
         }
 
-        internal static void Remove(ref ChannelMessageQueue head, ChannelMessageQueue queue)
+        internal static void Remove(ref ChannelMessageQueue? head, ChannelMessageQueue queue)
         {
-            if (queue == null) return;
+            if (queue is null)
+            {
+                return;
+            }
 
             bool found;
             do // if we fail due to a conflict, re-do from start
@@ -223,7 +227,7 @@ internal static void Remove(ref ChannelMessageQueue head, ChannelMessageQueue qu
                 }
                 else
                 {
-                    ChannelMessageQueue previous = current;
+                    ChannelMessageQueue? previous = current;
                     current = Volatile.Read(ref previous._next);
                     found = false;
                     do
@@ -242,13 +246,13 @@ internal static void Remove(ref ChannelMessageQueue head, ChannelMessageQueue qu
                             }
                         }
                         previous = current;
-                        current = Volatile.Read(ref previous._next);
+                        current = Volatile.Read(ref previous!._next);
                     } while (current != null);
                 }
             } while (found);
         }
 
-        internal static int Count(ref ChannelMessageQueue head)
+        internal static int Count(ref ChannelMessageQueue? head)
         {
             var current = Volatile.Read(ref head);
             int count = 0;
@@ -270,32 +274,32 @@ internal static void WriteAll(ref ChannelMessageQueue head, in RedisChannel chan
             }
         }
 
-        private ChannelMessageQueue _next;
+        private ChannelMessageQueue? _next;
 
         private async Task OnMessageAsyncImpl()
         {
-            var handler = (Func<ChannelMessage, Task>)_onMessageHandler;
+            var handler = (Func<ChannelMessage, Task>?)_onMessageHandler;
             while (!Completion.IsCompleted)
             {
                 ChannelMessage next;
                 try { if (!TryRead(out next)) next = await ReadAsync().ForAwait(); }
                 catch (ChannelClosedException) { break; } // expected
                 catch (Exception ex)
                 {
-                    _parent.multiplexer?.OnInternalError(ex);
+                    _parent?.multiplexer?.OnInternalError(ex);
                     break;
                 }
 
                 try
                 {
-                    var task = handler(next);
+                    var task = handler?.Invoke(next);
                     if (task != null && task.Status != TaskStatus.RanToCompletion) await task.ForAwait();
                 }
                 catch { } // matches MessageCompletable
             }
         }
 
-        internal static void MarkAllCompleted(ref ChannelMessageQueue head)
+        internal static void MarkAllCompleted(ref ChannelMessageQueue? head)
         {
             var current = Interlocked.Exchange(ref head, null);
             while (current != null)
@@ -305,13 +309,13 @@ internal static void MarkAllCompleted(ref ChannelMessageQueue head)
             }
         }
 
-        private void MarkCompleted(Exception error = null)
+        private void MarkCompleted(Exception? error = null)
         {
             _parent = null;
             _queue.Writer.TryComplete(error);
         }
 
-        internal void UnsubscribeImpl(Exception error = null, CommandFlags flags = CommandFlags.None)
+        internal void UnsubscribeImpl(Exception? error = null, CommandFlags flags = CommandFlags.None)
         {
             var parent = _parent;
             _parent = null;
@@ -322,7 +326,7 @@ internal void UnsubscribeImpl(Exception error = null, CommandFlags flags = Comma
             _queue.Writer.TryComplete(error);
         }
 
-        internal async Task UnsubscribeAsyncImpl(Exception error = null, CommandFlags flags = CommandFlags.None)
+        internal async Task UnsubscribeAsyncImpl(Exception? error = null, CommandFlags flags = CommandFlags.None)
         {
             var parent = _parent;
             _parent = null;