Enable analyzers for mono/netcore (#34052)

This is mostly just running auto-fixers for the various violated rules. In just a few places, I manually intervened.

* Locally disable pinvoke analyzer pending #33867

Co-authored-by: Dan Moseley <danmose@microsoft.com>

src/libraries/System.Private.CoreLib/src/System/String.cs

@@ -49,10 +49,11 @@ public sealed partial class String : IComparable, IEnumerable, IConvertible, IEn
         [PreserveDependency("Ctor(System.Char[])", "System.String")]
         public extern String(char[]? value);
 
+        private
 #if !CORECLR
         static
 #endif
-        private string Ctor(char[]? value)
+        string Ctor(char[]? value)
         {
             if (value == null || value.Length == 0)
                 return Empty;
@@ -71,10 +72,11 @@ private string Ctor(char[]? value)
         [PreserveDependency("Ctor(System.Char[],System.Int32,System.Int32)", "System.String")]
         public extern String(char[] value, int startIndex, int length);
 
+        private
 #if !CORECLR
         static
 #endif
-        private string Ctor(char[] value, int startIndex, int length)
+        string Ctor(char[] value, int startIndex, int length)
         {
             if (value == null)
                 throw new ArgumentNullException(nameof(value));
@@ -106,10 +108,11 @@ private string Ctor(char[] value, int startIndex, int length)
         [PreserveDependency("Ctor(System.Char*)", "System.String")]
         public extern unsafe String(char* value);
 
+        private
 #if !CORECLR
         static
 #endif
-        private unsafe string Ctor(char* ptr)
+        unsafe string Ctor(char* ptr)
         {
             if (ptr == null)
                 return Empty;
@@ -133,10 +136,11 @@ private unsafe string Ctor(char* ptr)
         [PreserveDependency("Ctor(System.Char*,System.Int32,System.Int32)", "System.String")]
         public extern unsafe String(char* value, int startIndex, int length);
 
+        private
 #if !CORECLR
         static
 #endif
-        private unsafe string Ctor(char* ptr, int startIndex, int length)
+        unsafe string Ctor(char* ptr, int startIndex, int length)
         {
             if (length < 0)
                 throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NegativeLength);
@@ -171,10 +175,11 @@ private unsafe string Ctor(char* ptr, int startIndex, int length)
         [PreserveDependency("Ctor(System.SByte*)", "System.String")]
         public extern unsafe String(sbyte* value);
 
+        private
 #if !CORECLR
         static
 #endif
-        private unsafe string Ctor(sbyte* value)
+        unsafe string Ctor(sbyte* value)
         {
             byte* pb = (byte*)value;
             if (pb == null)
@@ -190,10 +195,11 @@ private unsafe string Ctor(sbyte* value)
         [PreserveDependency("Ctor(System.SByte*,System.Int32,System.Int32)", "System.String")]
         public extern unsafe String(sbyte* value, int startIndex, int length);
 
+        private
 #if !CORECLR
         static
 #endif
-        private unsafe string Ctor(sbyte* value, int startIndex, int length)
+        unsafe string Ctor(sbyte* value, int startIndex, int length)
         {
             if (startIndex < 0)
                 throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_StartIndex);
@@ -250,10 +256,11 @@ private static unsafe string CreateStringForSByteConstructor(byte* pb, int numBy
         [PreserveDependency("Ctor(System.SByte*,System.Int32,System.Int32,System.Text.Encoding)", "System.String")]
         public extern unsafe String(sbyte* value, int startIndex, int length, Encoding enc);
 
+        private
 #if !CORECLR
         static
 #endif
-        private unsafe string Ctor(sbyte* value, int startIndex, int length, Encoding? enc)
+        unsafe string Ctor(sbyte* value, int startIndex, int length, Encoding? enc)
         {
             if (enc == null)
                 return new string(value, startIndex, length);
@@ -285,10 +292,11 @@ private unsafe string Ctor(sbyte* value, int startIndex, int length, Encoding? e
         [PreserveDependency("Ctor(System.Char,System.Int32)", "System.String")]
         public extern String(char c, int count);
 
+        private
 #if !CORECLR
         static
 #endif
-        private string Ctor(char c, int count)
+        string Ctor(char c, int count)
         {
             if (count <= 0)
             {
@@ -335,10 +343,11 @@ private string Ctor(char c, int count)
         [PreserveDependency("Ctor(System.ReadOnlySpan`1<System.Char>)", "System.String")]
         public extern String(ReadOnlySpan<char> value);
 
+        private
 #if !CORECLR
         static
 #endif
-        private unsafe string Ctor(ReadOnlySpan<char> value)
+        unsafe string Ctor(ReadOnlySpan<char> value)
         {
             if (value.Length == 0)
                 return Empty;

src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelLifoSemaphore.Windows.cs

@@ -57,7 +57,7 @@ public void ReleaseCore(int count)
 
             for (int i = 0; i < count; i++)
             {
-                if(!Interop.Kernel32.PostQueuedCompletionStatus(_completionPort, 1, UIntPtr.Zero, IntPtr.Zero))
+                if (!Interop.Kernel32.PostQueuedCompletionStatus(_completionPort, 1, UIntPtr.Zero, IntPtr.Zero))
                 {
                     var lastError = Marshal.GetLastWin32Error();
                     var exception = new OutOfMemoryException();

src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelLifoSemaphore.cs

@@ -27,7 +27,7 @@ public LowLevelLifoSemaphore(int initialSignalCount, int maximumSignalCount, int
             Debug.Assert(maximumSignalCount > 0);
             Debug.Assert(spinCount >= 0);
 
-            _separated = new CacheLineSeparatedCounts();
+            _separated = default;
             _separated._counts._signalCount = (uint)initialSignalCount;
             _maximumSignalCount = maximumSignalCount;
             _spinCount = spinCount;
@@ -204,7 +204,7 @@ private bool WaitForSignal(int timeoutMs)
                 {
                     // Unregister the waiter. The wait subsystem used above guarantees that a thread that wakes due to a timeout does
                     // not observe a signal to the object being waited upon.
-                    Counts toSubtract = new Counts();
+                    Counts toSubtract = default;
                     toSubtract._waiterCount++;
                     Counts newCounts = _separated._counts.Subtract(toSubtract);
                     Debug.Assert(newCounts._waiterCount != ushort.MaxValue); // Check for underflow

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.GateThread.cs

@@ -16,17 +16,15 @@ private static class GateThread
 
             private static int s_runningState;
 
-            private static AutoResetEvent s_runGateThreadEvent = new AutoResetEvent(true);
+            private static readonly AutoResetEvent s_runGateThreadEvent = new AutoResetEvent(initialState: true);
 
-            private static LowLevelLock s_createdLock = new LowLevelLock();
-
-            private static CpuUtilizationReader s_cpu = new CpuUtilizationReader();
+            private static CpuUtilizationReader s_cpu;
             private const int MaxRuns = 2;
 
             // TODO: CoreCLR: Worker Tracking in CoreCLR? (Config name: ThreadPool_EnableWorkerTracking)
             private static void GateThreadStart()
             {
-                var initialCpuRead = s_cpu.CurrentUtilization; // The first reading is over a time range other than what we are focusing on, so we do not use the read.
+                _ = s_cpu.CurrentUtilization; // The first reading is over a time range other than what we are focusing on, so we do not use the read.
 
                 AppContext.TryGetSwitch("System.Threading.ThreadPool.DisableStarvationDetection", out bool disableStarvationDetection);
                 AppContext.TryGetSwitch("System.Threading.ThreadPool.DebugBreakOnWorkerStarvation", out bool debuggerBreakOnWorkStarvation);
@@ -87,7 +85,7 @@ private static bool SufficientDelaySinceLastDequeue()
 
                 int minimumDelay;
 
-                if(ThreadPoolInstance._cpuUtilization < CpuUtilizationLow)
+                if (ThreadPoolInstance._cpuUtilization < CpuUtilizationLow)
                 {
                     minimumDelay = GateThreadDelayMs;
                 }

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.HillClimbing.Complex.cs

@@ -19,15 +19,15 @@ public Complex(double real, double imaginary)
                 public double Imaginary { get; }
                 public double Real { get; }
 
-                public static Complex operator*(double scalar, Complex complex) => new Complex(scalar * complex.Real, scalar * complex.Imaginary);
+                public static Complex operator *(double scalar, Complex complex) => new Complex(scalar * complex.Real, scalar * complex.Imaginary);
 
-                public static Complex operator*(Complex complex, double scalar) => scalar * complex;
+                public static Complex operator *(Complex complex, double scalar) => scalar * complex;
 
-                public static Complex operator/(Complex complex, double scalar) => new Complex(complex.Real / scalar, complex.Imaginary / scalar);
+                public static Complex operator /(Complex complex, double scalar) => new Complex(complex.Real / scalar, complex.Imaginary / scalar);
 
-                public static Complex operator-(Complex lhs, Complex rhs) => new Complex(lhs.Real - rhs.Real, lhs.Imaginary - rhs.Imaginary);
+                public static Complex operator -(Complex lhs, Complex rhs) => new Complex(lhs.Real - rhs.Real, lhs.Imaginary - rhs.Imaginary);
 
-                public static Complex operator/(Complex lhs, Complex rhs)
+                public static Complex operator /(Complex lhs, Complex rhs)
                 {
                     double denom = rhs.Real * rhs.Real + rhs.Imaginary * rhs.Imaginary;
                     return new Complex((lhs.Real * rhs.Real + lhs.Imaginary * rhs.Imaginary) / denom, (-lhs.Real * rhs.Imaginary + lhs.Imaginary * rhs.Real) / denom);

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.HillClimbing.cs

@@ -399,7 +399,7 @@ private void LogTransition(int newThreadCount, double throughput, StateOrTransit
                 // Use the _log array as a circular array for log entries
                 int index = (_logStart + _logSize) % LogCapacity;
 
-                if(_logSize == LogCapacity)
+                if (_logSize == LogCapacity)
                 {
                     _logStart = (_logStart + 1) % LogCapacity;
                     _logSize--; // hide this slot while we update it
@@ -420,7 +420,7 @@ private void LogTransition(int newThreadCount, double throughput, StateOrTransit
 
             public void ForceChange(int newThreadCount, StateOrTransition state)
             {
-                if(_lastThreadCount != newThreadCount)
+                if (_lastThreadCount != newThreadCount)
                 {
                     _currentControlSetting += newThreadCount - _lastThreadCount;
                     ChangeThreadCount(newThreadCount, state);
@@ -442,7 +442,7 @@ private Complex GetWaveComponent(double[] samples, int numSamples, double period
                 double cos = Math.Cos(w);
                 double coeff = 2 * cos;
                 double q0 = 0, q1 = 0, q2 = 0;
-                for(int i = 0; i < numSamples; ++i)
+                for (int i = 0; i < numSamples; ++i)
                 {
                     q0 = coeff * q1 - q2 + samples[(_totalSamples - numSamples + i) % _samplesToMeasure];
                     q2 = q1;

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.ThreadCounts.cs

@@ -13,7 +13,7 @@ internal partial class PortableThreadPool
         /// Tracks information on the number of threads we want/have in different states in our thread pool.
         /// </summary>
         [StructLayout(LayoutKind.Explicit)]
-        struct ThreadCounts
+        private struct ThreadCounts
         {
             /// <summary>
             /// Max possible thread pool threads we want to have.

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WaitThread.cs

@@ -238,7 +238,7 @@ private void WaitThreadStart()
                     }
                     else
                     {
-                        if(numUserWaits == 0)
+                        if (numUserWaits == 0)
                         {
                             if (ThreadPoolInstance.TryRemoveWaitThread(this))
                             {

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs

@@ -173,7 +173,7 @@ internal static void MaybeAddWorkingWorker()
                             newCounts.numExistingThreads -= (short)toCreate;
 
                             ThreadCounts oldCounts = ThreadCounts.CompareExchangeCounts(ref ThreadPoolInstance._separated.counts, newCounts, counts);
-                            if(oldCounts == counts)
+                            if (oldCounts == counts)
                             {
                                 break;
                             }

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.cs

@@ -232,13 +232,13 @@ private void AdjustMaxWorkersActive()
 
             double elapsedSeconds = (double)(endTime - startTime) / freq;
 
-            if(elapsedSeconds * 1000 >= _threadAdjustmentIntervalMs / 2)
+            if (elapsedSeconds * 1000 >= _threadAdjustmentIntervalMs / 2)
             {
                 ThreadCounts currentCounts = ThreadCounts.VolatileReadCounts(ref _separated.counts);
                 int newMax;
                 (newMax, _threadAdjustmentIntervalMs) = HillClimbing.ThreadPoolHillClimber.Update(currentCounts.numThreadsGoal, elapsedSeconds, numCompletions);
 
-                while(newMax != currentCounts.numThreadsGoal)
+                while (newMax != currentCounts.numThreadsGoal)
                 {
                     ThreadCounts newCounts = currentCounts;
                     newCounts.numThreadsGoal = (short)newMax;
@@ -260,7 +260,7 @@ private void AdjustMaxWorkersActive()
                     }
                     else
                     {
-                        if(oldCounts.numThreadsGoal > currentCounts.numThreadsGoal && oldCounts.numThreadsGoal >= newMax)
+                        if (oldCounts.numThreadsGoal > currentCounts.numThreadsGoal && oldCounts.numThreadsGoal >= newMax)
                         {
                             // someone (probably the gate thread) increased the thread count more than
                             // we are about to do.  Don't interfere.
@@ -283,7 +283,7 @@ private bool ShouldAdjustMaxWorkersActive()
             int priorTime = Volatile.Read(ref _separated.priorCompletedWorkRequestsTime);
             int requiredInterval = _separated.nextCompletedWorkRequestsTime - priorTime;
             int elapsedInterval = Environment.TickCount - priorTime;
-            if(elapsedInterval >= requiredInterval)
+            if (elapsedInterval >= requiredInterval)
             {
                 // Avoid trying to adjust the thread count goal if there are already more threads than the thread count goal.
                 // In that situation, hill climbing must have previously decided to decrease the thread count goal, so let's

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPoolEventSource.cs

@@ -68,7 +68,7 @@ public unsafe void WorkerThreadAdjustmentSample(double throughput)
             }
         }
 
-        [Event(5, Level = EventLevel.Informational, Message = WorkerThreadAdjustmentSampleMessage, Opcode = Opcodes.AdjustmentOpcode, Version = 0, Task = Tasks.WorkerThreadAdjustmentTask, Keywords = Keywords.ThreadingKeyword)]
+        [Event(5, Level = EventLevel.Informational, Message = WorkerThreadAdjustmentAdjustmentEventMessage, Opcode = Opcodes.AdjustmentOpcode, Version = 0, Task = Tasks.WorkerThreadAdjustmentTask, Keywords = Keywords.ThreadingKeyword)]
         public unsafe void WorkerThreadAdjustmentAdjustment(double averageThroughput, int newWorkerThreadCount, int stateOrTransition)
         {
             if (IsEnabled())
@@ -84,7 +84,7 @@ public unsafe void WorkerThreadAdjustmentAdjustment(double averageThroughput, in
             }
         }
 
-        [Event(6, Level = EventLevel.Verbose, Message = WorkerThreadAdjustmentSampleMessage, Opcode = Opcodes.StatsOpcode, Version = 0, Task = Tasks.WorkerThreadAdjustmentTask, Keywords = Keywords.ThreadingKeyword)]
+        [Event(6, Level = EventLevel.Verbose, Message = WorkerThreadAdjustmentStatsEventMessage, Opcode = Opcodes.StatsOpcode, Version = 0, Task = Tasks.WorkerThreadAdjustmentTask, Keywords = Keywords.ThreadingKeyword)]
         [CLSCompliant(false)]
         public unsafe void WorkerThreadAdjustmentStats(double duration, double throughput, double threadWave, double throughputWave, double throughputErrorEstimate,
             double averageThroughputNoise, double ratio, double confidence, double currentControlSetting, ushort newThreadWaveMagnitude)

src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Portable.cs

@@ -29,7 +29,7 @@ internal RegisteredWaitHandle(WaitHandle waitHandle, _ThreadPoolWaitOrTimerCallb
 
         ~RegisteredWaitHandle()
         {
-            if(WaitThread != null)
+            if (WaitThread != null)
             {
                 Unregister(null);
             }
@@ -408,7 +408,7 @@ internal static bool NotifyWorkItemComplete()
         private static RegisteredWaitHandle RegisterWaitForSingleObject(
              WaitHandle waitObject,
              WaitOrTimerCallback callBack,
-             Object state,
+             object state,
              uint millisecondsTimeOutInterval,
              bool executeOnlyOnce,
              bool flowExecutionContext)

src/mono/netcore/System.Private.CoreLib/GenerateMonoCoreLibVersionFile.targets

@@ -9,13 +9,19 @@
     <PropertyGroup>
       <_MonoCoreLibVersionNumber>$([System.Text.RegularExpressions.Regex]::Match($([System.IO.File]::ReadAllText('$(MonoProjectRoot)\configure.ac')), 'MONO_CORLIB_VERSION=([a-fA-F0-9\-]+)').Groups[1].Value)</_MonoCoreLibVersionNumber>
       <_MonoCoreLibVersionFileLines>
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
 // This is a generated file. Do not edit.
+
+#pragma warning disable CA1823 // unused field
 namespace System
 {
-  partial class Environment
-  {
-    const string mono_corlib_version = "$(_MonoCoreLibVersionNumber)"%3B
-  }
+    public partial class Environment
+    {
+        private const string mono_corlib_version = "$(_MonoCoreLibVersionNumber)"%3B
+    }
 }
       </_MonoCoreLibVersionFileLines>
     </PropertyGroup>
@@ -28,4 +34,4 @@ namespace System
     </ItemGroup>
   </Target>
 
-</Project>
+</Project>

src/mono/netcore/System.Private.CoreLib/System.Private.CoreLib.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <!-- Generate Mono corelib version file -->
   <Import Project="GenerateMonoCoreLibVersionFile.targets" />
 
@@ -9,6 +9,7 @@
     <GenerateNeutralResourcesLanguageAttribute>false</GenerateNeutralResourcesLanguageAttribute>
     <EnsureRuntimePackageDependencies>false</EnsureRuntimePackageDependencies>
     <TargetFramework>netcoreapp2.1</TargetFramework>
+    <EnablePInvokeAnalyzer>false</EnablePInvokeAnalyzer>
 
     <!-- Ensure a portable PDB is emitted for the project. A PDB is needed for crossgen. -->
     <DebugType>Portable</DebugType>
@@ -60,7 +61,7 @@
     <_FullFrameworkReferenceAssemblyPaths>$(MSBuildThisFileDirectory)/Documentation</_FullFrameworkReferenceAssemblyPaths>
     <SkipCommonResourcesIncludes>true</SkipCommonResourcesIncludes>
     <DocumentationFile>$(OutputPath)$(MSBuildProjectName).xml</DocumentationFile>
-    <EnableAnalyzers>false</EnableAnalyzers> <!-- TOOD: reenable, right now it causes many errors in mono sources -->
+    <EnableAnalyzers>true</EnableAnalyzers>
   </PropertyGroup>
 
   <!-- Platform specific properties -->

src/mono/netcore/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeWaitHandle.Unix.Mono.cs

@@ -8,15 +8,15 @@
 
 namespace Microsoft.Win32.SafeHandles
 {
-	partial class SafeWaitHandle
-	{
-		protected override bool ReleaseHandle ()
-		{
-			CloseEventInternal (handle);
-			return true;
-		}
+    public partial class SafeWaitHandle
+    {
+        protected override bool ReleaseHandle()
+        {
+            CloseEventInternal(handle);
+            return true;
+        }
 
-		[MethodImplAttribute (MethodImplOptions.InternalCall)]
-		static extern void CloseEventInternal (IntPtr handle);
-	}
+        [MethodImplAttribute(MethodImplOptions.InternalCall)]
+        private static extern void CloseEventInternal(IntPtr handle);
+    }
 }