Fix handling of repo analyzers and warnings-as-errors (#42272)

* Fix handling of repo analyzers and warnings-as-errors When we brought in the new SDK, it enabled analyzers by default (which then used our custom ruleset), but a bunch of projects (in particular tests) weren't expecting that, such that we now have thousands of warnings in the repo. This opts-out those projects. It also enables warnings-as-errors at the root level of the repo, to hopefully avoid such warning storms in the future, and to also clean up the remaining that exist. This includes a bunch of new obsoletion and platform compat warnings that are firing in the runtime tests. We may choose to run analyzers on additional projects in the future where it's currently disabled, but this gets us back to a state at least as good if not better than we were previously. * Fix analyzer warnings on Microsoft.NET.HostModel Fixes the warnings that were triggered by our rule set applying to this project. All fixes were automated. * Fix analyzer warnings in additional projects * Remove several `<RunAnalyzers>false</RunAnalyzers>` * Try to opt-out remaining coreclr tests Co-authored-by: David Mason <[email protected]>
dotnet · Sep 17, 2020 · e0e1919 · e0e1919
1 parent 876980c
commit e0e1919
Show file tree

Hide file tree

Showing 109 changed files with 868 additions and 902 deletions.
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -101,6 +101,7 @@
     <LangVersion>preview</LangVersion>
     <LangVersion Condition="'$(MSBuildProjectExtension)' == '.vbproj'">latest</LangVersion>
     <Deterministic>true</Deterministic>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>
 
   <PropertyGroup>

diff --git a/eng/Analyzers.props b/eng/Analyzers.props
@@ -2,9 +2,9 @@
   <PropertyGroup>
     <CodeAnalysisRuleset>$(MSBuildThisFileDirectory)CodeAnalysis.ruleset</CodeAnalysisRuleset>
     <!-- Disable analyzers in sourcebuild -->
-    <EnableAnalyzers Condition="'$(DotNetBuildFromSource)' == 'true'">false</EnableAnalyzers>
+    <RunAnalyzers Condition="'$(DotNetBuildFromSource)' == 'true'">false</RunAnalyzers>
   </PropertyGroup>
-  <ItemGroup Condition="'$(EnableAnalyzers)' == 'true'">
+  <ItemGroup Condition="'$(RunAnalyzers)' != 'false'">
     <PackageReference Include="Microsoft.DotNet.CodeAnalysis" Version="$(MicrosoftDotNetCodeAnalysisVersion)" PrivateAssets="all" IsImplicitlyDefined="true" />
     <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="5.0.0-rc2.20458.2" PrivateAssets="all" />
     <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.164" PrivateAssets="all" />

diff --git a/eng/referenceAssemblies.props b/eng/referenceAssemblies.props
@@ -6,7 +6,7 @@
     <AdditionalBuildTargetFrameworks Condition="!$(BuildTargetFramework.StartsWith('netcoreapp2.0'))">$(AdditionalBuildTargetFrameworks);netstandard2.1</AdditionalBuildTargetFrameworks>
 
     <!-- Reference assemblies are special and don't initialize fields or have empty finalizers, etc. -->
-    <EnableAnalyzers>false</EnableAnalyzers>
+    <RunAnalyzers>false</RunAnalyzers>
   </PropertyGroup>
 
   <PropertyGroup>

diff --git a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj
@@ -48,7 +48,6 @@
 
     <SkipCommonResourcesIncludes>true</SkipCommonResourcesIncludes>
     <DocumentationFile>$(OutputPath)$(MSBuildProjectName).xml</DocumentationFile>
-    <EnableAnalyzers>true</EnableAnalyzers>
   </PropertyGroup>
 
   <!-- Platform specific properties -->

diff --git a/src/coreclr/src/tools/Directory.Build.props b/src/coreclr/src/tools/Directory.Build.props
@@ -3,5 +3,6 @@
   <PropertyGroup>
     <IsShipping>false</IsShipping>
     <SignAssembly>false</SignAssembly>
+    <RunAnalyzers>false</RunAnalyzers>
   </PropertyGroup>
 </Project>
diff --git a/src/coreclr/tests/Directory.Build.props b/src/coreclr/tests/Directory.Build.props
@@ -65,4 +65,9 @@
     <CLRTestBuildAllTargets></CLRTestBuildAllTargets>
   </PropertyGroup>
 
+  <!-- Language settings -->
+  <PropertyGroup>
+    <RunAnalyzers>false</RunAnalyzers>
+  </PropertyGroup>
+
 </Project>
diff --git a/src/coreclr/tests/dir.sdkbuild.props b/src/coreclr/tests/dir.sdkbuild.props
@@ -10,6 +10,7 @@
     <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
     <EnableDefaultItems>false</EnableDefaultItems>
     <Platform>AnyCPU</Platform>
+    <NoWarn>$(NoWarn),618,SYSLIB0012</NoWarn>
 
     <!-- [ARCADE REMOVE] This line should be removed we use the Arcade Sdk. -->
     <DeterministicSourcePaths>false</DeterministicSourcePaths>

diff --git a/src/installer/Directory.Build.props b/src/installer/Directory.Build.props
@@ -142,11 +142,6 @@
 
   </PropertyGroup>
 
-  <!-- Set up handling of build warnings -->
-  <PropertyGroup>
-    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
-  </PropertyGroup>
-
   <PropertyGroup>
     <TargetsWindows>false</TargetsWindows>
     <TargetsOSX>false</TargetsOSX>

diff --git a/...ler/managed/Microsoft.NET.HostModel/AppHost/AppHostCustomizationUnsupportedOSException.cs b/...ler/managed/Microsoft.NET.HostModel/AppHost/AppHostCustomizationUnsupportedOSException.cs
@@ -6,11 +6,10 @@
 namespace Microsoft.NET.HostModel.AppHost
 {
     /// <summary>
-    /// The application host executable cannot be customized because adding resources requires 
+    /// The application host executable cannot be customized because adding resources requires
     /// that the build be performed on Windows (excluding Nano Server).
     /// </summary>
     public class AppHostCustomizationUnsupportedOSException : AppHostUpdateException
     {
     }
 }
-
diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppHostMachOFormatException.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppHostMachOFormatException.cs
@@ -22,11 +22,11 @@ public enum MachOFormatError
         SignBlobNotLast,        // Signature blob must be at the very end of the file
         SignDoesntFollowSymtab, // Signature blob must immediately follow the Symtab
         MemoryMapAccessFault,   // Error reading the memory-mapped apphost
-        InvalidUTF8             // UTF8 decoding failed 
+        InvalidUTF8             // UTF8 decoding failed
     }
 
     /// <summary>
-    /// The MachO application host executable cannot be customized because 
+    /// The MachO application host executable cannot be customized because
     /// it was not in the expected format
     /// </summary>
     public class AppHostMachOFormatException : AppHostUpdateException
@@ -39,4 +39,3 @@ public AppHostMachOFormatException(MachOFormatError error)
         }
     }
 }
-
diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppHostNotCUIException.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppHostNotCUIException.cs
@@ -6,11 +6,10 @@
 namespace Microsoft.NET.HostModel.AppHost
 {
     /// <summary>
-    /// Unable to use the input file as application host executable because it's not a 
+    /// Unable to use the input file as application host executable because it's not a
     /// Windows executable for the CUI (Console) subsystem.
     /// </summary>
     public class AppHostNotCUIException : AppHostUpdateException
     {
     }
 }
-
diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppHostNotPEFileException.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppHostNotPEFileException.cs
@@ -6,11 +6,10 @@
 namespace Microsoft.NET.HostModel.AppHost
 {
     /// <summary>
-    ///  Unable to use the input file as an application host executable 
+    ///  Unable to use the input file as an application host executable
     ///  because it's not a Windows PE file
     /// </summary>
     public class AppHostNotPEFileException : AppHostUpdateException
     {
     }
 }
-
diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppHostUpdateException.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppHostUpdateException.cs
@@ -6,11 +6,10 @@
 namespace Microsoft.NET.HostModel.AppHost
 {
     /// <summary>
-    /// An instance of this exception is thrown when an AppHost binary update 
+    /// An instance of this exception is thrown when an AppHost binary update
     /// fails due to known user errors.
     /// </summary>
     public class AppHostUpdateException : Exception
     {
     }
 }
-
diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppNameTooLongException.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/AppNameTooLongException.cs
@@ -18,4 +18,3 @@ public AppNameTooLongException(string name)
 
     }
 }
-
diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/BinaryUtils.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/BinaryUtils.cs
@@ -60,8 +60,8 @@ private static unsafe void Pad0(byte[] searchPattern, byte[] patternToReplace, b
         }
 
         public static unsafe void SearchAndReplace(
-            string filePath, 
-            byte[] searchPattern, 
+            string filePath,
+            byte[] searchPattern,
             byte[] patternToReplace,
             bool pad0s = true)
         {

diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/ElfUtils.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/ElfUtils.cs
@@ -10,12 +10,12 @@ internal static class ElfUtils
         // The Linux Headers are copied from elf.h
 
 #pragma warning disable 0649
-        struct ElfHeader
+        private struct ElfHeader
         {
-            byte EI_MAG0;
-            byte EI_MAG1;
-            byte EI_MAG2;
-            byte EI_MAG3;
+            private byte EI_MAG0;
+            private byte EI_MAG1;
+            private byte EI_MAG2;
+            private byte EI_MAG3;
 
             public bool IsValid()
             {
@@ -49,4 +49,3 @@ public static bool IsElfImage(string filePath)
         }
     }
 }
-
diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs
@@ -155,7 +155,7 @@ public static void SetAsBundle(
             long bundleHeaderOffset)
         {
             byte[] bundleHeaderPlaceholder = {
-                // 8 bytes represent the bundle header-offset 
+                // 8 bytes represent the bundle header-offset
                 // Zero for non-bundle apphosts (default).
                 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
                 // 32 bytes represent the bundle signature: SHA-256 for ".net core bundle"
@@ -206,7 +206,7 @@ void FindBundleHeader()
                             throw new PlaceHolderNotFoundInAppHostException(bundleSignature);
                         }
 
-                        headerOffset = accessor.ReadInt64(position - sizeof(Int64));
+                        headerOffset = accessor.ReadInt64(position - sizeof(long));
                     }
                 }
             }

diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/MachOUtils.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/MachOUtils.cs
@@ -11,27 +11,27 @@ namespace Microsoft.NET.HostModel.AppHost
 {
     internal static class MachOUtils
     {
-        // The MachO Headers are copied from 
+        // The MachO Headers are copied from
         // https://opensource.apple.com/source/cctools/cctools-870/include/mach-o/loader.h
         //
         // The data fields and enumerations match the structure definitions in the above file,
         // and hence do not conform to C# CoreFx naming style.
 
-        enum Magic : uint
+        private enum Magic : uint
         {
             MH_MAGIC = 0xfeedface,
             MH_CIGAM = 0xcefaedfe,
             MH_MAGIC_64 = 0xfeedfacf,
             MH_CIGAM_64 = 0xcffaedfe
         }
 
-        enum FileType : uint
+        private enum FileType : uint
         {
             MH_EXECUTE = 0x2
         }
 
 #pragma warning disable 0649
-        struct MachHeader
+        private struct MachHeader
         {
             public Magic magic;
             public int cputype;
@@ -63,30 +63,30 @@ public bool IsValid()
             }
         }
 
-        enum Command : uint
+        private enum Command : uint
         {
             LC_SYMTAB = 0x2,
             LC_SEGMENT_64 = 0x19,
             LC_CODE_SIGNATURE = 0x1d,
         }
 
-        struct LoadCommand
+        private struct LoadCommand
         {
             public Command cmd;
             public uint cmdsize;
         }
 
         // The linkedit_data_command contains the offsets and sizes of a blob
-        // of data in the __LINKEDIT segment (including LC_CODE_SIGNATURE).  
-        struct LinkEditDataCommand
+        // of data in the __LINKEDIT segment (including LC_CODE_SIGNATURE).
+        private struct LinkEditDataCommand
         {
             public Command cmd;
             public uint cmdsize;
             public uint dataoff;
             public uint datasize;
         }
 
-        struct SymtabCommand
+        private struct SymtabCommand
         {
             public uint cmd;
             public uint cmdsize;
@@ -96,7 +96,7 @@ struct SymtabCommand
             public uint strsize;
         };
 
-        unsafe struct SegmentCommand64
+        private unsafe struct SegmentCommand64
         {
             public Command cmd;
             public uint cmdsize;
@@ -159,47 +159,47 @@ public static bool IsMachOImage(string filePath)
         /// <summary>
         /// This Method is a utility to remove the code-signature (if any)
         /// from a MachO AppHost binary.
-        /// 
+        ///
         /// The tool assumes the following layout of the executable:
-        /// 
+        ///
         /// * MachoHeader (64-bit, executable, not swapped integers)
         /// * LoadCommands
         ///     LC_SEGMENT_64 (__PAGEZERO)
         ///     LC_SEGMENT_64 (__TEXT)
         ///     LC_SEGMENT_64 (__DATA)
         ///     LC_SEGMENT_64 (__LINKEDIT)
         ///     ...
-        ///     LC_SYMTAB 
+        ///     LC_SYMTAB
         ///     ...
         ///     LC_CODE_SIGNATURE (last)
-        ///     
+        ///
         ///  * ... Different Segments ...
-        ///  
+        ///
         ///  * The __LINKEDIT Segment (last)
-        ///      * ... Different sections ... 
-        ///      * SYMTAB 
+        ///      * ... Different sections ...
+        ///      * SYMTAB
         ///      * (Some alignment bytes)
         ///      * The Code-signature
-        ///      
+        ///
         /// In order to remove the signature, the method:
         /// - Removes (zeros out) the LC_CODE_SIGNATURE command
         /// - Adjusts the size and count of the load commands in the header
         /// - Truncates the size of the __LINKEDIT segment to the end of SYMTAB
         /// - Truncates the apphost file to the end of the __LINKEDIT segment
-        /// 
+        ///
         /// </summary>
         /// <param name="filePath">Path to the AppHost</param>
         /// <returns>
-        ///  True if 
-        ///    - The input is a MachO binary, and 
-        ///    - It is a signed binary, and 
+        ///  True if
+        ///    - The input is a MachO binary, and
+        ///    - It is a signed binary, and
         ///    - The signature was successfully removed
-        ///   False otherwise 
+        ///   False otherwise
         /// </returns>
         /// <exception cref="AppHostMachOFormatException">
         /// The input is a MachO file, but doesn't match the expect format of the AppHost.
         /// </exception>
-        unsafe public static bool RemoveSignature(string filePath)
+        public static unsafe bool RemoveSignature(string filePath)
         {
             using (var stream = new FileStream(filePath, FileMode.Open, FileAccess.ReadWrite))
             {
@@ -278,7 +278,7 @@ unsafe public static bool RemoveSignature(string filePath)
                                 Verify(symtab->symoff > linkEdit->fileoff, MachOFormatError.SymtabNotInLinkEdit);
                                 Verify(signature->dataoff > linkEdit->fileoff, MachOFormatError.SignNotInLinkEdit);
 
-                                // The signature blob immediately follows the symtab blob, 
+                                // The signature blob immediately follows the symtab blob,
                                 // except for a few bytes of padding.
                                 Verify(signature->dataoff >= symtabEnd && signature->dataoff - symtabEnd < 32, MachOFormatError.SignBlobNotLast);
 
@@ -294,7 +294,7 @@ unsafe public static bool RemoveSignature(string filePath)
                                 linkEdit->filesize -= signatureSize;
 
                                 // codesign --remove-signature doesn't reset the vmsize.
-                                // Setting the vmsize here makes the output bin-equal with the original 
+                                // Setting the vmsize here makes the output bin-equal with the original
                                 // unsigned apphost (and not bin-equal with a signed-unsigned-apphost).
                                 linkEdit->vmsize = linkEdit->filesize;
                             }
@@ -321,4 +321,3 @@ unsafe public static bool RemoveSignature(string filePath)
         }
     }
 }
-
Original file line number	Diff line number	Diff line change
Expand Up		@@ -18,4 +18,3 @@ public AppNameTooLongException(string name)

		}
		}