From 6dc0759154b2892cdd3639c319f218644aa0bd3d Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Thu, 30 Dec 2021 10:18:37 +1000 Subject: [PATCH 1/5] CA1802 Fixes and Severity to Warning --- eng/CodeAnalysis.ruleset | 2 +- .../Definition/Project_Tests.cs | 2 +- .../Graph/GraphTestingUtilities.cs | 10 ++--- .../Graph/ProjectGraph_Tests.cs | 12 +++--- .../ProjectCache/ProjectCacheTests.cs | 2 +- .../Evaluation/ProjectRootElementCache.cs | 2 + src/Build/Graph/GraphBuilder.cs | 2 +- src/Build/Microsoft.Build.csproj | 40 +++++++++++++++++++ src/Shared/FileUtilitiesRegex.cs | 4 +- .../TypeLoader_Dependencies_Tests.cs | 6 +-- src/Shared/UnitTests/TypeLoader_Tests.cs | 4 +- .../WriteCodeFragment_Tests.cs | 4 +- 12 files changed, 66 insertions(+), 24 deletions(-) diff --git a/eng/CodeAnalysis.ruleset b/eng/CodeAnalysis.ruleset index 2078c42fe6c..4c6bbeb7faf 100644 --- a/eng/CodeAnalysis.ruleset +++ b/eng/CodeAnalysis.ruleset @@ -82,7 +82,7 @@ - + diff --git a/src/Build.OM.UnitTests/Definition/Project_Tests.cs b/src/Build.OM.UnitTests/Definition/Project_Tests.cs index afaf6ecf8ab..f2bfed5cc5f 100644 --- a/src/Build.OM.UnitTests/Definition/Project_Tests.cs +++ b/src/Build.OM.UnitTests/Definition/Project_Tests.cs @@ -71,7 +71,7 @@ public void Dispose() ProjectCollection.GlobalProjectCollection.GlobalProperties.ShouldBeEmpty(); } - private static readonly string ProjectWithItemGroup = + private const string ProjectWithItemGroup = @" {0} diff --git a/src/Build.UnitTests/Graph/GraphTestingUtilities.cs b/src/Build.UnitTests/Graph/GraphTestingUtilities.cs index 4bb00a27384..42e475fc7c7 100644 --- a/src/Build.UnitTests/Graph/GraphTestingUtilities.cs +++ b/src/Build.UnitTests/Graph/GraphTestingUtilities.cs @@ -16,21 +16,21 @@ internal static class GraphTestingUtilities { public static readonly ImmutableDictionary EmptyGlobalProperties = new Dictionary {{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 = $@" + public const string MultitargetingSpecificationPropertyGroup = $@" {InnerBuildPropertyName} {InnerBuildPropertiesName} <{InnerBuildPropertiesName}>a;b "; - public static readonly string HardCodedInnerBuildWithMultitargetingSpecification = $@" + public const string HardCodedInnerBuildWithMultitargetingSpecification = $@" {InnerBuildPropertyName} {InnerBuildPropertiesName} <{InnerBuildPropertyName}>a "; - public static readonly string EnableTransitiveProjectReferencesPropertyGroup = @" + public const string EnableTransitiveProjectReferencesPropertyGroup = @" true "; diff --git a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs index 97802055673..630e7a4d826 100644 --- a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs +++ b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs @@ -25,15 +25,15 @@ public class ProjectGraphTests : IDisposable { private TestEnvironment _env; - private static readonly string ProjectReferenceTargetsWithMultitargeting = @" - - - - "; + private const string ProjectReferenceTargetsWithMultitargeting = @" + + + + "; 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 index c994edab0ff..0781cc7e896 100644 --- a/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs +++ b/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs @@ -43,7 +43,7 @@ public void Dispose() _env.Dispose(); } - private static readonly string AssemblyMockCache = nameof(AssemblyMockCache); + private const string AssemblyMockCache = nameof(AssemblyMockCache); private static readonly Lazy SamplePluginAssemblyPath = new Lazy( diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs index ed90b1fb9cf..1e7b8a1be96 100644 --- a/src/Build/Evaluation/ProjectRootElementCache.cs +++ b/src/Build/Evaluation/ProjectRootElementCache.cs @@ -69,7 +69,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). /// +#pragma warning disable CA1802 // Use literals where appropriate private static readonly int s_maximumStrongCacheSize = 200; +#pragma warning restore CA1802 // Use literals where appropriate /// /// 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 index ce9499c546b..081698ea8c8 100644 --- a/src/Build/Graph/GraphBuilder.cs +++ b/src/Build/Graph/GraphBuilder.cs @@ -20,7 +20,7 @@ namespace Microsoft.Build.Graph { internal class GraphBuilder { - internal static readonly string SolutionItemReference = "_SolutionReference"; + internal const string SolutionItemReference = "_SolutionReference"; /// /// The thread calling BuildGraph() will act as an implicit worker diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index cd99bc84721..0bb3075d073 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -26,6 +26,46 @@ $(NoWarn);NU5104 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Shared/FileUtilitiesRegex.cs b/src/Shared/FileUtilitiesRegex.cs index d3e3b97af5f..0e606cd33b6 100644 --- a/src/Shared/FileUtilitiesRegex.cs +++ b/src/Shared/FileUtilitiesRegex.cs @@ -13,8 +13,8 @@ namespace Microsoft.Build.Shared /// internal static class FileUtilitiesRegex { - private static readonly char _backSlash = '\\'; - private static readonly char _forwardSlash = '/'; + private const char _backSlash = '\\'; + private const char _forwardSlash = '/'; /// /// 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 index f8acd297b4e..d0c3c8c7cc3 100644 --- a/src/Shared/UnitTests/TypeLoader_Dependencies_Tests.cs +++ b/src/Shared/UnitTests/TypeLoader_Dependencies_Tests.cs @@ -13,9 +13,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 index a572f51576a..5edf241553e 100644 --- a/src/Shared/UnitTests/TypeLoader_Tests.cs +++ b/src/Shared/UnitTests/TypeLoader_Tests.cs @@ -14,8 +14,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 index b389d8c784c..aec60bb94bb 100644 --- a/src/Tasks.UnitTests/WriteCodeFragment_Tests.cs +++ b/src/Tasks.UnitTests/WriteCodeFragment_Tests.cs @@ -482,8 +482,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; From 5b0ace2cdd9e167a1d0b91752f8306f810894c61 Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Thu, 30 Dec 2021 11:17:33 +1000 Subject: [PATCH 2/5] Revert src/Build/Microsoft.Build.csproj file changes --- src/Build/Microsoft.Build.csproj | 40 -------------------------------- 1 file changed, 40 deletions(-) diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 0bb3075d073..cd99bc84721 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -26,46 +26,6 @@ $(NoWarn);NU5104 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From 4691633a2a3d56edd157502f13214c1e37456653 Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Sat, 8 Jan 2022 10:16:56 +1000 Subject: [PATCH 3/5] revert changes to CodeAnalysis.ruleset --- eng/CodeAnalysis.ruleset | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/CodeAnalysis.ruleset b/eng/CodeAnalysis.ruleset index 4c6bbeb7faf..2078c42fe6c 100644 --- a/eng/CodeAnalysis.ruleset +++ b/eng/CodeAnalysis.ruleset @@ -82,7 +82,7 @@ - + From b66bd2af787502aa2fba17081155c0e9b588de29 Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Sat, 8 Jan 2022 10:18:04 +1000 Subject: [PATCH 4/5] enable warning on CA1802 --- eng/Common.globalconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/Common.globalconfig b/eng/Common.globalconfig index fd878420d57..a68901d9435 100644 --- 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 From 2a424de1f7c4bb817d9948a387d19d7a97644a03 Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Sat, 8 Jan 2022 13:36:55 +1000 Subject: [PATCH 5/5] Fix remaining occurrences of CA1802 violations --- src/Utilities.UnitTests/ToolLocationHelper_Tests.cs | 10 +++++----- src/Utilities/CommandLineBuilder.cs | 8 ++++---- src/Utilities/TrackedDependencies/FileTracker.cs | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Utilities.UnitTests/ToolLocationHelper_Tests.cs b/src/Utilities.UnitTests/ToolLocationHelper_Tests.cs index 845959692ef..30282dab454 100644 --- 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; ]]> @@ -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; ]]> @@ -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 index 52f30d7510c..4fbb21b508c 100644 --- 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,;""]+"; /// /// Should hyphens be quoted or not diff --git a/src/Utilities/TrackedDependencies/FileTracker.cs b/src/Utilities/TrackedDependencies/FileTracker.cs index 6e59887bd4b..e81f203b2cf 100644 --- a/src/Utilities/TrackedDependencies/FileTracker.cs +++ b/src/Utilities/TrackedDependencies/FileTracker.cs @@ -88,11 +88,11 @@ public static class FileTracker private static readonly List 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";