Align comments between netfx and netcore codebases (#2966)

Author: MichelZ

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs

@@ -5,6 +5,7 @@
 using System;
 using System.Collections;
 using System.Collections.ObjectModel;
+using System.ComponentModel;
 using System.Data;
 using System.Data.Common;
 using System.Data.SqlTypes;
@@ -917,7 +918,7 @@ public override void Close()
                         {
                             if (_stateObj != null)
                             {  // reader not closed while we waited for the lock
-                               // TryCloseInternal will clear out the snapshot when it is done
+                                // TryCloseInternal will clear out the snapshot when it is done
                                 if (_snapshot != null)
                                 {
 #if DEBUG
@@ -2045,6 +2046,7 @@ override public TextReader GetTextReader(int i)
         }
 
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/GetChar/*' />
+        [EditorBrowsableAttribute(EditorBrowsableState.Never)]
         override public char GetChar(int i)
         {
             throw ADP.NotSupported();
@@ -3020,7 +3022,7 @@ private T GetFieldValueFromSqlBufferInternal<T>(SqlBuffer data, _SqlMetaData met
                 // Stream is only for Binary, Image, VarBinary, Udt, Xml and Timestamp(RowVersion) types
                 MetaType metaType = metaData.metaType;
                 if (
-                    (!metaType.IsBinType || metaType.SqlDbType == SqlDbType.Timestamp) && 
+                    (!metaType.IsBinType || metaType.SqlDbType == SqlDbType.Timestamp) &&
                     metaType.SqlDbType != SqlDbType.Variant
                 )
                 {
@@ -4360,7 +4362,7 @@ internal TdsOperationStatus TrySetMetaData(_SqlMetaDataSet metaData, bool moreIn
                         // simply rip the order token off the wire
                         if (b == TdsEnums.SQLORDER)
                         {
-                            //  same logic as SetAltMetaDataSet
+                            // same logic as SetAltMetaDataSet
                             result = _parser.TryRun(RunBehavior.ReturnImmediately, null, null, null, _stateObj, out _);
                             if (result != TdsOperationStatus.Done)
                             {
@@ -5381,7 +5383,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi
         }
 
 #endif
-        
+
         internal abstract class SqlDataReaderBaseAsyncCallContext<T> : AAsyncBaseCallContext<SqlDataReader, T>
         {
             internal static readonly Action<Task<T>, object> s_completeCallback = CompleteAsyncCallCallback;

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs

@@ -76,10 +76,6 @@ internal class SharedState
         private static int s_objectTypeCount; // EventSource Counter
         internal readonly int ObjectID = Interlocked.Increment(ref s_objectTypeCount);
 
-        // context
-        // undone: we may still want to do this...it's nice to pass in an lpvoid (essentially) and just have the reader keep the state
-        // private object _context = null; // this is never looked at by the stream object.  It is used by upper layers who wish
-        // to remain stateless
 
         // metadata (no explicit table, use 'Table')
         private MultiPartTableName[] _tableNames = null;
@@ -353,7 +349,7 @@ internal virtual SmiExtendedMetaData[] GetInternalSmiMetaData()
                         }
                         else if (SqlDbType.Udt == colMetaData.type)
                         {
-                            Connection.CheckGetExtendedUDTInfo(colMetaData, true);    // SQLBUDT #370593 ensure that colMetaData.udtType is set
+                            Connection.CheckGetExtendedUDTInfo(colMetaData, true); // Ensure that colMetaData.udtType is set
 
                             typeSpecificNamePart1 = colMetaData.udt?.DatabaseName;
                             typeSpecificNamePart2 = colMetaData.udt?.SchemaName;
@@ -500,9 +496,6 @@ internal void Bind(TdsParserStateObject stateObj)
             _defaultLCID = _parser.DefaultLCID;
         }
 
-        // Fills in a schema table with meta data information.  This function should only really be called by
-        // UNDONE: need a way to refresh the table with more information as more data comes online for browse info like
-        // table names and key information
 #if !NETFRAMEWORK
         [SuppressMessage("ReflectionAnalysis", "IL2111",
                    Justification = "System.Type.TypeInitializer would not be used in dataType and providerSpecificDataType columns.")]
@@ -1193,7 +1186,7 @@ private TdsOperationStatus TryCloseInternal(bool closeReader)
                         CleanupAfterAsyncInvocationInternal(stateObj);
                     }
 
-                    // SQLBUDT #284712 - Note the order here is extremely important:
+                    // Note the order here is extremely important:
                     //
                     // (1) First, we remove the reader from the reference collection
                     //     to prevent it from being forced closed by the parser if
@@ -1271,6 +1264,7 @@ private TdsOperationStatus TryCloseInternal(bool closeReader)
                         }
                         throw;
                     }
+                    // DO NOT USE stateObj after this point - it has been returned to the TdsParser's session pool and potentially handed out to another thread
 
                     // do not retry here
                     result = TrySetMetaData(null, false);
@@ -1657,7 +1651,7 @@ override public int GetOrdinal(string name)
                     CheckMetaDataIsReady();
                     _fieldNameLookup = new FieldNameLookup(this, _defaultLCID);
                 }
-                return _fieldNameLookup.GetOrdinal(name); // MDAC 71470
+                return _fieldNameLookup.GetOrdinal(name);
             }
             finally
             {
@@ -2344,7 +2338,7 @@ override public TextReader GetTextReader(int i)
         }
 
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/GetChar/*' />
-        [EditorBrowsableAttribute(EditorBrowsableState.Never)] // MDAC 69508
+        [EditorBrowsableAttribute(EditorBrowsableState.Never)]
         override public char GetChar(int i)
         {
             throw ADP.NotSupported();
@@ -2423,7 +2417,6 @@ override public long GetChars(int i, long dataIndex, char[] buffer, int bufferIn
                         }
                         catch (Exception ex)
                         {
-                            // Dev11 Bug #315513: Exception type breaking change from 4.0 RTM when calling GetChars on null xml
                             // We need to wrap all exceptions inside a TargetInvocationException to simulate calling CreateSqlReader via MethodInfo.Invoke
                             if (ADP.IsCatchableExceptionType(ex))
                             {
@@ -2504,7 +2497,6 @@ override public long GetChars(int i, long dataIndex, char[] buffer, int bufferIn
                 }
                 catch (Exception e)
                 {
-                    // UNDONE - should not be catching all exceptions!!!
                     if (!ADP.IsCatchableExceptionType(e))
                     {
                         throw;
@@ -2895,7 +2887,6 @@ virtual public SqlSingle GetSqlSingle(int i)
         }
 
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/GetSqlString/*' />
-        // UNDONE: need non-unicode SqlString support
         virtual public SqlString GetSqlString(int i)
         {
             ReadColumn(i);
@@ -2985,8 +2976,6 @@ private object GetSqlValueInternal(int i)
         //       Always make sure to take reference copies of anything set to null in TryCloseInternal()
         private object GetSqlValueFromSqlBufferInternal(SqlBuffer data, _SqlMetaData metaData)
         {
-            // Dev11 Bug #336820, Dev10 Bug #479607 (SqlClient: IsDBNull always returns false for timestamp datatype)
-            // Due to a bug in TdsParser.GetNullSqlValue, Timestamps' IsNull is not correctly set - so we need to bypass the following check
             Debug.Assert(!data.IsEmpty || data.IsNull || metaData.type == SqlDbType.Timestamp, "Data has been read, but the buffer is empty");
 
             // Convert 2008 types to string
@@ -3177,8 +3166,6 @@ private object GetValueInternal(int i)
         //       Always make sure to take reference copies of anything set to null in TryCloseInternal()
         private object GetValueFromSqlBufferInternal(SqlBuffer data, _SqlMetaData metaData)
         {
-            // Dev11 Bug #336820, Dev10 Bug #479607 (SqlClient: IsDBNull always returns false for timestamp datatype)
-            // Due to a bug in TdsParser.GetNullSqlValue, Timestamps' IsNull is not correctly set - so we need to bypass the following check
             Debug.Assert(!data.IsEmpty || data.IsNull || metaData.type == SqlDbType.Timestamp, "Data has been read, but the buffer is empty");
 
             if (_typeSystem <= SqlConnectionString.TypeSystem.SQLServer2005 && metaData.Is2008DateTimeType)
@@ -3503,7 +3490,7 @@ override public int GetValues(object[] values)
                         _data[i].Clear();
                         if (fieldIndex > i && fieldIndex > 0)
                         {
-                            // if we jumped an index forward because of a hidden column see if the buffer before the 
+                            // if we jumped an index forward because of a hidden column see if the buffer before the
                             // current one was populated by the seek forward and clear it if it was
                             _data[fieldIndex - 1].Clear();
                         }
@@ -3595,10 +3582,6 @@ private TdsOperationStatus TryHasMoreResults(out bool moreResults)
                             // always happens if there is a row following an altrow
                             moreResults = true;
                             return TdsOperationStatus.Done;
-
-                        // VSTFDEVDIV 926281: DONEINPROC case is missing here; we have decided to reject this bug as it would result in breaking change
-                        // from VS2008 RTM/SP1 and Dev10 RTM. See the bug for more details.
-                        // case TdsEnums.DONEINPROC:
                         case TdsEnums.SQLDONE:
                             Debug.Assert(_altRowStatus == ALTROWSTATUS.Done || _altRowStatus == ALTROWSTATUS.Null, "invalid AltRowStatus");
                             _altRowStatus = ALTROWSTATUS.Null;
@@ -3628,8 +3611,6 @@ private TdsOperationStatus TryHasMoreResults(out bool moreResults)
                             return TdsOperationStatus.Done;
                     }
 
-                    // Dev11 Bug 316483: Stuck at SqlDataReader::TryHasMoreResults using MARS
-                    // http://vstfdevdiv:8080/web/wi.aspx?pcguid=22f9acc9-569a-41ff-b6ac-fac1b6370209&id=316483
                     // TryRun() will immediately return if the TdsParser is closed/broken, causing us to enter an infinite loop
                     // Instead, we will throw a closed connection exception
                     if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed)
@@ -3674,19 +3655,6 @@ private TdsOperationStatus TryHasMoreRows(out bool moreRows)
                 if (_stateObj.HasPendingData)
                 {
                     // Consume error's, info's, done's on HasMoreRows, so user obtains error on Read.
-                    // Previous bug where Read() would return false with error on the wire in the case
-                    // of metadata and error immediately following.  See MDAC 78285 and 75225.
-
-                    // BUGBUG - currently in V1 the if (_parser.PendingData) does not
-                    // exist, so under certain conditions HasMoreRows can timeout.  However,
-                    // this should only occur when executing as SqlBatch and returning a reader.
-                    // Updated - SQL Bug: 20001249
-                    // Modifed while loop and added parsedDoneToken, to revert a regression from everettfs.
-                    // "Error Exceptions" are now only thown in read when a error occures in the exception, otherwise the exception will be thrown on the call to get the next result set.
-                    // resultset.
-
-                    // process any done, doneproc and doneinproc token streams and
-                    // any order, error or info token preceeding the first done, doneproc or doneinproc token stream
                     byte b;
                     TdsOperationStatus result = _stateObj.TryPeekByte(out b);
                     if (result != TdsOperationStatus.Done)
@@ -3713,8 +3681,6 @@ private TdsOperationStatus TryHasMoreRows(out bool moreRows)
                             ParsedDoneToken = true;
                         }
 
-                        // Dev11 Bug 316483: Stuck at SqlDataReader::TryHasMoreResults when using MARS
-                        // http://vstfdevdiv:8080/web/wi.aspx?pcguid=22f9acc9-569a-41ff-b6ac-fac1b6370209&id=316483
                         // TryRun() will immediately return if the TdsParser is closed/broken, causing us to enter an infinite loop
                         // Instead, we will throw a closed connection exception
                         if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed)
@@ -4895,25 +4861,19 @@ internal TdsOperationStatus TrySetMetaData(_SqlMetaDataSet metaData, bool moreIn
 
                     if (_parser != null)
                     { // There is a valid case where parser is null
-                        // Peek, and if row token present, set _hasRows true since there is a
-                        // row in the result
+                      // Peek, and if row token present, set _hasRows true since there is a
+                      // row in the result
                         byte b;
                         TdsOperationStatus result = _stateObj.TryPeekByte(out b);
                         if (result != TdsOperationStatus.Done)
                         {
                             return result;
                         }
 
-                        // UNDONE - should we be consuming tokens here??? Maybe we should be calling HasMoreRows?
-                        // Would that have other side effects?
-
                         // simply rip the order token off the wire
                         if (b == TdsEnums.SQLORDER)
-                        {                     
+                        {
                             // same logic as SetAltMetaDataSet
-                            // Devnote: That's not the right place to process TDS
-                            // Can this result in Reentrance to Run?
-
                             result = _parser.TryRun(RunBehavior.ReturnImmediately, null, null, null, _stateObj, out _);
                             if (result != TdsOperationStatus.Done)
                             {
@@ -5151,7 +5111,7 @@ private static Task<bool> NextResultAsyncExecute(Task task, object state)
 
             if (context.Reader.TryNextResult(out bool more) == TdsOperationStatus.Done)
             {
-                // completed 
+                // completed
                 return more ? ADP.TrueTask : ADP.FalseTask;
             }
 
@@ -5573,7 +5533,7 @@ private static Task<bool> ReadAsyncExecute(Task task, object state)
                 // If there are no more rows, or this is Sequential Access, then we are done
                 if (!hasMoreData || (reader._commandBehavior & CommandBehavior.SequentialAccess) == CommandBehavior.SequentialAccess)
                 {
-                    // completed 
+                    // completed
                     return hasMoreData ? ADP.TrueTask : ADP.FalseTask;
                 }
                 else
@@ -5590,7 +5550,7 @@ private static Task<bool> ReadAsyncExecute(Task task, object state)
                     TdsOperationStatus result = reader.TryReadColumn(reader._metaData.Length - 1, true);
                     if (result == TdsOperationStatus.Done)
                     {
-                        // completed 
+                        // completed
                         return ADP.TrueTask;
                     }
                 }