Nullability annotations for bindables

osu.Framework/Bindables/AggregateBindable.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
 using System.Linq;
@@ -31,7 +29,7 @@ public class AggregateBindable<T>
         /// </summary>
         /// <param name="aggregateFunction">The function to be used for aggregation, taking two input <typeparamref name="T"/> values and returning one output.</param>
         /// <param name="resultBindable">An optional newly constructed bindable to use for <see cref="Result"/>. The initial value of this bindable is used as the initial value for the aggregate.</param>
-        public AggregateBindable(Func<T, T, T> aggregateFunction, Bindable<T> resultBindable = null)
+        public AggregateBindable(Func<T, T, T> aggregateFunction, Bindable<T>? resultBindable = null)
         {
             this.aggregateFunction = aggregateFunction;
             result = resultBindable ?? new Bindable<T>();
@@ -77,10 +75,10 @@ public void RemoveSource(IBindable<T> bindable)
             }
         }
 
-        private WeakRefPair findExistingPair(IBindable<T> bindable) =>
+        private WeakRefPair? findExistingPair(IBindable<T> bindable) =>
             sourceMapping.FirstOrDefault(p => p.WeakReference.TryGetTarget(out var target) && target == bindable);
 
-        private void recalculateAggregate(ValueChangedEvent<T> obj = null)
+        private void recalculateAggregate(ValueChangedEvent<T>? obj = null)
         {
             T calculated = initialValue;
 

osu.Framework/Bindables/Bindable.cs

@@ -1,14 +1,13 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 using JetBrains.Annotations;
 using Newtonsoft.Json;
+using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Extensions.TypeExtensions;
 using osu.Framework.IO.Serialization;
 using osu.Framework.Lists;
@@ -24,17 +23,17 @@ public class Bindable<T> : IBindable<T>, IBindable, IParseable, ISerializableBin
         /// <summary>
         /// An event which is raised when <see cref="Value"/> has changed (or manually via <see cref="TriggerValueChange"/>).
         /// </summary>
-        public event Action<ValueChangedEvent<T>> ValueChanged;
+        public event Action<ValueChangedEvent<T>>? ValueChanged;
 
         /// <summary>
         /// An event which is raised when <see cref="Disabled"/> has changed (or manually via <see cref="TriggerDisabledChange"/>).
         /// </summary>
-        public event Action<bool> DisabledChanged;
+        public event Action<bool>? DisabledChanged;
 
         /// <summary>
         /// An event which is raised when <see cref="Default"/> has changed (or manually via <see cref="TriggerDefaultChange"/>).
         /// </summary>
-        public event Action<ValueChangedEvent<T>> DefaultChanged;
+        public event Action<ValueChangedEvent<T>>? DefaultChanged;
 
         private T value;
 
@@ -59,7 +58,7 @@ public virtual bool Disabled
             }
         }
 
-        internal void SetDisabled(bool value, bool bypassChecks = false, Bindable<T> source = null)
+        internal void SetDisabled(bool value, bool bypassChecks = false, Bindable<T>? source = null)
         {
             if (!bypassChecks)
                 throwIfLeased();
@@ -90,15 +89,15 @@ public virtual T Value
                 // if the leased bindable decides to disable exclusive access (by setting Disabled = false) then anything will be able to write to Value.
 
                 if (Disabled)
-                    throw new InvalidOperationException($"Can not set value to \"{value.ToString()}\" as bindable is disabled.");
+                    throw new InvalidOperationException($"Can not set value to \"{value}\" as bindable is disabled.");
 
                 if (EqualityComparer<T>.Default.Equals(this.value, value)) return;
 
                 SetValue(this.value, value);
             }
         }
 
-        internal void SetValue(T previousValue, T value, bool bypassChecks = false, Bindable<T> source = null)
+        internal void SetValue(T previousValue, T value, bool bypassChecks = false, Bindable<T>? source = null)
         {
             this.value = value;
             TriggerValueChange(previousValue, source ?? this, true, bypassChecks);
@@ -116,21 +115,21 @@ public virtual T Default
                 // if the leased bindable decides to disable exclusive access (by setting Disabled = false) then anything will be able to write to Default.
 
                 if (Disabled)
-                    throw new InvalidOperationException($"Can not set default value to \"{value.ToString()}\" as bindable is disabled.");
+                    throw new InvalidOperationException($"Can not set default value to \"{value}\" as bindable is disabled.");
 
                 if (EqualityComparer<T>.Default.Equals(defaultValue, value)) return;
 
                 SetDefaultValue(defaultValue, value);
             }
         }
 
-        internal void SetDefaultValue(T previousValue, T value, bool bypassChecks = false, Bindable<T> source = null)
+        internal void SetDefaultValue(T previousValue, T value, bool bypassChecks = false, Bindable<T>? source = null)
         {
             defaultValue = value;
             TriggerDefaultChange(previousValue, source ?? this, true, bypassChecks);
         }
 
-        private WeakReference<Bindable<T>> weakReferenceInstance;
+        private WeakReference<Bindable<T>>? weakReferenceInstance;
 
         private WeakReference<Bindable<T>> weakReference => weakReferenceInstance ??= new WeakReference<Bindable<T>>(this);
 
@@ -139,20 +138,22 @@ internal void SetDefaultValue(T previousValue, T value, bool bypassChecks = fals
         /// </summary>
         [UsedImplicitly]
         private Bindable()
-            : this(default)
+            : this(default!)
         {
         }
 
         /// <summary>
         /// Creates a new bindable instance initialised with a default value.
         /// </summary>
         /// <param name="defaultValue">The initial and default value for this bindable.</param>
-        public Bindable(T defaultValue = default)
+        /// <remarks>Consider passing a default value for non-nullable <typeparamref name="T"/>.</remarks>
+        public Bindable(T defaultValue = default!)
         {
-            value = Default = defaultValue;
+            // TODO: add a custom analyser warning about no default value provided for non-nullable T
+            value = this.defaultValue = defaultValue;
         }
 
-        protected LockedWeakList<Bindable<T>> Bindings { get; private set; }
+        protected LockedWeakList<Bindable<T>>? Bindings { get; private set; }
 
         void IBindable.BindTo(IBindable them)
         {
@@ -254,7 +255,7 @@ public virtual void Parse(object input)
 
                 default:
                     if (underlyingType.IsEnum)
-                        Value = (T)Enum.Parse(underlyingType, input.ToString());
+                        Value = (T)Enum.Parse(underlyingType, input.ToString()!);
                     else
                         Value = (T)Convert.ChangeType(input, underlyingType, CultureInfo.InvariantCulture);
 
@@ -361,8 +362,9 @@ public void UnbindBindings()
 
         internal virtual void UnbindAllInternal()
         {
+            // TODO: annotate isLeased with [MemberNotNull(nameof(leasedBindable))] on .NET 5+ to satisfy the nullability check
             if (isLeased)
-                leasedBindable.Return();
+                leasedBindable.AsNonNull().Return();
 
             UnbindEvents();
             UnbindBindings();
@@ -377,7 +379,7 @@ public virtual void UnbindFrom(IUnbindable them)
             tThem.removeWeakReference(weakReference);
         }
 
-        public string Description { get; set; }
+        public string? Description { get; set; }
 
         public override string ToString() => value?.ToString() ?? string.Empty;
 
@@ -410,10 +412,11 @@ void ISerializableBindable.SerializeTo(JsonWriter writer, JsonSerializer seriali
 
         void ISerializableBindable.DeserializeFrom(JsonReader reader, JsonSerializer serializer)
         {
-            Value = serializer.Deserialize<T>(reader);
+            // Deserialize returns null for json literal "null".
+            Value = serializer.Deserialize<T>(reader)!;
         }
 
-        private LeasedBindable<T> leasedBindable;
+        private LeasedBindable<T>? leasedBindable;
 
         private bool isLeased => leasedBindable != null;
 

osu.Framework/Bindables/BindableBool.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 namespace osu.Framework.Bindables
 {
     public class BindableBool : Bindable<bool>

osu.Framework/Bindables/BindableDouble.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System.Globalization;
 
 namespace osu.Framework.Bindables

osu.Framework/Bindables/BindableFloat.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System.Globalization;
 
 namespace osu.Framework.Bindables

osu.Framework/Bindables/BindableInt.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System.Globalization;
 
 namespace osu.Framework.Bindables

osu.Framework/Bindables/BindableList.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System;
 using System.Collections;
 using System.Collections.Generic;
@@ -18,26 +16,26 @@ public class BindableList<T> : IBindableList<T>, IBindable, IParseable, IList<T>
         /// <summary>
         /// An event which is raised when this <see cref="BindableList{T}"/> changes.
         /// </summary>
-        public event NotifyCollectionChangedEventHandler CollectionChanged;
+        public event NotifyCollectionChangedEventHandler? CollectionChanged;
 
         /// <summary>
         /// An event which is raised when <see cref="Disabled"/>'s state has changed (or manually via <see cref="triggerDisabledChange(bool)"/>).
         /// </summary>
-        public event Action<bool> DisabledChanged;
+        public event Action<bool>? DisabledChanged;
 
         private readonly List<T> collection = new List<T>();
 
         private readonly Cached<WeakReference<BindableList<T>>> weakReferenceCache = new Cached<WeakReference<BindableList<T>>>();
 
         private WeakReference<BindableList<T>> weakReference => weakReferenceCache.IsValid ? weakReferenceCache.Value : weakReferenceCache.Value = new WeakReference<BindableList<T>>(this);
 
-        private LockedWeakList<BindableList<T>> bindings;
+        private LockedWeakList<BindableList<T>>? bindings;
 
         /// <summary>
         /// Creates a new <see cref="BindableList{T}"/>, optionally adding the items of the given collection.
         /// </summary>
         /// <param name="items">The items that are going to be contained in the newly created <see cref="BindableList{T}"/>.</param>
-        public BindableList(IEnumerable<T> items = null)
+        public BindableList(IEnumerable<T>? items = null)
         {
             if (items != null)
                 collection.AddRange(items);
@@ -56,7 +54,7 @@ public T this[int index]
             set => setIndex(index, value, null);
         }
 
-        private void setIndex(int index, T item, BindableList<T> caller)
+        private void setIndex(int index, T item, BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -86,7 +84,7 @@ private void setIndex(int index, T item, BindableList<T> caller)
         public void Add(T item)
             => add(item, null);
 
-        private void add(T item, BindableList<T> caller)
+        private void add(T item, BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -122,7 +120,7 @@ private void add(T item, BindableList<T> caller)
         public void Insert(int index, T item)
             => insert(index, item, null);
 
-        private void insert(int index, T item, BindableList<T> caller)
+        private void insert(int index, T item, BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -149,7 +147,7 @@ private void insert(int index, T item, BindableList<T> caller)
         public void Clear()
             => clear(null);
 
-        private void clear(BindableList<T> caller)
+        private void clear(BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -192,7 +190,7 @@ public bool Contains(T item)
         public bool Remove(T item)
             => remove(item, null);
 
-        private bool remove(T item, BindableList<T> caller)
+        private bool remove(T item, BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -233,7 +231,7 @@ public void RemoveRange(int index, int count)
             removeRange(index, count, null);
         }
 
-        private void removeRange(int index, int count, BindableList<T> caller)
+        private void removeRange(int index, int count, BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -266,7 +264,7 @@ private void removeRange(int index, int count, BindableList<T> caller)
         public void RemoveAt(int index)
             => removeAt(index, null);
 
-        private void removeAt(int index, BindableList<T> caller)
+        private void removeAt(int index, BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -295,7 +293,7 @@ private void removeAt(int index, BindableList<T> caller)
         public int RemoveAll(Predicate<T> match)
             => removeAll(match, null);
 
-        private int removeAll(Predicate<T> match, BindableList<T> caller)
+        private int removeAll(Predicate<T> match, BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -349,25 +347,25 @@ public void CopyTo(Array array, int index)
 
         #region IList
 
-        object IList.this[int index]
+        object? IList.this[int index]
         {
             get => this[index];
-            set => this[index] = (T)value;
+            set => this[index] = (T)value!;
         }
 
-        int IList.Add(object value)
+        int IList.Add(object? value)
         {
-            Add((T)value);
+            Add((T)value!);
             return Count - 1;
         }
 
-        bool IList.Contains(object value) => Contains((T)value);
+        bool IList.Contains(object? value) => Contains((T)value!);
 
-        int IList.IndexOf(object value) => IndexOf((T)value);
+        int IList.IndexOf(object? value) => IndexOf((T)value!);
 
-        void IList.Insert(int index, object value) => Insert(index, (T)value);
+        void IList.Insert(int index, object? value) => Insert(index, (T)value!);
 
-        void IList.Remove(object value) => Remove((T)value);
+        void IList.Remove(object? value) => Remove((T)value!);
 
         bool IList.IsFixedSize => false;
 
@@ -490,7 +488,7 @@ public virtual void UnbindFrom(IUnbindable them)
 
         #region IHasDescription
 
-        public string Description { get; set; }
+        public string? Description { get; set; }
 
         #endregion IHasDescription
 
@@ -504,7 +502,7 @@ public virtual void UnbindFrom(IUnbindable them)
         public void AddRange(IEnumerable<T> items)
             => addRange(items as IList ?? items.ToArray(), null);
 
-        private void addRange(IList items, BindableList<T> caller)
+        private void addRange(IList items, BindableList<T>? caller)
         {
             ensureMutationAllowed();
 
@@ -532,7 +530,7 @@ private void addRange(IList items, BindableList<T> caller)
         public void Move(int oldIndex, int newIndex)
             => move(oldIndex, newIndex, null);
 
-        private void move(int oldIndex, int newIndex, BindableList<T> caller)
+        private void move(int oldIndex, int newIndex, BindableList<T>? caller)
         {
             ensureMutationAllowed();