Address nullable feedback for System.Diagnostics.Tracing and StackTrace (#25303)
authorSantiago Fernandez Madero <safern@microsoft.com>
Fri, 21 Jun 2019 17:54:36 +0000 (10:54 -0700)
committerGitHub <noreply@github.com>
Fri, 21 Jun 2019 17:54:36 +0000 (10:54 -0700)
* Address nullable feedback for System.Diagnostics.Tracing and StackTrace

* Disable corefx tests that need to be updated

src/System.Private.CoreLib/shared/System/Diagnostics/StackTrace.cs
src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/DiagnosticCounter.cs
src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventProvider.cs
src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventSource.cs
src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs
tests/CoreFX/CoreFX.issues.rsp

index 5a3af0c..8365c48 100644 (file)
@@ -159,10 +159,10 @@ namespace System.Diagnostics
         /// The nth element of this array is the same as GetFrame(n).
         /// The length of the array is the same as FrameCount.
         /// </summary>
-        public virtual StackFrame?[]? GetFrames()
+        public virtual StackFrame?[] GetFrames()
         {
             if (_stackFrames == null || _numOfFrames <= 0)
-                return null;
+                return Array.Empty<StackFrame>();
 
             // We have to return a subset of the array. Unfortunately this
             // means we have to allocate a new array and copy over.
index 666545a..c8c9c3e 100644 (file)
@@ -67,11 +67,11 @@ namespace System.Diagnostics.Tracing
         /// <summary>
         /// Adds a key-value metadata to the EventCounter that will be included as a part of the payload
         /// </summary>
-        public void AddMetadata(string key, string value)
+        public void AddMetadata(string key, string? value)
         {
             lock (this)
             {
-                _metadata ??= new Dictionary<string, string>();
+                _metadata ??= new Dictionary<string, string?>();
                 _metadata.Add(key, value);
             }
         }
@@ -107,7 +107,7 @@ namespace System.Diagnostics.Tracing
         #region private implementation
 
         private CounterGroup _group;
-        private Dictionary<string, string>? _metadata;
+        private Dictionary<string, string?>? _metadata;
 
         internal abstract void WritePayload(float intervalSec, int pollingIntervalMillisec);
 
@@ -127,13 +127,13 @@ namespace System.Diagnostics.Tracing
 
             // The dictionary is only initialized to non-null when there's metadata to add, and no items
             // are ever removed, so if the dictionary is non-null, there must also be at least one element.
-            Dictionary<string, string>.Enumerator enumerator = _metadata.GetEnumerator();
+            Dictionary<string, string?>.Enumerator enumerator = _metadata.GetEnumerator();
             Debug.Assert(_metadata.Count > 0);
             bool gotOne = enumerator.MoveNext();
             Debug.Assert(gotOne);
 
             // If there's only one element, just concat a string for it.
-            KeyValuePair<string, string> current = enumerator.Current;
+            KeyValuePair<string, string?> current = enumerator.Current;
             if (!enumerator.MoveNext())
             {
                 return current.Key + ":" + current.Value;
index fde1b47..7322209 100644 (file)
@@ -278,7 +278,7 @@ namespace System.Diagnostics.Tracing
             try
             {
                 ControllerCommand command = ControllerCommand.Update;
-                IDictionary<string, string>? args = null;
+                IDictionary<string, string?>? args = null;
                 bool skipFinalOnControllerCommand = false;
                 if (controlCode == Interop.Advapi32.EVENT_CONTROL_CODE_ENABLE_PROVIDER)
                 {
@@ -321,7 +321,7 @@ namespace System.Diagnostics.Tracing
                         if (bEnabling &&
                             GetDataFromController(etwSessionId, filterData, out command, out data, out keyIndex))
                         {
-                            args = new Dictionary<string, string>(4);
+                            args = new Dictionary<string, string?>(4);
                             Debug.Assert(data != null);
                             while (keyIndex < data.Length)
                             {
@@ -368,7 +368,7 @@ namespace System.Diagnostics.Tracing
         }
 
         // New in CLR4.0
-        protected virtual void OnControllerCommand(ControllerCommand command, IDictionary<string, string>? arguments, int sessionId, int etwSessionId) { }
+        protected virtual void OnControllerCommand(ControllerCommand command, IDictionary<string, string?>? arguments, int sessionId, int etwSessionId) { }
         protected EventLevel Level { get { return (EventLevel)m_level; } set { m_level = (byte)value; } }
         protected EventKeywords MatchAnyKeyword { get { return (EventKeywords)m_anyKeywordMask; } set { m_anyKeywordMask = unchecked((long)value); } }
         protected EventKeywords MatchAllKeyword { get { return (EventKeywords)m_allKeywordMask; } set { m_allKeywordMask = unchecked((long)value); } }
@@ -729,7 +729,7 @@ namespace System.Diagnostics.Tracing
         // <UsesUnsafeCode Name="Parameter dataDescriptor of type: EventData*" />
         // <UsesUnsafeCode Name="Parameter dataBuffer of type: Byte*" />
         // </SecurityKernel>
-        private static unsafe object? EncodeObject(ref object data, ref EventData* dataDescriptor, ref byte* dataBuffer, ref uint totalEventSize)
+        private static unsafe object? EncodeObject(ref object? data, ref EventData* dataDescriptor, ref byte* dataBuffer, ref uint totalEventSize)
         /*++
 
         Routine Description:
@@ -970,7 +970,7 @@ namespace System.Diagnostics.Tracing
         // </SecurityKernel>
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity", Justification = "Performance-critical code")]
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1045:DoNotPassTypesByReference")]
-        internal unsafe bool WriteEvent(ref EventDescriptor eventDescriptor, IntPtr eventHandle, Guid* activityID, Guid* childActivityID, params object[] eventPayload)
+        internal unsafe bool WriteEvent(ref EventDescriptor eventDescriptor, IntPtr eventHandle, Guid* activityID, Guid* childActivityID, params object?[] eventPayload)
         {
             WriteEventErrorCode status = WriteEventErrorCode.NoError;
 
index d06e292..90edaed 100644 (file)
@@ -420,7 +420,7 @@ namespace System.Diagnostics.Tracing
         /// <param name="eventSource">The instance of EventSource to send the command to</param>
         /// <param name="command">A positive user-defined EventCommand, or EventCommand.SendManifest</param>
         /// <param name="commandArguments">A set of (name-argument, value-argument) pairs associated with the command</param>
-        public static void SendCommand(EventSource eventSource, EventCommand command, IDictionary<string, string>? commandArguments)
+        public static void SendCommand(EventSource eventSource, EventCommand command, IDictionary<string, string?>? commandArguments)
         {
             if (eventSource == null)
                 throw new ArgumentNullException(nameof(eventSource));
@@ -1305,7 +1305,7 @@ namespace System.Diagnostics.Tracing
         /// check so that the varargs call is not made when the EventSource is not active.  
         /// </summary>
         [SuppressMessage("Microsoft.Concurrency", "CA8001", Justification = "This does not need to be correct when racing with other threads")]
-        protected unsafe void WriteEvent(int eventId, params object[] args)
+        protected unsafe void WriteEvent(int eventId, params object?[] args)
         {
             WriteEventVarargs(eventId, null, args);
         }
@@ -1318,7 +1318,7 @@ namespace System.Diagnostics.Tracing
         /// particular method signature. Even if you use this for rare events, this call should be guarded by an <see cref="IsEnabled()"/>
         /// check so that the varargs call is not made when the EventSource is not active.
         /// </summary>
-        protected unsafe void WriteEventWithRelatedActivityId(int eventId, Guid relatedActivityId, params object[] args)
+        protected unsafe void WriteEventWithRelatedActivityId(int eventId, Guid relatedActivityId, params object?[] args)
         {
             WriteEventVarargs(eventId, &relatedActivityId, args);
         }
@@ -1899,7 +1899,7 @@ namespace System.Diagnostics.Tracing
             return dispatcher;
         }
 
-        private unsafe void WriteEventVarargs(int eventId, Guid* childActivityID, object[] args)
+        private unsafe void WriteEventVarargs(int eventId, Guid* childActivityID, object?[] args)
         {
             if (m_eventSourceEnabled)
             {
@@ -2052,7 +2052,7 @@ namespace System.Diagnostics.Tracing
         /// </summary>
         /// <param name="infos"></param>
         /// <param name="args"></param>
-        private void LogEventArgsMismatches(ParameterInfo[] infos, object[] args)
+        private void LogEventArgsMismatches(ParameterInfo[] infos, object?[] args)
         {
 #if (!ES_BUILD_PCL && !ES_BUILD_PN)
             // It would be nice to have this on PCL builds, but it would be pointless since there isn't support for 
@@ -2067,7 +2067,7 @@ namespace System.Diagnostics.Tracing
                 // Checking to see if the Parameter types (from the Event method) match the supplied argument types.
                 // Fail if one of two things hold : either the argument type is not equal to the parameter type, or the 
                 // argument is null and the parameter type is non-nullable.
-                if ((args[i] != null && (args[i].GetType() != pType))
+                if ((args[i] != null && (args[i]!.GetType() != pType)) // TODO-NULLABLE: Indexer nullability tracked (https://github.com/dotnet/roslyn/issues/34644)
                     || (args[i] == null && (!(pType.IsGenericType && pType.GetGenericTypeDefinition() == typeof(Nullable<>))))
                     )
                 {
@@ -2417,7 +2417,7 @@ namespace System.Diagnostics.Tracing
                 this.m_eventSource = eventSource;
                 this.m_eventProviderType = providerType;
             }
-            protected override void OnControllerCommand(ControllerCommand command, IDictionary<string, string>? arguments,
+            protected override void OnControllerCommand(ControllerCommand command, IDictionary<string, string?>? arguments,
                                                               int perEventSourceSessionId, int etwSessionId)
             {
                 // We use null to represent the ETW EventListener.  
@@ -2641,7 +2641,7 @@ namespace System.Diagnostics.Tracing
         internal void SendCommand(EventListener? listener, EventProviderType eventProviderType, int perEventSourceSessionId, int etwSessionId,
                                   EventCommand command, bool enable,
                                   EventLevel level, EventKeywords matchAnyKeyword,
-                                  IDictionary<string, string>? commandArguments)
+                                  IDictionary<string, string?>? commandArguments)
         {
             var commandArgs = new EventCommandEventArgs(command, commandArguments, this, listener, eventProviderType, perEventSourceSessionId, etwSessionId, enable, level, matchAnyKeyword);
             lock (EventListener.EventListenersLock)
@@ -2706,7 +2706,7 @@ namespace System.Diagnostics.Tracing
                 }
 
                 if (commandArgs.Arguments == null)
-                    commandArgs.Arguments = new Dictionary<string, string>();
+                    commandArgs.Arguments = new Dictionary<string, string?>();
 
                 if (commandArgs.Command == EventCommand.Update)
                 {
@@ -4203,7 +4203,7 @@ namespace System.Diagnostics.Tracing
         /// 
         /// This call never has an effect on other EventListeners.
         /// </summary>       
-        public void EnableEvents(EventSource eventSource, EventLevel level, EventKeywords matchAnyKeyword, IDictionary<string, string>? arguments)
+        public void EnableEvents(EventSource eventSource, EventLevel level, EventKeywords matchAnyKeyword, IDictionary<string, string?>? arguments)
         {
             if (eventSource == null)
             {
@@ -4630,7 +4630,7 @@ namespace System.Diagnostics.Tracing
         /// <summary>
         /// Gets the arguments for the callback.
         /// </summary>
-        public IDictionary<string, string>? Arguments { get; internal set; }
+        public IDictionary<string, string?>? Arguments { get; internal set; }
 
         /// <summary>
         /// Enables the event that has the specified identifier.
@@ -4658,7 +4658,7 @@ namespace System.Diagnostics.Tracing
 
 #region private
 
-        internal EventCommandEventArgs(EventCommand command, IDictionary<string, string>? arguments, EventSource eventSource,
+        internal EventCommandEventArgs(EventCommand command, IDictionary<string, string?>? arguments, EventSource eventSource,
             EventListener? listener, EventProviderType eventProviderType, int perEventSourceSessionId, int etwSessionId, bool enable, EventLevel level, EventKeywords matchAnyKeyword)
         {
             this.Command = command;
index b737da4..0aedc3a 100644 (file)
@@ -110,14 +110,9 @@ namespace System.Diagnostics.Tracing
         /// Writes an event with no fields and default options.
         /// (Native API: EventWriteTransfer)
         /// </summary>
-        /// <param name="eventName">The name of the event. Must not be null.</param>
-        public unsafe void Write(string eventName)
+        /// <param name="eventName">The name of the event.</param>
+        public unsafe void Write(string? eventName)
         {
-            if (eventName == null)
-            {
-                throw new ArgumentNullException(nameof(eventName));
-            }
-
             if (!this.IsEnabled())
             {
                 return;
@@ -131,18 +126,13 @@ namespace System.Diagnostics.Tracing
         /// Writes an event with no fields.
         /// (Native API: EventWriteTransfer)
         /// </summary>
-        /// <param name="eventName">The name of the event. Must not be null.</param>
+        /// <param name="eventName">The name of the event.</param>
         /// <param name="options">
         /// Options for the event, such as the level, keywords, and opcode. Unset
         /// options will be set to default values.
         /// </param>
-        public unsafe void Write(string eventName, EventSourceOptions options)
+        public unsafe void Write(string? eventName, EventSourceOptions options)
         {
-            if (eventName == null)
-            {
-                throw new ArgumentNullException(nameof(eventName));
-            }
-
             if (!this.IsEnabled())
             {
                 return;
@@ -351,7 +341,7 @@ namespace System.Diagnostics.Tracing
             TraceLoggingEventTypes eventTypes,
              Guid* activityID,
              Guid* childActivityID,
-            params object[] values)
+            params object?[] values)
         {
             if (!this.IsEnabled())
             {
@@ -411,7 +401,7 @@ namespace System.Diagnostics.Tracing
             TraceLoggingEventTypes eventTypes,
             Guid* activityID,
             Guid* childActivityID,
-            params object[] values)
+            params object?[] values)
         {
 #if FEATURE_MANAGED_ETW
             int identity = 0;
index b6f7abe..e7b11ab 100644 (file)
 
 # requires corefx test updates: https://github.com/dotnet/corefx/pull/38692
 -nomethod System.Tests.StringTests.CasingAsSpan_NullCulture_ThrowsArgumentNullException
+
+# requires corefx test updates: https://github.com/dotnet/corefx/pull/38747
+-nomethod System.Diagnostics.Tests.StackTraceTests.Ctor_EmptyException_FNeedFileInfo
+-nomethod System.Diagnostics.Tests.StackTraceTests.Ctor_EmptyException_GetFramesReturnsNull
+-nomethod System.Diagnostics.Tests.StackTraceTests.Ctor_EmptyException_SkipFrames
+-nomethod System.Diagnostics.Tests.StackTraceTests.Ctor_EmptyException_SkipFrames_FNeedFileInfo
+-nomethod System.Diagnostics.Tests.StackTraceTests.Ctor_Exception_LargeSkipFrames
+-nomethod System.Diagnostics.Tests.StackTraceTests.Ctor_Exception_LargeSkipFrames_FNeedFileInfo
+-nomethod System.Diagnostics.Tests.StackTraceTests.Ctor_LargeSkipFrames_GetFramesReturnsNull
+-nomethod System.Diagnostics.Tests.StackTraceTests.Ctor_LargeSkipFramesFNeedFileInfo_GetFramesReturnsNull