Fix/remove TODO-NULLABLEs

src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/CompareAttribute.cs

@@ -60,8 +60,6 @@ public override string FormatErrorMessage(string name) =>
             var display = attributes.OfType<DisplayAttribute>().FirstOrDefault();
             if (display != null)
             {
-                // TODO-NULLABLE: This will return null if [DisplayName] is specified but no Name has been defined - probably a bug.
-                // Should fall back to OtherProperty in this case instead.
                 return display.GetName();
             }
 

src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/ValidationAttribute.cs

@@ -273,8 +273,6 @@ private void SetResourceAccessorByPropertyLookup()
                                                     _errorMessageResourceType.FullName));
             }
 
-
-            // TODO-NULLABLE: If the user-provided resource returns null, an ArgumentNullException is thrown - should probably throw a better exception
             _errorMessageResourceAccessor = () => (string)property.GetValue(null, null)!;
         }
 

src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/Validator.cs

@@ -137,7 +137,6 @@ public static bool TryValidateObject(object instance, ValidationContext validati
                 throw new ArgumentNullException(nameof(instance));
             }
 
-            // TODO-NULLABLE: null validationContext isn't supported (GetObjectValidationErrors will throw), remove that check
             if (validationContext != null && instance != validationContext.ObjectInstance)
             {
                 throw new ArgumentException(SR.Validator_InstanceMustMatchValidationContextInstance, nameof(instance));

src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcConnectionOpen.cs

@@ -32,7 +32,7 @@ public override string ServerVersion
         {
             get
             {
-                // TODO-NULLABLE: This seems like it returns null if the connection is open, whereas the docs say it should throw
+                // https://github.com/dotnet/runtime/issues/44289: This seems like it returns null if the connection is open, whereas the docs say it should throw
                 // InvalidOperationException
                 return OuterConnection.Open_GetServerVersion()!;
             }

src/libraries/System.Data.OleDb/src/ColumnBinding.cs

@@ -948,8 +948,7 @@ internal OleDbDataReader Value_HCHAPTER()
             Debug.Assert(NativeDBType.HCHAPTER == DbType, "Value_HCHAPTER");
             Debug.Assert(DBStatus.S_OK == StatusValue(), "Value_HCHAPTER");
 
-            // TODO-NULLABLE: This shouldn't return null
-            return DataReader().ResetChapter(IndexForAccessor, IndexWithinAccessor, RowBinding, ValueOffset)!;
+            return DataReader().ResetChapter(IndexForAccessor, IndexWithinAccessor, RowBinding, ValueOffset);
         }
 
         private sbyte Value_I1()

src/libraries/System.Data.OleDb/src/OleDbCommand.cs

@@ -818,7 +818,6 @@ private int ExecuteCommandText(out object executeResult)
             RuntimeHelpers.PrepareConstrainedRegions();
             try
             {
-                // TODO-NULLABLE: Code below seems to assume that bindings will always be non-null
                 if (null != bindings)
                 { // parameters may be suppressed
                     rowbinding = bindings.RowBinding();

src/libraries/System.Data.OleDb/src/OleDbConnectionFactory.cs

@@ -33,8 +33,7 @@ public override DbProviderFactory ProviderFactory
 
         protected override DbConnectionInternal CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, object poolGroupProviderInfo, DbConnectionPool? pool, DbConnection? owningObject)
         {
-            // TODO-NULLABLE: owningObject may actually be null (see DbConnectionPool.CreateObject), in which case this will throw...
-            DbConnectionInternal result = new OleDbConnectionInternal((OleDbConnectionString)options, (OleDbConnection)owningObject!);
+            DbConnectionInternal result = new OleDbConnectionInternal((OleDbConnectionString)options, (OleDbConnection?)owningObject);
             return result;
         }
 

src/libraries/System.Data.OleDb/src/OleDbConnectionInternal.cs

@@ -385,7 +385,7 @@ internal void EnlistTransactionInternal(SysTx.Transaction? transaction)
             using (IDBInfoWrapper wrapper = IDBInfo())
             {
                 UnsafeNativeMethods.IDBInfo dbInfo = wrapper.Value;
-                // TODO-NULLABLE: check may not be necessary (and thus method may return non-nullable)
+                // https://github.com/dotnet/runtime/issues/44288: check may not be necessary (and thus method may return non-nullable)
                 if (null == dbInfo)
                 {
                     return null;

src/libraries/System.Data.OleDb/src/OleDbDataReader.cs

@@ -951,15 +951,15 @@ protected override DbDataReader GetDbDataReader(int ordinal)
             return GetData(ordinal);
         }
 
-        internal OleDbDataReader? ResetChapter(int bindingIndex, int index, RowBinding rowbinding, int valueOffset)
+        internal OleDbDataReader ResetChapter(int bindingIndex, int index, RowBinding rowbinding, int valueOffset)
         {
             return GetDataForReader(_metadata![bindingIndex + index].ordinal, rowbinding, valueOffset);
         }
 
-        private OleDbDataReader? GetDataForReader(IntPtr ordinal, RowBinding rowbinding, int valueOffset)
+        private OleDbDataReader GetDataForReader(IntPtr ordinal, RowBinding rowbinding, int valueOffset)
         {
             UnsafeNativeMethods.IRowsetInfo rowsetInfo = IRowsetInfo();
-            UnsafeNativeMethods.IRowset? result;
+            UnsafeNativeMethods.IRowset result;
             OleDbHResult hr;
             hr = rowsetInfo.GetReferencedRowset((IntPtr)ordinal, ref ODB.IID_IRowset, out result);
 
@@ -1022,8 +1022,9 @@ public override Type GetFieldType(int index)
         {
             if (null != _metadata)
             {
-                // TODO-NULLABLE: Should throw if null (empty), though it probably doesn't happen
-                return _metadata[index].type.dataType!;
+                Type? fieldType = _metadata[index].type.dataType!;
+                Debug.Assert(fieldType != null);
+                return fieldType;
             }
             throw ADP.DataReaderNoData();
         }
@@ -2273,7 +2274,6 @@ internal void DumpToSchemaTable(UnsafeNativeMethods.IRowset? rowset)
             using (OleDbDataReader dataReader = new OleDbDataReader(_connection, _command, int.MinValue, 0))
             {
                 dataReader.InitializeIRowset(rowset, ChapterHandle.DB_NULL_HCHAPTER, IntPtr.Zero);
-                // TODO-NULLABLE: BuildSchemaTableInfo asserts that rowset isn't null, but doesn't do anything with it
                 dataReader.BuildSchemaTableInfo(rowset!, true, false);
 
                 hiddenColumns = GetPropertyValue(ODB.DBPROP_HIDDENCOLUMNS);

src/libraries/System.Linq/src/System/Linq/Select.SpeedOpt.cs

@@ -11,7 +11,7 @@ namespace System.Linq
     public static partial class Enumerable
     {
         static partial void CreateSelectIPartitionIterator<TResult, TSource>(
-            Func<TSource, TResult> selector, IPartition<TSource> partition, ref IEnumerable<TResult>? result) // TODO-NULLABLE: https://github.com/dotnet/roslyn/issues/38327
+            Func<TSource, TResult> selector, IPartition<TSource> partition, ref IEnumerable<TResult>? result)
         {
             result = partition is EmptyPartition<TSource> ?
                 EmptyPartition<TResult>.Instance :

src/libraries/System.Private.Xml.Linq/src/System/Xml/Linq/Extensions.cs

@@ -277,8 +277,6 @@ public static IEnumerable<T> InDocumentOrder<T>(this IEnumerable<T> source) wher
             return DocumentOrderIterator<T>(source);
         }
 
-        // TODO-NULLABLE: Consider changing to T? instead.
-        // If we do it, we will also need to change XNodeDocumentOrderComparer to implement IComparer<XNode?> instead.
         private static IEnumerable<T> DocumentOrderIterator<T>(IEnumerable<T> source) where T : XNode
         {
             int count;

src/libraries/System.Private.Xml.Linq/src/System/Xml/Linq/XContainer.cs

@@ -1041,8 +1041,7 @@ public async ValueTask<bool> ReadContentFromAsync(XContainer rootContainer, XmlR
             public bool ReadContentFrom(XContainer rootContainer, XmlReader r, LoadOptions o)
             {
                 XNode? newNode = null;
-                // TODO-NULLABLE: Consider changing XmlReader.BaseURI to non-nullable.
-                string baseUri = r.BaseURI!;
+                string baseUri = r.BaseURI;
 
                 switch (r.NodeType)
                 {

src/libraries/System.Private.Xml.Linq/src/System/Xml/Linq/XDocumentType.cs

@@ -14,12 +14,12 @@ public class XDocumentType : XNode
         private string _name;
         private string? _publicId;
         private string? _systemId;
-        private string _internalSubset;
+        private string? _internalSubset;
 
         /// <summary>
         /// Initializes an empty instance of the <see cref="XDocumentType"/> class.
         /// </summary>
-        public XDocumentType(string name, string? publicId, string? systemId, string internalSubset)
+        public XDocumentType(string name, string? publicId, string? systemId, string? internalSubset)
         {
             _name = XmlConvert.VerifyName(name);
             _publicId = publicId;
@@ -53,14 +53,10 @@ internal XDocumentType(XmlReader r)
         /// <summary>
         /// Gets or sets the internal subset for this Document Type Definition (DTD).
         /// </summary>
-        public string InternalSubset
+        public string? InternalSubset
         {
             get
             {
-                // TODO-NULLABLE: As per documentation, this should return string.Empty.
-                // Should we check for null here?
-                // This is also referenced by XNodeReader.Value which overrides XmlReader.Value, which is non-nullable.
-                // There is one case that passes a nullable parameter (XNodeBuilder.WriteDocType), currently we are just asserting that the nullable parameter does not receive null.
                 return _internalSubset;
             }
             set

src/libraries/System.Private.Xml.Linq/src/System/Xml/Linq/XNodeBuilder.cs

@@ -83,7 +83,6 @@ public override void WriteComment(string? text)
 
         public override void WriteDocType(string name, string? pubid, string? sysid, string? subset)
         {
-            Debug.Assert(subset != null);
             AddNode(new XDocumentType(name, pubid, sysid, subset));
         }
 

src/libraries/System.Private.Xml.Linq/src/System/Xml/Linq/XNodeReader.cs

@@ -378,7 +378,7 @@ public override string Value
                         case XmlNodeType.ProcessingInstruction:
                             return ((XProcessingInstruction)o).Data;
                         case XmlNodeType.DocumentType:
-                            return ((XDocumentType)o).InternalSubset;
+                            return ((XDocumentType)o).InternalSubset ?? string.Empty;
                         default:
                             return string.Empty;
                     }
@@ -552,9 +552,11 @@ public override void Close()
             return null;
         }
 
-        // TODO-NULLABLE: decide if base signature should be switched to return string?
         public override string GetAttribute(int index)
         {
+            // https://github.com/dotnet/runtime/issues/44287
+            //     We should replace returning null with ArgumentOutOfRangeException
+            //     In case of not interactive state likely we should throw InvalidOperationException
             if (!IsInteractive)
             {
                 return null!;

src/libraries/System.Private.Xml.Linq/src/System/Xml/XPath/XNodeNavigator.cs

@@ -462,11 +462,12 @@ public override bool MoveToNamespace(string localName)
                 {
                     return false; // backcompat
                 }
-                // TODO-NULLABLE: Unnecessary null check?
+
                 if (localName != null && localName.Length == 0)
                 {
                     localName = "xmlns"; // backcompat
                 }
+
                 XAttribute? a = GetFirstNamespaceDeclarationGlobal(e);
                 while (a != null)
                 {
@@ -478,6 +479,7 @@ public override bool MoveToNamespace(string localName)
                     }
                     a = GetNextNamespaceDeclarationGlobal(a);
                 }
+
                 if (localName == "xml")
                 {
                     _source = GetXmlNamespaceDeclaration();

src/libraries/System.Xml.XDocument/ref/System.Xml.XDocument.cs

@@ -212,9 +212,9 @@ public override void WriteTo(System.Xml.XmlWriter writer) { }
     }
     public partial class XDocumentType : System.Xml.Linq.XNode
     {
-        public XDocumentType(string name, string? publicId, string? systemId, string internalSubset) { }
+        public XDocumentType(string name, string? publicId, string? systemId, string? internalSubset) { }
         public XDocumentType(System.Xml.Linq.XDocumentType other) { }
-        public string InternalSubset { get { throw null; } set { } }
+        public string? InternalSubset { get { throw null; } set { } }
         public string Name { get { throw null; } set { } }
         public override System.Xml.XmlNodeType NodeType { get { throw null; } }
         public string? PublicId { get { throw null; } set { } }