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:/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:/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,22 +951,23 @@ 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;
  OleDbHResult hr;
  hr = rowsetInfo.GetReferencedRowset((IntPtr)ordinal, ref ODB.IID_IRowset, out result);
 
  ProcessResults(hr);
+ // Per docs result can be null only when hr is DB_E_NOTAREFERENCECOLUMN which in most of the cases will cause the exception in ProcessResult
 
  OleDbDataReader? reader = null;
- // TODO: Not sure if GetReferenceRowset above actually returns null, calling code seems to assume it doesn't
+
  if (null != result)
  {
  // only when the first datareader is closed will the connection close
@@ -983,6 +984,7 @@ protected override DbDataReader GetDbDataReader(int ordinal)
  _connection.AddWeakReference(reader, OleDbReferenceCollection.DataReaderTag);
  }
  }
+
  return reader;
  }
 
@@ -1022,8 +1024,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 +2276,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.Data.OleDb/src/UnsafeNativeMethods.cs

@@ -723,7 +723,7 @@ System.Data.OleDb.OleDbHResult GetProperties(
  System.Data.OleDb.OleDbHResult GetReferencedRowset(
  [In] IntPtr iOrdinal,
  [In] ref Guid riid,
- [Out, MarshalAs(UnmanagedType.Interface)] out IRowset ppRowset);
+ [Out, MarshalAs(UnmanagedType.Interface)] out IRowset? ppRowset);
 
  //[PreserveSig]
  //int GetSpecification(/*deleted parameter signature*/);

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:/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

@@ -271,15 +271,13 @@ public static IEnumerable<XElement> Elements<T>(this IEnumerable<T?> source, XNa
  /// for each <see cref="XElement"/> in this <see cref="IEnumerable"/> of <see cref="XElement"/>.
  /// in document order
  /// </returns>
- public static IEnumerable<T> InDocumentOrder<T>(this IEnumerable<T> source) where T : XNode
+ public static IEnumerable<T> InDocumentOrder<T>(this IEnumerable<T> source) where T : XNode?
  {
  if (source == null) throw new ArgumentNullException(nameof(source));
  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
+ private static IEnumerable<T> DocumentOrderIterator<T>(IEnumerable<T> source) where T : XNode?
  {
  int count;
  T[] items = EnumerableHelpers.ToArray(source, out 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

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -19,12 +20,12 @@ public class XDocumentType : XNode
  /// <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;
  _systemId = systemId;
- _internalSubset = internalSubset;
+ _internalSubset = internalSubset ?? string.Empty;
  }
 
  /// <summary>
@@ -53,20 +54,17 @@ internal XDocumentType(XmlReader r)
  /// <summary>
  /// Gets or sets the internal subset for this Document Type Definition (DTD).
  /// </summary>
+ [AllowNull]
  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
  {
  bool notify = NotifyChanging(this, XObjectChangeEventArgs.Value);
- _internalSubset = value;
+ _internalSubset = value ?? string.Empty;
  if (notify) NotifyChanged(this, XObjectChangeEventArgs.Value);
  }
  }
@@ -184,7 +182,7 @@ internal override int GetDeepHashCode()
  return _name.GetHashCode() ^
  (_publicId != null ? _publicId.GetHashCode() : 0) ^
  (_systemId != null ? _systemId.GetHashCode() : 0) ^
- (_internalSubset != null ? _internalSubset.GetHashCode() : 0);
+ _internalSubset.GetHashCode();
  }
  }
 }

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/XNodeDocumentOrderComparer.cs

@@ -13,7 +13,7 @@ namespace System.Xml.Linq
  /// </summary>
  public sealed class XNodeDocumentOrderComparer :
  IComparer,
- IComparer<XNode>
+ IComparer<XNode?>
  {
  /// <summary>
  /// Compares two nodes to determine their relative XML document order.

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

@@ -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:/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

@@ -22,7 +22,7 @@ public static partial class Extensions
  public static System.Collections.Generic.IEnumerable<System.Xml.Linq.XElement> Descendants<T>(this System.Collections.Generic.IEnumerable<T?> source, System.Xml.Linq.XName? name) where T : System.Xml.Linq.XContainer { throw null; }
  public static System.Collections.Generic.IEnumerable<System.Xml.Linq.XElement> Elements<T>(this System.Collections.Generic.IEnumerable<T?> source) where T : System.Xml.Linq.XContainer { throw null; }
  public static System.Collections.Generic.IEnumerable<System.Xml.Linq.XElement> Elements<T>(this System.Collections.Generic.IEnumerable<T?> source, System.Xml.Linq.XName? name) where T : System.Xml.Linq.XContainer { throw null; }
- public static System.Collections.Generic.IEnumerable<T> InDocumentOrder<T>(this System.Collections.Generic.IEnumerable<T> source) where T : System.Xml.Linq.XNode { throw null; }
+ public static System.Collections.Generic.IEnumerable<T> InDocumentOrder<T>(this System.Collections.Generic.IEnumerable<T> source) where T : System.Xml.Linq.XNode? { throw null; }
  public static System.Collections.Generic.IEnumerable<System.Xml.Linq.XNode> Nodes<T>(this System.Collections.Generic.IEnumerable<T?> source) where T : System.Xml.Linq.XContainer { throw null; }
  public static void Remove(this System.Collections.Generic.IEnumerable<System.Xml.Linq.XAttribute?> source) { }
  public static void Remove<T>(this System.Collections.Generic.IEnumerable<T?> source) where T : System.Xml.Linq.XNode { }
@@ -212,8 +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) { }
+ [System.Diagnostics.CodeAnalysis.AllowNull]
  public string InternalSubset { get { throw null; } set { } }
  public string Name { get { throw null; } set { } }
  public override System.Xml.XmlNodeType NodeType { get { throw null; } }
@@ -425,7 +426,7 @@ public void ReplaceWith(params object?[] content) { }
  public abstract void WriteTo(System.Xml.XmlWriter writer);
  public abstract System.Threading.Tasks.Task WriteToAsync(System.Xml.XmlWriter writer, System.Threading.CancellationToken cancellationToken);
  }
- public sealed partial class XNodeDocumentOrderComparer : System.Collections.Generic.IComparer<System.Xml.Linq.XNode>, System.Collections.IComparer
+ public sealed partial class XNodeDocumentOrderComparer : System.Collections.Generic.IComparer<System.Xml.Linq.XNode?>, System.Collections.IComparer
  {
  public XNodeDocumentOrderComparer() { }
  public int Compare(System.Xml.Linq.XNode? x, System.Xml.Linq.XNode? y) { throw null; }