CA1802 Fixes and Severity to Warning (#7177)

* CA1802 Fixes and Severity to Warning * Revert src/Build/Microsoft.Build.csproj file changes * revert changes to CodeAnalysis.ruleset * enable warning on CA1802 * Fix remaining occurrences of CA1802 violations
dotnet · Jan 10, 2022 · 178d9e9 · 178d9e9
1 parent 636be09
commit 178d9e9
Show file tree

Hide file tree

Showing 14 changed files with 37 additions and 35 deletions.
diff --git a/eng/Common.globalconfig b/eng/Common.globalconfig
@@ -245,7 +245,7 @@ dotnet_diagnostic.CA1724.severity = none
 dotnet_diagnostic.CA1801.severity = none
 
 # Use literals where appropriate
-dotnet_diagnostic.CA1802.severity = suggestion
+dotnet_diagnostic.CA1802.severity = warning
 
 # Do not initialize unnecessarily
 dotnet_diagnostic.CA1805.severity = suggestion

diff --git a/src/Build.OM.UnitTests/Definition/Project_Tests.cs b/src/Build.OM.UnitTests/Definition/Project_Tests.cs
@@ -73,7 +73,7 @@ public void Dispose()
             ProjectCollection.GlobalProjectCollection.GlobalProperties.ShouldBeEmpty();
         }
 
-        private static readonly string ProjectWithItemGroup =
+        private const string ProjectWithItemGroup =
 @"<Project ToolsVersion='msbuilddefaulttoolsversion' DefaultTargets='Build' xmlns='msbuildnamespace'>
                   <ItemGroup>
 {0}

diff --git a/src/Build.UnitTests/Graph/GraphTestingUtilities.cs b/src/Build.UnitTests/Graph/GraphTestingUtilities.cs
@@ -18,21 +18,21 @@ internal static class GraphTestingUtilities
     {
         public static readonly ImmutableDictionary<string, string> EmptyGlobalProperties = new Dictionary<string, string> {{PropertyNames.IsGraphBuild, "true"}}.ToImmutableDictionary();
 
-        public static readonly string InnerBuildPropertyName = "InnerBuild";
-        public static readonly string InnerBuildPropertiesName = "InnerBuildProperties";
+        public const string InnerBuildPropertyName = "InnerBuild";
+        public const string InnerBuildPropertiesName = "InnerBuildProperties";
 
-        public static readonly string MultitargetingSpecificationPropertyGroup = $@"<PropertyGroup>
+        public const string MultitargetingSpecificationPropertyGroup = $@"<PropertyGroup>
                                                                         <InnerBuildProperty>{InnerBuildPropertyName}</InnerBuildProperty>
                                                                         <InnerBuildPropertyValues>{InnerBuildPropertiesName}</InnerBuildPropertyValues>
                                                                         <{InnerBuildPropertiesName}>a;b</{InnerBuildPropertiesName}>
                                                                      </PropertyGroup>";
-        public static readonly string HardCodedInnerBuildWithMultitargetingSpecification = $@"<PropertyGroup>
+        public const string HardCodedInnerBuildWithMultitargetingSpecification = $@"<PropertyGroup>
                                                                         <InnerBuildProperty>{InnerBuildPropertyName}</InnerBuildProperty>
                                                                         <InnerBuildPropertyValues>{InnerBuildPropertiesName}</InnerBuildPropertyValues>
                                                                         <{InnerBuildPropertyName}>a</{InnerBuildPropertyName}>
                                                                      </PropertyGroup>";
 
-        public static readonly string EnableTransitiveProjectReferencesPropertyGroup = @"<PropertyGroup>
+        public const string EnableTransitiveProjectReferencesPropertyGroup = @"<PropertyGroup>
                                                                                             <AddTransitiveProjectReferencesInStaticGraph>true</AddTransitiveProjectReferencesInStaticGraph>
                                                                                          </PropertyGroup>";
 

diff --git a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs
@@ -27,15 +27,15 @@ public class ProjectGraphTests : IDisposable
     {
         private TestEnvironment _env;
 
-        private static readonly string ProjectReferenceTargetsWithMultitargeting = @"<ItemGroup>
-                                                                                        <!-- Item order is important to ensure outer build targets are put in front of inner build ones -->
-                                                                                        <ProjectReferenceTargets Include='A' Targets='AHelperInner;A' />
-                                                                                        <ProjectReferenceTargets Include='A' Targets='AHelperOuter' OuterBuild='true' />
-                                                                                     </ItemGroup>";
+        private const string ProjectReferenceTargetsWithMultitargeting = @"<ItemGroup>
+                                                                               <!-- Item order is important to ensure outer build targets are put in front of inner build ones -->
+                                                                               <ProjectReferenceTargets Include='A' Targets='AHelperInner;A' />
+                                                                               <ProjectReferenceTargets Include='A' Targets='AHelperOuter' OuterBuild='true' />
+                                                                           </ItemGroup>";
         private static string[] NonOuterBuildTargets = {"AHelperOuter", "AHelperInner", "A"};
         private static string[] OuterBuildTargets = {"AHelperOuter"};
 
-        private static readonly string OuterBuildSpecificationWithProjectReferenceTargets = MultitargetingSpecificationPropertyGroup + ProjectReferenceTargetsWithMultitargeting;
+        private const string OuterBuildSpecificationWithProjectReferenceTargets = MultitargetingSpecificationPropertyGroup + ProjectReferenceTargetsWithMultitargeting;
 
         public ProjectGraphTests(ITestOutputHelper outputHelper)
         {

diff --git a/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs b/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs
@@ -42,7 +42,7 @@ public void Dispose()
             _env.Dispose();
         }
 
-        private static readonly string AssemblyMockCache = nameof(AssemblyMockCache);
+        private const string AssemblyMockCache = nameof(AssemblyMockCache);
 
         private static readonly Lazy<string> SamplePluginAssemblyPath =
             new Lazy<string>(

diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs
@@ -71,7 +71,9 @@ internal class ProjectRootElementCache : ProjectRootElementCacheBase
         /// If this number is increased much higher, the datastructure may
         /// need to be changed from a linked list, since it's currently O(n).
         /// </remarks>
+#pragma warning disable CA1802 // Use literals where appropriate
         private static readonly int s_maximumStrongCacheSize = 200;
+#pragma warning restore CA1802 // Use literals where appropriate
 
         /// <summary>
         /// Whether the cache should log activity to the Debug.Out stream

diff --git a/src/Build/Graph/GraphBuilder.cs b/src/Build/Graph/GraphBuilder.cs
@@ -22,7 +22,7 @@ namespace Microsoft.Build.Graph
 {
     internal class GraphBuilder
     {
-        internal static readonly string SolutionItemReference = "_SolutionReference";
+        internal const string SolutionItemReference = "_SolutionReference";
 
         /// <summary>
         /// The thread calling BuildGraph() will act as an implicit worker

diff --git a/src/Shared/FileUtilitiesRegex.cs b/src/Shared/FileUtilitiesRegex.cs
@@ -15,8 +15,8 @@ namespace Microsoft.Build.Shared
     /// </summary>
     internal static class FileUtilitiesRegex
     {
-        private static readonly char _backSlash = '\\';
-        private static readonly char _forwardSlash = '/';
+        private const char _backSlash = '\\';
+        private const char _forwardSlash = '/';
 
         /// <summary>
         /// Indicates whether the specified string follows the pattern drive pattern (for example "C:", "D:").

diff --git a/src/Shared/UnitTests/TypeLoader_Dependencies_Tests.cs b/src/Shared/UnitTests/TypeLoader_Dependencies_Tests.cs
@@ -15,9 +15,9 @@ namespace Microsoft.Build.UnitTests
     public class TypeLoader_Dependencies_Tests
     {
         private static readonly string ProjectFileFolder = Path.Combine(BuildEnvironmentHelper.Instance.CurrentMSBuildToolsDirectory, "TaskWithDependency");
-        private static readonly string ProjectFileName = "TaskWithDependencyTest.proj";
-        private static readonly string TaskDllFileName = "TaskWithDependency.dll";
-        private static readonly string DependencyDllFileName = "Dependency.dll";
+        private const string ProjectFileName = "TaskWithDependencyTest.proj";
+        private const string TaskDllFileName = "TaskWithDependency.dll";
+        private const string DependencyDllFileName = "Dependency.dll";
 
         [Fact]
         public void LoadAssemblyAndDependency_InsideProjectFolder()

diff --git a/src/Shared/UnitTests/TypeLoader_Tests.cs b/src/Shared/UnitTests/TypeLoader_Tests.cs
@@ -16,8 +16,8 @@ namespace Microsoft.Build.UnitTests
     public class TypeLoader_Tests
     {
         private static readonly string ProjectFileFolder = Path.Combine(BuildEnvironmentHelper.Instance.CurrentMSBuildToolsDirectory, "PortableTask");
-        private static readonly string ProjectFileName = "portableTaskTest.proj";
-        private static readonly string DLLFileName = "PortableTask.dll";
+        private const string ProjectFileName = "portableTaskTest.proj";
+        private const string DLLFileName = "PortableTask.dll";
 
         [Fact]
         public void Basic()

diff --git a/src/Tasks.UnitTests/WriteCodeFragment_Tests.cs b/src/Tasks.UnitTests/WriteCodeFragment_Tests.cs
@@ -484,8 +484,8 @@ public void MultilineAttributeCSharp()
             File.Delete(task.OutputFile.ItemSpec);
         }
 
-        private static readonly string VBCarriageReturn = "Global.Microsoft.VisualBasic.ChrW(13)";
-        private static readonly string VBLineFeed = "Global.Microsoft.VisualBasic.ChrW(10)";
+        private const string VBCarriageReturn = "Global.Microsoft.VisualBasic.ChrW(13)";
+        private const string VBLineFeed = "Global.Microsoft.VisualBasic.ChrW(10)";
 
         public static readonly string VBLineSeparator = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? $"{VBCarriageReturn}&{VBLineFeed}" : VBLineFeed;
 

diff --git a/src/Utilities.UnitTests/ToolLocationHelper_Tests.cs b/src/Utilities.UnitTests/ToolLocationHelper_Tests.cs
@@ -1039,7 +1039,7 @@ public void GetPathToWindowsSdk()
 #pragma warning restore 618
 
 #if FEATURE_CODETASKFACTORY
-        private static readonly string s_verifyToolsetAndToolLocationHelperProjectCommonContent = @"
+        private const string VerifyToolsetAndToolLocationHelperProjectCommonContent = @"
                                     string currentInstallFolderLocation = null;
 
                                     using (RegistryKey baseKey = Registry.LocalMachine.OpenSubKey(""SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows""))
@@ -1121,7 +1121,7 @@ public void VerifyToolsetAndToolLocationHelperAgree()
                                     }
 
                                     string pathTo81WinSDK = ToolLocationHelper.GetPathToWindowsSdk(TargetDotNetFrameworkVersion.VersionLatest, VisualStudioVersion.VersionLatest);" +
-                                    s_verifyToolsetAndToolLocationHelperProjectCommonContent +
+                                    VerifyToolsetAndToolLocationHelperProjectCommonContent +
                                   @"if (!String.Equals(WindowsSDK80Path, pathTo81WinSDK, StringComparison.OrdinalIgnoreCase))
                                     {
                                         Log.LogError(""WindowsSDK80Path is incorrect! Registry: {0}  ToolLocationHelper: {1}"", WindowsSDK80Path, pathTo81WinSDK);
@@ -1167,7 +1167,7 @@ public void VerifyToolsetAndToolLocationHelperAgreeWhenVisualStudioVersionIsEmpt
 
                                     pathTo35Sdk = pathTo35Sdk == null ? pathTo35Sdk : Path.Combine(pathTo35Sdk, ""bin\\"");
                                     pathTo40Sdk = pathTo40Sdk == null ? pathTo40Sdk : Path.Combine(pathTo40Sdk, ""bin\\NetFX 4.0 Tools\\"");" +
-                                    s_verifyToolsetAndToolLocationHelperProjectCommonContent +
+                                    VerifyToolsetAndToolLocationHelperProjectCommonContent +
                                   @"return !Log.HasLoggedErrors;
                                 ]]>
                               </Code>
@@ -1209,7 +1209,7 @@ public void VerifyToolsetAndToolLocationHelperAgreeWhenVisualStudioVersionIs10()
 
                                     pathTo35Sdk = pathTo35Sdk == null ? pathTo35Sdk : Path.Combine(pathTo35Sdk, ""bin\\"");
                                     pathTo40Sdk = pathTo40Sdk == null ? pathTo40Sdk : Path.Combine(pathTo40Sdk, ""bin\\NetFX 4.0 Tools\\"");" +
-                                    s_verifyToolsetAndToolLocationHelperProjectCommonContent +
+                                    VerifyToolsetAndToolLocationHelperProjectCommonContent +
                                   @"return !Log.HasLoggedErrors;
                                 ]]>
                               </Code>
@@ -1254,7 +1254,7 @@ public void VerifyToolsetAndToolLocationHelperAgreeWhenVisualStudioVersionIs11()
 
                                     pathTo35Sdk = pathTo35Sdk == null ? pathTo35Sdk : Path.Combine(pathTo35Sdk, ""bin\\"");
                                     pathTo40Sdk = pathTo40Sdk == null ? pathTo40Sdk : Path.Combine(pathTo40Sdk, ""bin\\NetFX 4.0 Tools\\"");" +
-                                    s_verifyToolsetAndToolLocationHelperProjectCommonContent +
+                                    VerifyToolsetAndToolLocationHelperProjectCommonContent +
                                    @"if (String.IsNullOrEmpty(WindowsSDK80Path))
                                     {
                                         Log.LogWarning(""WindowsSDK80Path is empty, which is technically not correct, but we're letting it slide for now because the OTG build won't have the updated registry for a while.  Make sure we don't see this warning on PURITs runs, though!"");

diff --git a/src/Utilities/CommandLineBuilder.cs b/src/Utilities/CommandLineBuilder.cs
@@ -112,19 +112,19 @@ public CommandLineBuilder(bool quoteHyphensOnCommandLine, bool useNewLineSeparat
         public override string ToString() => CommandLine.ToString();
 
         // Use if escaping of hyphens is supposed to take place
-        private static readonly string s_allowedUnquotedRegexNoHyphen =
+        private const string s_allowedUnquotedRegexNoHyphen =
                          "^"                             // Beginning of line
                        + @"[a-z\\/:0-9\._+=]*"
                        + "$";
 
-        private static readonly string s_definitelyNeedQuotesRegexWithHyphen = @"[|><\s,;\-""]+";
+        private const string s_definitelyNeedQuotesRegexWithHyphen = @"[|><\s,;\-""]+";
 
         // Use if escaping of hyphens is not to take place
-        private static readonly string s_allowedUnquotedRegexWithHyphen =
+        private const string s_allowedUnquotedRegexWithHyphen =
                         "^"                             // Beginning of line
                        + @"[a-z\\/:0-9\._\-+=]*"       //  Allow hyphen to be unquoted
                        + "$";
-        private static readonly string s_definitelyNeedQuotesRegexNoHyphen = @"[|><\s,;""]+";
+        private const string s_definitelyNeedQuotesRegexNoHyphen = @"[|><\s,;""]+";
 
         /// <summary>
         ///  Should hyphens be quoted or not

diff --git a/src/Utilities/TrackedDependencies/FileTracker.cs b/src/Utilities/TrackedDependencies/FileTracker.cs
@@ -88,11 +88,11 @@ public static class FileTracker
         private static readonly List<string> s_commonApplicationDataPaths;
 
         // The name of the standalone tracker tool.
-        private static readonly string s_TrackerFilename = "Tracker.exe";
+        private const string s_TrackerFilename = "Tracker.exe";
 
         // The name of the assembly that is injected into the executing process.
         // Detours handles picking between FileTracker{32,64}.dll so only mention one.
-        private static readonly string s_FileTrackerFilename = "FileTracker32.dll";
+        private const string s_FileTrackerFilename = "FileTracker32.dll";
 
         // The name of the PATH environment variable.
         private const string pathEnvironmentVariableName = "PATH";