From ee41eb2303c70b7baa388bd89adc20ec081056f7 Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Thu, 30 Dec 2021 10:59:48 +1000 Subject: [PATCH 01/13] CA1810 Initialize reference type static fields inline --- eng/CodeAnalysis.ruleset | 2 +- .../IntrinsicTasks/ItemGroupLoggingHelper.cs | 2 ++ .../Evaluation/ProjectRootElementCache.cs | 25 +++---------------- src/MSBuild/XMake.cs | 2 ++ src/Shared/ExceptionHandling.cs | 7 +----- src/Shared/FrameworkLocationHelper.cs | 12 +++------ src/Shared/MSBuildNameIgnoreCaseComparer.cs | 11 +------- src/Shared/TypeLoader.cs | 9 +------ 8 files changed, 15 insertions(+), 55 deletions(-) diff --git a/eng/CodeAnalysis.ruleset b/eng/CodeAnalysis.ruleset index 2078c42fe6c..69a9d87e2ea 100644 --- a/eng/CodeAnalysis.ruleset +++ b/eng/CodeAnalysis.ruleset @@ -85,7 +85,7 @@ - + diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs index da174cbb259..d2ddb29632f 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs @@ -41,7 +41,9 @@ internal static class ItemGroupLoggingHelper /// to materialize the Message as that's a declaration assembly. We inject the logic /// here. /// +#pragma warning disable CA1810 // Initialize reference type static fields inline static ItemGroupLoggingHelper() +#pragma warning restore CA1810 // Initialize reference type static fields inline { BuildEventArgs.ResourceStringFormatter = ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword; TaskParameterEventArgs.MessageGetter = GetTaskParameterText; diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs index ed90b1fb9cf..abc66060292 100644 --- a/src/Build/Evaluation/ProjectRootElementCache.cs +++ b/src/Build/Evaluation/ProjectRootElementCache.cs @@ -69,12 +69,13 @@ 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). /// - private static readonly int s_maximumStrongCacheSize = 200; + private static readonly int s_maximumStrongCacheSize = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDPROJECTROOTELEMENTCACHESIZE")) ? + Convert.ToInt32(Environment.GetEnvironmentVariable("MSBUILDPROJECTROOTELEMENTCACHESIZE"), NumberFormatInfo.InvariantInfo) : 200; /// /// Whether the cache should log activity to the Debug.Out stream /// - private static bool s_debugLogCacheActivity; + private static bool s_debugLogCacheActivity = Environment.GetEnvironmentVariable("MSBUILDDEBUGXMLCACHE") == "1"; /// /// Whether the cache should check file content for cache entry invalidation. @@ -82,7 +83,7 @@ internal class ProjectRootElementCache : ProjectRootElementCacheBase /// /// Value shall be true only in case of testing. Outside QA tests it shall be false. /// - private static bool s_сheckFileContent; + private static bool s_сheckFileContent = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDCACHECHECKFILECONTENT")); #if DEBUG /// @@ -119,24 +120,6 @@ internal class ProjectRootElementCache : ProjectRootElementCacheBase /// private Object _locker = new Object(); - /// - /// Static constructor to choose cache size. - /// - static ProjectRootElementCache() - { - // Configurable in case a customer has related perf problems after shipping and so that - // we can measure different values for perf easily. - string userSpecifiedSize = Environment.GetEnvironmentVariable("MSBUILDPROJECTROOTELEMENTCACHESIZE"); - if (!String.IsNullOrEmpty(userSpecifiedSize)) - { - // Not catching as this is an undocumented setting - s_maximumStrongCacheSize = Convert.ToInt32(userSpecifiedSize, NumberFormatInfo.InvariantInfo); - } - - s_debugLogCacheActivity = Environment.GetEnvironmentVariable("MSBUILDDEBUGXMLCACHE") == "1"; - s_сheckFileContent = !String.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDCACHECHECKFILECONTENT")); - } - /// /// Creates an empty cache. /// diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index ae95d608193..ebf30dfdd6e 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -124,7 +124,9 @@ public enum ExitType /// /// Static constructor /// +#pragma warning disable CA1810 // Initialize reference type static fields inline static MSBuildApp() +#pragma warning restore CA1810 // Initialize reference type static fields inline { try { diff --git a/src/Shared/ExceptionHandling.cs b/src/Shared/ExceptionHandling.cs index f359e71a71d..54163132184 100644 --- a/src/Shared/ExceptionHandling.cs +++ b/src/Shared/ExceptionHandling.cs @@ -32,12 +32,7 @@ namespace Microsoft.Build.Shared /// internal static class ExceptionHandling { - private static readonly string s_debugDumpPath; - - static ExceptionHandling() - { - s_debugDumpPath = GetDebugDumpPath(); - } + private static readonly string s_debugDumpPath = GetDebugDumpPath(); /// /// Gets the location of the directory used for diagnostic log files. diff --git a/src/Shared/FrameworkLocationHelper.cs b/src/Shared/FrameworkLocationHelper.cs index 42237dde8c9..409a0093e6a 100644 --- a/src/Shared/FrameworkLocationHelper.cs +++ b/src/Shared/FrameworkLocationHelper.cs @@ -374,16 +374,10 @@ private static readonly (Version, Version)[,] s_explicitFallbackRulesForPathToDo }; #endif // FEATURE_WIN32_REGISTRY - private static readonly IReadOnlyDictionary s_dotNetFrameworkSpecDict; - private static readonly IReadOnlyDictionary s_visualStudioSpecDict; + private static readonly IReadOnlyDictionary s_dotNetFrameworkSpecDict = s_dotNetFrameworkSpecs.ToDictionary(spec => spec.Version); + private static readonly IReadOnlyDictionary s_visualStudioSpecDict = s_visualStudioSpecs.ToDictionary(spec => spec.Version); -#endregion // Static member variables - - static FrameworkLocationHelper() - { - s_dotNetFrameworkSpecDict = s_dotNetFrameworkSpecs.ToDictionary(spec => spec.Version); - s_visualStudioSpecDict = s_visualStudioSpecs.ToDictionary(spec => spec.Version); - } + #endregion // Static member variables #region Static properties diff --git a/src/Shared/MSBuildNameIgnoreCaseComparer.cs b/src/Shared/MSBuildNameIgnoreCaseComparer.cs index d0930cc0e19..1e436a21364 100644 --- a/src/Shared/MSBuildNameIgnoreCaseComparer.cs +++ b/src/Shared/MSBuildNameIgnoreCaseComparer.cs @@ -20,16 +20,7 @@ internal class MSBuildNameIgnoreCaseComparer : IConstrainedEqualityComparer /// The processor architecture on which we are running, but default it will be x86 /// - private static readonly NativeMethodsShared.ProcessorArchitectures s_runningProcessorArchitecture; - - /// - /// We need a static constructor to retrieve the running ProcessorArchitecture that way we can - /// avoid using optimized code that will not run correctly on IA64 due to alignment issues - /// - static MSBuildNameIgnoreCaseComparer() - { - s_runningProcessorArchitecture = NativeMethodsShared.ProcessorArchitecture; - } + private static readonly NativeMethodsShared.ProcessorArchitectures s_runningProcessorArchitecture = NativeMethodsShared.ProcessorArchitecture; /// /// The default immutable comparer instance. diff --git a/src/Shared/TypeLoader.cs b/src/Shared/TypeLoader.cs index 7f879afa8aa..47ee32d2b33 100644 --- a/src/Shared/TypeLoader.cs +++ b/src/Shared/TypeLoader.cs @@ -21,7 +21,7 @@ internal class TypeLoader /// /// AssemblyContextLoader used to load DLLs outside of msbuild.exe directory /// - private static readonly CoreClrAssemblyLoader s_coreClrAssemblyLoader; + private static readonly CoreClrAssemblyLoader s_coreClrAssemblyLoader = new CoreClrAssemblyLoader(); #endif /// @@ -39,13 +39,6 @@ internal class TypeLoader /// private Func _isDesiredType; -#if FEATURE_ASSEMBLYLOADCONTEXT - static TypeLoader() - { - s_coreClrAssemblyLoader = new CoreClrAssemblyLoader(); - } -#endif - /// /// Constructor. /// From 25a0ad0d9ab4128f1a4911d341bb33dd2d892c3a Mon Sep 17 00:00:00 2001 From: Lachlan Ennis <2433737+elachlan@users.noreply.github.com> Date: Fri, 31 Dec 2021 08:06:03 +1000 Subject: [PATCH 02/13] Update src/Build/Evaluation/ProjectRootElementCache.cs Co-authored-by: Forgind --- src/Build/Evaluation/ProjectRootElementCache.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs index abc66060292..a0bfe944c77 100644 --- a/src/Build/Evaluation/ProjectRootElementCache.cs +++ b/src/Build/Evaluation/ProjectRootElementCache.cs @@ -69,8 +69,8 @@ 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). /// - private static readonly int s_maximumStrongCacheSize = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDPROJECTROOTELEMENTCACHESIZE")) ? - Convert.ToInt32(Environment.GetEnvironmentVariable("MSBUILDPROJECTROOTELEMENTCACHESIZE"), NumberFormatInfo.InvariantInfo) : 200; + private static readonly int s_maximumStrongCacheSize = + Convert.ToInt32(Environment.GetEnvironmentVariable("MSBUILDPROJECTROOTELEMENTCACHESIZE") ?? "200", NumberFormatInfo.InvariantInfo); /// /// Whether the cache should log activity to the Debug.Out stream From bb9912197c679492036dce216f4061d31dcfb6aa Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Fri, 31 Dec 2021 08:11:04 +1000 Subject: [PATCH 03/13] unindent region --- src/Shared/FrameworkLocationHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/FrameworkLocationHelper.cs b/src/Shared/FrameworkLocationHelper.cs index 409a0093e6a..7962df7743d 100644 --- a/src/Shared/FrameworkLocationHelper.cs +++ b/src/Shared/FrameworkLocationHelper.cs @@ -377,7 +377,7 @@ private static readonly (Version, Version)[,] s_explicitFallbackRulesForPathToDo private static readonly IReadOnlyDictionary s_dotNetFrameworkSpecDict = s_dotNetFrameworkSpecs.ToDictionary(spec => spec.Version); private static readonly IReadOnlyDictionary s_visualStudioSpecDict = s_visualStudioSpecs.ToDictionary(spec => spec.Version); - #endregion // Static member variables +#endregion // Static member variables #region Static properties From ebabaaa69d77efd5cf91034267d9d0a961a09478 Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Sat, 8 Jan 2022 10:22:32 +1000 Subject: [PATCH 04/13] Revert CodeAnalysis.ruleset --- eng/CodeAnalysis.ruleset | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/CodeAnalysis.ruleset b/eng/CodeAnalysis.ruleset index 69a9d87e2ea..2078c42fe6c 100644 --- a/eng/CodeAnalysis.ruleset +++ b/eng/CodeAnalysis.ruleset @@ -85,7 +85,7 @@ - + From 97bab175f78ad026891760d41b13da846a6155c8 Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Sat, 8 Jan 2022 10:23:06 +1000 Subject: [PATCH 05/13] enable warning on CA1810 --- eng/Common.globalconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/Common.globalconfig b/eng/Common.globalconfig index fd878420d57..3222b0275f4 100644 --- a/eng/Common.globalconfig +++ b/eng/Common.globalconfig @@ -253,7 +253,7 @@ dotnet_diagnostic.CA1805.severity = suggestion dotnet_diagnostic.CA1806.severity = none # Initialize reference type static fields inline -dotnet_diagnostic.CA1810.severity = suggestion +dotnet_diagnostic.CA1810.severity = warning # Avoid uninstantiated internal classes dotnet_diagnostic.CA1812.severity = none From 60126c50272cc1b7ada0b79437cd761d21af288b Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Sat, 8 Jan 2022 11:40:36 +1000 Subject: [PATCH 06/13] Fix remaining occurrences of CA1810 violations --- src/Tasks/GenerateResource.cs | 27 ++---------------- src/Tasks/GetFrameworkPath.cs | 53 +++++++++++------------------------ 2 files changed, 18 insertions(+), 62 deletions(-) diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index 4dabaed7f16..17bebe45194 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -538,29 +538,6 @@ public GenerateResource() // do nothing } -#if FEATURE_COM_INTEROP - /// - /// Static constructor checks the registry opt-out for mark-of-the-web rejection. - /// - static GenerateResource() - { - if (!NativeMethodsShared.IsWindows) - { - allowMOTW = true; - return; - } - try - { - object allowUntrustedFiles = Registry.GetValue(@"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\SDK", "AllowProcessOfUntrustedResourceFiles", null); - if (allowUntrustedFiles is String) - { - allowMOTW = ((string)allowUntrustedFiles).Equals("true", StringComparison.OrdinalIgnoreCase); - } - } - catch { } - } -#endif - /// /// Logs a Resgen.exe command line that indicates what parameters were /// passed to this task. Since this task is replacing Resgen, and we used @@ -923,7 +900,7 @@ public override bool Execute() } #if FEATURE_COM_INTEROP - private static bool allowMOTW; + private static readonly bool AllowMOTW = !NativeMethodsShared.IsWindows || (Registry.GetValue(@"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\SDK", "AllowProcessOfUntrustedResourceFiles", null) as string).Equals("true", StringComparison.OrdinalIgnoreCase); private const string CLSID_InternetSecurityManager = "7b8a2d94-0ac9-11d1-896c-00c04fb6bfc4"; @@ -944,7 +921,7 @@ public override bool Execute() private bool IsDangerous(String filename) { // If they are opted out, there's no work to do - if (allowMOTW) + if (AllowMOTW) { return false; } diff --git a/src/Tasks/GetFrameworkPath.cs b/src/Tasks/GetFrameworkPath.cs index 999bd44f434..7b3d4e4988e 100644 --- a/src/Tasks/GetFrameworkPath.cs +++ b/src/Tasks/GetFrameworkPath.cs @@ -14,26 +14,6 @@ namespace Microsoft.Build.Tasks /// public class GetFrameworkPath : TaskExtension { - static GetFrameworkPath() - { - s_path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Latest)); - s_version11Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version11)); - s_version20Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version20)); - s_version30Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version30)); - s_version35Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version35)); - s_version40Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version40)); - s_version45Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version45)); - s_version451Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version451)); - s_version452Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version452)); - s_version46Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version46)); - s_version461Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version461)); - s_version462Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version462)); - s_version47Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version47)); - s_version471Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version471)); - s_version472Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version472)); - s_version48Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version48)); - } - #region ITask Members /// @@ -52,23 +32,22 @@ public override bool Execute() // it still seems to give an advantage perhaps because there is one less string copy. // In a large build, this adds up. // PERF NOTE: We also only find paths we are actually asked for (via tags) - - private static readonly Lazy s_path; - private static readonly Lazy s_version11Path; - private static readonly Lazy s_version20Path; - private static readonly Lazy s_version30Path; - private static readonly Lazy s_version35Path; - private static readonly Lazy s_version40Path; - private static readonly Lazy s_version45Path; - private static readonly Lazy s_version451Path; - private static readonly Lazy s_version452Path; - private static readonly Lazy s_version46Path; - private static readonly Lazy s_version461Path; - private static readonly Lazy s_version462Path; - private static readonly Lazy s_version47Path; - private static readonly Lazy s_version471Path; - private static readonly Lazy s_version472Path; - private static readonly Lazy s_version48Path; + private static readonly Lazy s_path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Latest)); + private static readonly Lazy s_version11Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version11)); + private static readonly Lazy s_version20Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version20)); + private static readonly Lazy s_version30Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version30)); + private static readonly Lazy s_version35Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version35)); + private static readonly Lazy s_version40Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version40)); + private static readonly Lazy s_version45Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version45)); + private static readonly Lazy s_version451Path=new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version451)); + private static readonly Lazy s_version452Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version452)); + private static readonly Lazy s_version46Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version46)); + private static readonly Lazy s_version461Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version461)); + private static readonly Lazy s_version462Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version462)); + private static readonly Lazy s_version47Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version47)); + private static readonly Lazy s_version471Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version471)); + private static readonly Lazy s_version472Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version472)); + private static readonly Lazy s_version48Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version48)); /// /// Path to the latest framework, whatever version it happens to be From c9c7e8a160ddede79e020c1d9ab36f43bafd4cdc Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Sat, 8 Jan 2022 11:58:17 +1000 Subject: [PATCH 07/13] handle null value after cast --- src/Tasks/GenerateResource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index 17bebe45194..d4eab76ef14 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -900,7 +900,7 @@ public override bool Execute() } #if FEATURE_COM_INTEROP - private static readonly bool AllowMOTW = !NativeMethodsShared.IsWindows || (Registry.GetValue(@"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\SDK", "AllowProcessOfUntrustedResourceFiles", null) as string).Equals("true", StringComparison.OrdinalIgnoreCase); + private static readonly bool AllowMOTW = !NativeMethodsShared.IsWindows || (Registry.GetValue(@"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\SDK", "AllowProcessOfUntrustedResourceFiles", null) as string ?? string.Empty).Equals("true", StringComparison.OrdinalIgnoreCase); private const string CLSID_InternetSecurityManager = "7b8a2d94-0ac9-11d1-896c-00c04fb6bfc4"; From 4140c8f568209fa0e128f0e47a22055bd5a3d2cd Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Wed, 19 Jan 2022 09:30:22 +1000 Subject: [PATCH 08/13] Remove static constructors --- src/Tasks/CultureInfoCache.cs | 19 ++++++++++--------- .../TrackedDependencies/FileTracker.cs | 16 +++++++++------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/Tasks/CultureInfoCache.cs b/src/Tasks/CultureInfoCache.cs index 4955e8a766e..5ae73395146 100644 --- a/src/Tasks/CultureInfoCache.cs +++ b/src/Tasks/CultureInfoCache.cs @@ -19,35 +19,36 @@ namespace Microsoft.Build.Tasks /// internal static class CultureInfoCache { - private static readonly HashSet ValidCultureNames; + private static readonly HashSet ValidCultureNames = InitializeValidCultureNames(); - static CultureInfoCache() + static HashSet InitializeValidCultureNames() { - ValidCultureNames = new HashSet(StringComparer.OrdinalIgnoreCase); - + HashSet validCultureNames = new(StringComparer.OrdinalIgnoreCase); #if !FEATURE_CULTUREINFO_GETCULTURES if (!AssemblyUtilities.CultureInfoHasGetCultures()) { - ValidCultureNames = HardcodedCultureNames; - return; + validCultureNames = HardcodedCultureNames; + return validCultureNames; } #endif foreach (CultureInfo cultureName in AssemblyUtilities.GetAllCultures()) { - ValidCultureNames.Add(cultureName.Name); + validCultureNames.Add(cultureName.Name); } // https://docs.microsoft.com/en-gb/windows/desktop/Intl/using-pseudo-locales-for-localization-testing // These pseudo-locales are available in versions of Windows from Vista and later. // However, from Windows 10, version 1803, they are not returned when enumerating the // installed cultures, even if the registry keys are set. Therefore, add them to the list manually. - var pseudoLocales = new[] { "qps-ploc", "qps-ploca", "qps-plocm", "qps-Latn-x-sh" }; + string[] pseudoLocales = new[] { "qps-ploc", "qps-ploca", "qps-plocm", "qps-Latn-x-sh" }; foreach (string pseudoLocale in pseudoLocales) { - ValidCultureNames.Add(pseudoLocale); + validCultureNames.Add(pseudoLocale); } + + return validCultureNames; } /// diff --git a/src/Utilities/TrackedDependencies/FileTracker.cs b/src/Utilities/TrackedDependencies/FileTracker.cs index e81f203b2cf..39ecf90d202 100644 --- a/src/Utilities/TrackedDependencies/FileTracker.cs +++ b/src/Utilities/TrackedDependencies/FileTracker.cs @@ -85,7 +85,7 @@ public static class FileTracker // Is equal to C:\Documents and Settings\All Users\Application Data on XP, and C:\ProgramData on Vista+. // But for backward compatibility, the paths "C:\Documents and Settings\All Users\Application Data" and "C:\Users\All Users\Application Data" are still accessible via Junction point on Vista+. // Thus this list is created to store all possible common application data paths to cover more cases as possible. - private static readonly List s_commonApplicationDataPaths; + private static readonly List s_commonApplicationDataPaths = InitializeCommonApplicationDataPaths(); // The name of the standalone tracker tool. private const string s_TrackerFilename = "Tracker.exe"; @@ -105,29 +105,31 @@ public static class FileTracker #endregion -#region Static constructor +#region Static Member Initializers - static FileTracker() + static List InitializeCommonApplicationDataPaths() { - s_commonApplicationDataPaths = new List(); + List commonApplicationDataPaths = new(); string defaultCommonApplicationDataPath = FileUtilities.EnsureTrailingSlash(Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData).ToUpperInvariant()); - s_commonApplicationDataPaths.Add(defaultCommonApplicationDataPath); + commonApplicationDataPaths.Add(defaultCommonApplicationDataPath); string defaultRootDirectory = Path.GetPathRoot(defaultCommonApplicationDataPath); string alternativeCommonApplicationDataPath1 = FileUtilities.EnsureTrailingSlash(Path.Combine(defaultRootDirectory, @"Documents and Settings\All Users\Application Data").ToUpperInvariant()); if (!alternativeCommonApplicationDataPath1.Equals(defaultCommonApplicationDataPath, StringComparison.Ordinal)) { - s_commonApplicationDataPaths.Add(alternativeCommonApplicationDataPath1); + commonApplicationDataPaths.Add(alternativeCommonApplicationDataPath1); } string alternativeCommonApplicationDataPath2 = FileUtilities.EnsureTrailingSlash(Path.Combine(defaultRootDirectory, @"Users\All Users\Application Data").ToUpperInvariant()); if (!alternativeCommonApplicationDataPath2.Equals(defaultCommonApplicationDataPath, StringComparison.Ordinal)) { - s_commonApplicationDataPaths.Add(alternativeCommonApplicationDataPath2); + commonApplicationDataPaths.Add(alternativeCommonApplicationDataPath2); } + + return commonApplicationDataPaths; } #endregion From 0c0dfd4f0777e711f28a1a40babbd112b78b964a Mon Sep 17 00:00:00 2001 From: Lachlan Ennis <2433737+elachlan@users.noreply.github.com> Date: Wed, 19 Jan 2022 09:49:38 +1000 Subject: [PATCH 09/13] Apply suggestions from code review Co-authored-by: Forgind --- src/Tasks/GenerateResource.cs | 2 +- src/Tasks/GetFrameworkPath.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index 711b6185a0d..af449d722c9 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -900,7 +900,7 @@ public override bool Execute() } #if FEATURE_COM_INTEROP - private static readonly bool AllowMOTW = !NativeMethodsShared.IsWindows || (Registry.GetValue(@"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\SDK", "AllowProcessOfUntrustedResourceFiles", null) as string ?? string.Empty).Equals("true", StringComparison.OrdinalIgnoreCase); + private static readonly bool AllowMOTW = !NativeMethodsShared.IsWindows || (Registry.GetValue(@"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\SDK", "AllowProcessOfUntrustedResourceFiles", null) is string allowUntrustedFiles && allowUntrustedFiles.Equals("true", StringComparison.OrdinalIgnoreCase)); private const string CLSID_InternetSecurityManager = "7b8a2d94-0ac9-11d1-896c-00c04fb6bfc4"; diff --git a/src/Tasks/GetFrameworkPath.cs b/src/Tasks/GetFrameworkPath.cs index 7b3d4e4988e..5793e004a7c 100644 --- a/src/Tasks/GetFrameworkPath.cs +++ b/src/Tasks/GetFrameworkPath.cs @@ -39,7 +39,7 @@ public override bool Execute() private static readonly Lazy s_version35Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version35)); private static readonly Lazy s_version40Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version40)); private static readonly Lazy s_version45Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version45)); - private static readonly Lazy s_version451Path=new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version451)); + private static readonly Lazy s_version451Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version451)); private static readonly Lazy s_version452Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version452)); private static readonly Lazy s_version46Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version46)); private static readonly Lazy s_version461Path = new Lazy(() => ToolLocationHelper.GetPathToDotNetFramework(TargetDotNetFrameworkVersion.Version461)); From 4bdda5bd1e70d2398badb79b92d20d27b840eaab Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Wed, 19 Jan 2022 09:51:20 +1000 Subject: [PATCH 10/13] refactor from code review --- src/Tasks/CultureInfoCache.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Tasks/CultureInfoCache.cs b/src/Tasks/CultureInfoCache.cs index 5ae73395146..a5c52d41275 100644 --- a/src/Tasks/CultureInfoCache.cs +++ b/src/Tasks/CultureInfoCache.cs @@ -23,15 +23,13 @@ internal static class CultureInfoCache static HashSet InitializeValidCultureNames() { - HashSet validCultureNames = new(StringComparer.OrdinalIgnoreCase); #if !FEATURE_CULTUREINFO_GETCULTURES if (!AssemblyUtilities.CultureInfoHasGetCultures()) { - validCultureNames = HardcodedCultureNames; - return validCultureNames; + return HardcodedCultureNames; } #endif - + HashSet validCultureNames = new(StringComparer.OrdinalIgnoreCase); foreach (CultureInfo cultureName in AssemblyUtilities.GetAllCultures()) { validCultureNames.Add(cultureName.Name); From eb3ca9f91758ad7b4c3dbae7722d7d72a8fd4b7b Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Wed, 26 Jan 2022 08:14:36 +1000 Subject: [PATCH 11/13] Change from review --- src/Build/Evaluation/ProjectRootElementCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs index 93549a8e188..ca1b7fef87e 100644 --- a/src/Build/Evaluation/ProjectRootElementCache.cs +++ b/src/Build/Evaluation/ProjectRootElementCache.cs @@ -72,7 +72,7 @@ internal class ProjectRootElementCache : ProjectRootElementCacheBase /// need to be changed from a linked list, since it's currently O(n). /// private static readonly int s_maximumStrongCacheSize = - Convert.ToInt32(Environment.GetEnvironmentVariable("MSBUILDPROJECTROOTELEMENTCACHESIZE") ?? "200", NumberFormatInfo.InvariantInfo); + int.TryParse(Environment.GetEnvironmentVariable("MSBUILDPROJECTROOTELEMENTCACHESIZE"), out int cacheSize) ? cacheSize : 200; /// /// Whether the cache should log activity to the Debug.Out stream From 158e42946c7a3d0d82bb149a8b0592493786ba3a Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Thu, 27 Jan 2022 09:17:08 +1000 Subject: [PATCH 12/13] Lazy initialization of Spec Dictionaries --- src/Shared/FrameworkLocationHelper.cs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/Shared/FrameworkLocationHelper.cs b/src/Shared/FrameworkLocationHelper.cs index 4b3e4559650..7cad2ee41ff 100644 --- a/src/Shared/FrameworkLocationHelper.cs +++ b/src/Shared/FrameworkLocationHelper.cs @@ -145,7 +145,7 @@ internal static class FrameworkLocationHelper /// /// List the supported .net versions. /// - private static readonly DotNetFrameworkSpec[] s_dotNetFrameworkSpecs = + private static readonly DotNetFrameworkSpec[] DotNetFrameworkSpecs = { // v1.1 new DotNetFrameworkSpecLegacy( @@ -225,7 +225,7 @@ internal static class FrameworkLocationHelper /// /// The items must be ordered by the version, because some methods depend on that fact to find the previous visual studio version. /// - private static readonly VisualStudioSpec[] s_visualStudioSpecs = + private static readonly VisualStudioSpec[] VisualStudioSpecs = { // VS10 new VisualStudioSpec(visualStudioVersion100, "Windows\\v7.0A", null, null, new [] @@ -376,8 +376,8 @@ private static readonly (Version, Version)[,] s_explicitFallbackRulesForPathToDo }; #endif // FEATURE_WIN32_REGISTRY - private static readonly IReadOnlyDictionary s_dotNetFrameworkSpecDict = s_dotNetFrameworkSpecs.ToDictionary(spec => spec.Version); - private static readonly IReadOnlyDictionary s_visualStudioSpecDict = s_visualStudioSpecs.ToDictionary(spec => spec.Version); + private static readonly Lazy> DotNetFrameworkSpecDict = new(() => DotNetFrameworkSpecs.ToDictionary(spec => spec.Version)); + private static readonly Lazy> VisualStudioSpecDict = new(() => VisualStudioSpecs.ToDictionary(spec => spec.Version)); #endregion // Static member variables @@ -1112,13 +1112,13 @@ private static string FindRegistryValueUnderKey private static VisualStudioSpec GetVisualStudioSpec(Version version) { - ErrorUtilities.VerifyThrowArgument(s_visualStudioSpecDict.TryGetValue(version, out VisualStudioSpec spec), "FrameworkLocationHelper.UnsupportedVisualStudioVersion", version); + ErrorUtilities.VerifyThrowArgument(VisualStudioSpecDict.Value.TryGetValue(version, out VisualStudioSpec spec), "FrameworkLocationHelper.UnsupportedVisualStudioVersion", version); return spec; } private static DotNetFrameworkSpec GetDotNetFrameworkSpec(Version version) { - ErrorUtilities.VerifyThrowArgument(s_dotNetFrameworkSpecDict.TryGetValue(version, out DotNetFrameworkSpec spec), "FrameworkLocationHelper.UnsupportedFrameworkVersion", version); + ErrorUtilities.VerifyThrowArgument(DotNetFrameworkSpecDict.Value.TryGetValue(version, out DotNetFrameworkSpec spec), "FrameworkLocationHelper.UnsupportedFrameworkVersion", version); return spec; } @@ -1467,11 +1467,11 @@ public virtual string GetPathToDotNetFrameworkSdkTools(VisualStudioSpec visualSt // i.e. fallback to v110 if the current visual studio version is v120. if (!foundExplicitRule) { - int index = Array.IndexOf(s_visualStudioSpecs, visualStudioSpec); + int index = Array.IndexOf(VisualStudioSpecs, visualStudioSpec); if (index > 0) { // The items in the array "visualStudioSpecs" must be ordered by version. That would allow us to fallback to the previous visual studio version easily. - VisualStudioSpec fallbackVisualStudioSpec = s_visualStudioSpecs[index - 1]; + VisualStudioSpec fallbackVisualStudioSpec = VisualStudioSpecs[index - 1]; generatedPathToDotNetFrameworkSdkTools = FallbackToPathToDotNetFrameworkSdkToolsInPreviousVersion( this.Version, fallbackVisualStudioSpec.Version); @@ -1564,10 +1564,8 @@ public virtual string GetPathToWindowsSdk() #if FEATURE_WIN32_REGISTRY private static string FallbackToPathToDotNetFrameworkSdkToolsInPreviousVersion(Version dotNetFrameworkVersion, Version visualStudioVersion) { - VisualStudioSpec visualStudioSpec; - DotNetFrameworkSpec dotNetFrameworkSpec; - if (s_visualStudioSpecDict.TryGetValue(visualStudioVersion, out visualStudioSpec) - && s_dotNetFrameworkSpecDict.TryGetValue(dotNetFrameworkVersion, out dotNetFrameworkSpec) + if (VisualStudioSpecDict.Value.TryGetValue(visualStudioVersion, out VisualStudioSpec visualStudioSpec) + && DotNetFrameworkSpecDict.Value.TryGetValue(dotNetFrameworkVersion, out DotNetFrameworkSpec dotNetFrameworkSpec) && visualStudioSpec.SupportedDotNetFrameworkVersions.Contains(dotNetFrameworkVersion)) { return dotNetFrameworkSpec.GetPathToDotNetFrameworkSdkTools(visualStudioSpec); From a36f0844a6593a6bcd7e074641253b1f9713845b Mon Sep 17 00:00:00 2001 From: elachlan <2433737+elachlan@users.noreply.github.com> Date: Thu, 27 Jan 2022 20:39:58 +1000 Subject: [PATCH 13/13] Changes from review plus a few other fixes --- src/Shared/FrameworkLocationHelper.cs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Shared/FrameworkLocationHelper.cs b/src/Shared/FrameworkLocationHelper.cs index 7cad2ee41ff..01ff320cf7d 100644 --- a/src/Shared/FrameworkLocationHelper.cs +++ b/src/Shared/FrameworkLocationHelper.cs @@ -33,7 +33,7 @@ internal enum DotNetFrameworkArchitecture /// /// Indicates the 64-bit .NET Framework /// - Bitness64 = 2 + Bitness64 = 2, } /// @@ -72,7 +72,7 @@ internal static class FrameworkLocationHelper internal static readonly Version visualStudioVersion170 = new Version(17, 0); // keep this up-to-date; always point to the latest visual studio version. - internal static readonly Version visualStudioVersionLatest = visualStudioVersion160; + internal static readonly Version visualStudioVersionLatest = visualStudioVersion170; private const string dotNetFrameworkRegistryPath = "SOFTWARE\\Microsoft\\.NETFramework"; private const string dotNetFrameworkSetupRegistryPath = "SOFTWARE\\Microsoft\\NET Framework Setup\\NDP"; @@ -145,7 +145,7 @@ internal static class FrameworkLocationHelper /// /// List the supported .net versions. /// - private static readonly DotNetFrameworkSpec[] DotNetFrameworkSpecs = + private static DotNetFrameworkSpec[] DotNetFrameworkSpecs() => new DotNetFrameworkSpec[] { // v1.1 new DotNetFrameworkSpecLegacy( @@ -225,7 +225,7 @@ internal static class FrameworkLocationHelper /// /// The items must be ordered by the version, because some methods depend on that fact to find the previous visual studio version. /// - private static readonly VisualStudioSpec[] VisualStudioSpecs = + private static readonly Lazy VisualStudioSpecs = new(() => new VisualStudioSpec[] { // VS10 new VisualStudioSpec(visualStudioVersion100, "Windows\\v7.0A", null, null, new [] @@ -255,7 +255,7 @@ internal static class FrameworkLocationHelper dotNetFrameworkVersion40, dotNetFrameworkVersion45, dotNetFrameworkVersion451, - dotNetFrameworkVersion452 + dotNetFrameworkVersion452, }), // VS14 @@ -269,7 +269,7 @@ internal static class FrameworkLocationHelper dotNetFrameworkVersion451, dotNetFrameworkVersion452, dotNetFrameworkVersion46, - dotNetFrameworkVersion461 + dotNetFrameworkVersion461, }), // VS15 @@ -328,7 +328,7 @@ internal static class FrameworkLocationHelper dotNetFrameworkVersion472, dotNetFrameworkVersion48, }), - }; + }); #if FEATURE_WIN32_REGISTRY /// @@ -373,11 +373,11 @@ private static readonly (Version, Version)[,] s_explicitFallbackRulesForPathToDo { (dotNetFrameworkVersion471, visualStudioVersion160), (dotNetFrameworkVersion47, visualStudioVersion160) }, { (dotNetFrameworkVersion472, visualStudioVersion160), (dotNetFrameworkVersion471, visualStudioVersion160) }, { (dotNetFrameworkVersion48, visualStudioVersion160), (dotNetFrameworkVersion472, visualStudioVersion160) }, - }; + }; #endif // FEATURE_WIN32_REGISTRY - private static readonly Lazy> DotNetFrameworkSpecDict = new(() => DotNetFrameworkSpecs.ToDictionary(spec => spec.Version)); - private static readonly Lazy> VisualStudioSpecDict = new(() => VisualStudioSpecs.ToDictionary(spec => spec.Version)); + private static readonly Lazy> DotNetFrameworkSpecDict = new(() => DotNetFrameworkSpecs().ToDictionary(spec => spec.Version)); + private static readonly Lazy> VisualStudioSpecDict = new(() => VisualStudioSpecs.Value.ToDictionary(spec => spec.Version)); #endregion // Static member variables @@ -1467,11 +1467,11 @@ public virtual string GetPathToDotNetFrameworkSdkTools(VisualStudioSpec visualSt // i.e. fallback to v110 if the current visual studio version is v120. if (!foundExplicitRule) { - int index = Array.IndexOf(VisualStudioSpecs, visualStudioSpec); + int index = Array.IndexOf(VisualStudioSpecs.Value, visualStudioSpec); if (index > 0) { // The items in the array "visualStudioSpecs" must be ordered by version. That would allow us to fallback to the previous visual studio version easily. - VisualStudioSpec fallbackVisualStudioSpec = VisualStudioSpecs[index - 1]; + VisualStudioSpec fallbackVisualStudioSpec = VisualStudioSpecs.Value[index - 1]; generatedPathToDotNetFrameworkSdkTools = FallbackToPathToDotNetFrameworkSdkToolsInPreviousVersion( this.Version, fallbackVisualStudioSpec.Version);