diff --git a/.editorconfig b/.editorconfig index 7b0f1419bb8..93a196caa51 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,4 +1,4 @@ -# editorconfig.org +# editorconfig.org # top-most EditorConfig file root = true @@ -163,6 +163,9 @@ dotnet_code_quality.ca2208.api_surface = public # CA1852: Seal internal types dotnet_diagnostic.ca1852.severity = warning +# CA2000: Dispose objects before losing scope +dotnet_diagnostic.ca2000.severity = error + # RS0037: Enable tracking of nullability of reference types in the declared API # Our API is not annotated but new classes get nullable enabled so disable this. # We'd be happy if everything was annotated and this could be removed. diff --git a/eng/Common.globalconfig b/eng/Common.globalconfig index dd47c3b3336..57c789f0371 100644 --- a/eng/Common.globalconfig +++ b/eng/Common.globalconfig @@ -321,9 +321,6 @@ dotnet_diagnostic.CA1837.severity = suggestion # Avoid 'StringBuilder' parameters for P/Invokes dotnet_diagnostic.CA1838.severity = warning -# Dispose objects before losing scope -dotnet_diagnostic.CA2000.severity = none - # Do not lock on objects with weak identity dotnet_diagnostic.CA2002.severity = none diff --git a/src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs b/src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs index 0e83218dce7..4f35fa823fe 100644 --- a/src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs +++ b/src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs @@ -18,7 +18,7 @@ public async Task EmitConsoleMessages() { StringBuilder sb = new StringBuilder(); - using (TextWriter writer = OutOfProcServerNode.RedirectConsoleWriter.Create(text => sb.Append(text))) + using (var writer = OutOfProcServerNode.RedirectConsoleWriter.Create(text => sb.Append(text))) { writer.WriteLine("Line 1"); await Task.Delay(80); // should be somehow bigger than `RedirectConsoleWriter` flush period - see its constructor diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 33fba22ca1e..0cd7cf3c08c 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -2004,7 +2004,7 @@ private Dictionary BuildGraph( IReadOnlyDictionary> targetsPerNode, GraphBuildRequestData graphBuildRequestData) { - var waitHandle = new AutoResetEvent(true); + using var waitHandle = new AutoResetEvent(true); var graphBuildStateLock = new object(); var blockedNodes = new HashSet(projectGraph.ProjectNodes); diff --git a/src/Build/BackEnd/BuildManager/LegacyThreadingData.cs b/src/Build/BackEnd/BuildManager/LegacyThreadingData.cs index 5670a61e3f1..33dc9fe2cc4 100644 --- a/src/Build/BackEnd/BuildManager/LegacyThreadingData.cs +++ b/src/Build/BackEnd/BuildManager/LegacyThreadingData.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; using Microsoft.Build.BackEnd; @@ -82,6 +83,7 @@ internal int MainThreadSubmissionId /// Given a submission ID, assign it "start" and "finish" events to track its use of /// the legacy thread. /// + [SuppressMessage("Microsoft.Dispose", "CA2000:Dispose objects before losing scope", Justification = "The events are disposed in UnregisterSubmissionForLegacyThread")] internal void RegisterSubmissionForLegacyThread(int submissionId) { lock (_legacyThreadingEventsLock) @@ -104,6 +106,10 @@ internal void UnregisterSubmissionForLegacyThread(int submissionId) { ErrorUtilities.VerifyThrow(_legacyThreadingEventsById.ContainsKey(submissionId), "Submission {0} should have been previously registered with LegacyThreadingData", submissionId); + // Dispose the events + _legacyThreadingEventsById[submissionId].Item1?.Dispose(); + _legacyThreadingEventsById[submissionId].Item2?.Dispose(); + _legacyThreadingEventsById.Remove(submissionId); } } diff --git a/src/Build/BackEnd/Node/OutOfProcServerNode.cs b/src/Build/BackEnd/Node/OutOfProcServerNode.cs index 6b2715903e7..4f9e62b9343 100644 --- a/src/Build/BackEnd/Node/OutOfProcServerNode.cs +++ b/src/Build/BackEnd/Node/OutOfProcServerNode.cs @@ -4,6 +4,9 @@ using System; using System.Collections.Concurrent; using System.IO; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.Build.BackEnd; @@ -430,28 +433,115 @@ private void HandleServerNodeBuildCommand(ServerNodeBuildCommand command) _shutdownReason = _cancelRequested ? NodeEngineShutdownReason.BuildComplete : NodeEngineShutdownReason.BuildCompleteReuse; _shutdownEvent.Set(); } - internal sealed class RedirectConsoleWriter : StringWriter + + internal sealed class RedirectConsoleWriter : TextWriter { private readonly Action _writeCallback; private readonly Timer _timer; private readonly TextWriter _syncWriter; + private readonly StringWriter _internalWriter; + private RedirectConsoleWriter(Action writeCallback) { _writeCallback = writeCallback; - _syncWriter = Synchronized(this); + _internalWriter = new StringWriter(); + _syncWriter = Synchronized(_internalWriter); _timer = new Timer(TimerCallback, null, 0, 40); } + public override Encoding Encoding => _internalWriter.Encoding; + public static TextWriter Create(Action writeCallback) { - RedirectConsoleWriter writer = new(writeCallback); - return writer._syncWriter; + RedirectConsoleWriter writer = new RedirectConsoleWriter(writeCallback); + + return writer; } + public override void Flush() + { + var sb = _internalWriter.GetStringBuilder(); + string captured = sb.ToString(); + sb.Clear(); + + _writeCallback(captured); + _internalWriter.Flush(); + } + + public override void Write(char value) => _syncWriter.Write(value); + + public override void Write(char[]? buffer) => _syncWriter.Write(buffer); + + public override void Write(char[] buffer, int index, int count) => _syncWriter.Write(buffer, index, count); + + public override void Write(bool value) => _syncWriter.Write(value); + + public override void Write(int value) => _syncWriter.Write(value); + + public override void Write(uint value) => _syncWriter.Write(value); + + public override void Write(long value) => _syncWriter.Write(value); + + public override void Write(ulong value) => _syncWriter.Write(value); + + public override void Write(float value) => _syncWriter.Write(value); + + public override void Write(double value) => _syncWriter.Write(value); + + public override void Write(decimal value) => _syncWriter.Write(value); + + public override void Write(string? value) => _syncWriter.Write(value); + + public override void Write(object? value) => _syncWriter.Write(value); + + public override void Write(string format, object? arg0) => _syncWriter.Write(format, arg0); + + public override void Write(string format, object? arg0, object? arg1) => _syncWriter.Write(format, arg0, arg1); + + public override void Write(string format, object? arg0, object? arg1, object? arg2) => _syncWriter.Write(format, arg0, arg1, arg2); + + public override void Write(string format, params object?[] arg) => _syncWriter.WriteLine(format, arg); + + public override void WriteLine() => _syncWriter.WriteLine(); + + public override void WriteLine(char value) => _syncWriter.WriteLine(value); + + public override void WriteLine(decimal value) => _syncWriter.WriteLine(value); + + public override void WriteLine(char[]? buffer) => _syncWriter.WriteLine(buffer); + + public override void WriteLine(char[] buffer, int index, int count) => _syncWriter.WriteLine(buffer, index, count); + + public override void WriteLine(bool value) => _syncWriter.WriteLine(value); + + public override void WriteLine(int value) => _syncWriter.WriteLine(value); + + public override void WriteLine(uint value) => _syncWriter.WriteLine(value); + + public override void WriteLine(long value) => _syncWriter.WriteLine(value); + + public override void WriteLine(ulong value) => _syncWriter.WriteLine(value); + + public override void WriteLine(float value) => _syncWriter.WriteLine(value); + + public override void WriteLine(double value) => _syncWriter.WriteLine(value); + + public override void WriteLine(string? value) => _syncWriter.WriteLine(value); + + public override void WriteLine(object? value) => _syncWriter.WriteLine(value); + + public override void WriteLine(string format, object? arg0) => _syncWriter.WriteLine(format, arg0); + + public override void WriteLine(string format, object? arg0, object? arg1) => _syncWriter.WriteLine(format, arg0, arg1); + + public override void WriteLine(string format, object? arg0, object? arg1, object? arg2) => _syncWriter.WriteLine(format, arg0, arg1, arg2); + + public override void WriteLine(string format, params object?[] arg) => _syncWriter.WriteLine(format, arg); + private void TimerCallback(object? state) { - if (GetStringBuilder().Length > 0) + if (_internalWriter.GetStringBuilder().Length > 0) { _syncWriter.Flush(); } @@ -463,20 +553,11 @@ protected override void Dispose(bool disposing) { _timer.Dispose(); Flush(); + _internalWriter?.Dispose(); } base.Dispose(disposing); } - - public override void Flush() - { - var sb = GetStringBuilder(); - var captured = sb.ToString(); - sb.Clear(); - _writeCallback(captured); - - base.Flush(); - } } } } diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs index eda42874f86..a42c7a08078 100644 --- a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs +++ b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs @@ -698,9 +698,21 @@ public void CacheIfPossible() { if (IsCacheable) { - using ITranslator translator = GetConfigurationTranslator(TranslationDirection.WriteToStream); + string cacheFile = GetCacheFile(); + try + { + Directory.CreateDirectory(Path.GetDirectoryName(cacheFile)); + using Stream stream = File.Create(cacheFile); + using ITranslator translator = GetConfigurationTranslator(TranslationDirection.WriteToStream, stream); + + _project.Cache(translator); + } + catch (Exception e) when (e is DirectoryNotFoundException or UnauthorizedAccessException) + { + ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e); + throw; + } - _project.Cache(translator); _baseLookup = null; IsCached = true; @@ -726,9 +738,19 @@ public void RetrieveFromCache() return; } - using ITranslator translator = GetConfigurationTranslator(TranslationDirection.ReadFromStream); + string cacheFile = GetCacheFile(); + try + { + using Stream stream = File.OpenRead(cacheFile); + using ITranslator translator = GetConfigurationTranslator(TranslationDirection.ReadFromStream, stream); - _project.RetrieveFromCache(translator); + _project.RetrieveFromCache(translator); + } + catch (Exception e) when (e is DirectoryNotFoundException or UnauthorizedAccessException) + { + ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e); + throw; + } IsCached = false; } @@ -939,6 +961,7 @@ internal void ApplyTransferredState(ProjectInstance instance) internal string GetCacheFile() { string filename = Path.Combine(FileUtilities.GetCacheDirectory(), String.Format(CultureInfo.InvariantCulture, "Configuration{0}.cache", _configId)); + return filename; } @@ -1024,27 +1047,10 @@ private static string ResolveToolsVersion(BuildRequestData data, string defaultT /// /// Gets the translator for this configuration. /// - private ITranslator GetConfigurationTranslator(TranslationDirection direction) - { - string cacheFile = GetCacheFile(); - try - { - if (direction == TranslationDirection.WriteToStream) - { - Directory.CreateDirectory(Path.GetDirectoryName(cacheFile)); - return BinaryTranslator.GetWriteTranslator(File.Create(cacheFile)); - } - else - { + private ITranslator GetConfigurationTranslator(TranslationDirection direction, Stream stream) => + direction == TranslationDirection.WriteToStream + ? BinaryTranslator.GetWriteTranslator(stream) // Not using sharedReadBuffer because this is not a memory stream and so the buffer won't be used anyway. - return BinaryTranslator.GetReadTranslator(File.OpenRead(cacheFile), InterningBinaryReader.PoolingBuffer); - } - } - catch (Exception e) when (e is DirectoryNotFoundException || e is UnauthorizedAccessException) - { - ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e); - throw; - } - } + : BinaryTranslator.GetReadTranslator(stream, InterningBinaryReader.PoolingBuffer); } } diff --git a/src/Build/BackEnd/Shared/TargetResult.cs b/src/Build/BackEnd/Shared/TargetResult.cs index d435d1c3606..5418d5aee52 100644 --- a/src/Build/BackEnd/Shared/TargetResult.cs +++ b/src/Build/BackEnd/Shared/TargetResult.cs @@ -245,15 +245,23 @@ internal void CacheItems(int configId, string targetName) return; } - using ITranslator translator = GetResultsCacheTranslator(configId, targetName, TranslationDirection.WriteToStream); + string cacheFile = GetCacheFile(configId, targetName); + Directory.CreateDirectory(Path.GetDirectoryName(cacheFile)); - // If the translator is null, it means these results were cached once before. Since target results are immutable once they - // have been created, there is no point in writing them again. - if (translator != null) + // If the file doesn't already exists, then we haven't cached this once before. We need to cache it again since it could have changed. + if (!FileSystems.Default.FileExists(cacheFile)) { - TranslateItems(translator); - _items = null; - _cacheInfo = new CacheInfo(configId, targetName); + using Stream stream = File.Create(cacheFile); + using ITranslator translator = GetResultsCacheTranslator(TranslationDirection.WriteToStream, stream); + + // If the translator is null, it means these results were cached once before. Since target results are immutable once they + // have been created, there is no point in writing them again. + if (translator != null) + { + TranslateItems(translator); + _items = null; + _cacheInfo = new CacheInfo(configId, targetName); + } } } } @@ -279,7 +287,9 @@ private void RetrieveItemsFromCache() { if (_items == null) { - using ITranslator translator = GetResultsCacheTranslator(_cacheInfo.ConfigId, _cacheInfo.TargetName, TranslationDirection.ReadFromStream); + string cacheFile = GetCacheFile(_cacheInfo.ConfigId, _cacheInfo.TargetName); + using Stream stream = File.OpenRead(cacheFile); + using ITranslator translator = GetResultsCacheTranslator(TranslationDirection.ReadFromStream, stream); TranslateItems(translator); _cacheInfo = new CacheInfo(); @@ -339,25 +349,10 @@ private void TranslateItems(ITranslator translator) /// /// Gets the translator for this configuration. /// - private static ITranslator GetResultsCacheTranslator(int configId, string targetToCache, TranslationDirection direction) - { - string cacheFile = GetCacheFile(configId, targetToCache); - if (direction == TranslationDirection.WriteToStream) - { - Directory.CreateDirectory(Path.GetDirectoryName(cacheFile)); - if (FileSystems.Default.FileExists(cacheFile)) - { - // If the file already exists, then we have cached this once before. No need to cache it again since it cannot have changed. - return null; - } - - return BinaryTranslator.GetWriteTranslator(File.Create(cacheFile)); - } - else - { - return BinaryTranslator.GetReadTranslator(File.OpenRead(cacheFile), InterningBinaryReader.PoolingBuffer); - } - } + private static ITranslator GetResultsCacheTranslator(TranslationDirection direction, Stream stream) => + direction == TranslationDirection.WriteToStream + ? BinaryTranslator.GetWriteTranslator(stream) + : BinaryTranslator.GetReadTranslator(stream, InterningBinaryReader.PoolingBuffer); /// /// Information about where the cache for the items in this result are stored. diff --git a/src/Build/Evaluation/IntrinsicFunctions.cs b/src/Build/Evaluation/IntrinsicFunctions.cs index 6f8c5ed00f6..92c09ec6c01 100644 --- a/src/Build/Evaluation/IntrinsicFunctions.cs +++ b/src/Build/Evaluation/IntrinsicFunctions.cs @@ -291,6 +291,7 @@ internal static object GetRegistryValueFromView(string keyName, string valueName return string.Empty; } +#pragma warning disable CA2000 // Dispose objects before losing scope is false positive here. using (RegistryKey key = GetBaseKeyFromKeyName(keyName, view, out string subKeyName)) { if (key != null) @@ -311,6 +312,7 @@ internal static object GetRegistryValueFromView(string keyName, string valueName } } } +#pragma warning restore CA2000 // Dispose objects before losing scope } } @@ -446,12 +448,13 @@ internal static object StableStringHash(string toHash, StringHashingAlgorithm al private static string CalculateSha256(string toHash) { - var sha = System.Security.Cryptography.SHA256.Create(); + using var sha = System.Security.Cryptography.SHA256.Create(); var hashResult = new StringBuilder(); foreach (byte theByte in sha.ComputeHash(Encoding.UTF8.GetBytes(toHash))) { hashResult.Append(theByte.ToString("x2")); } + return hashResult.ToString(); } diff --git a/src/Build/Evaluation/LazyItemEvaluator.IncludeOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.IncludeOperation.cs index 9dfd281b165..d40ea3145fa 100644 --- a/src/Build/Evaluation/LazyItemEvaluator.IncludeOperation.cs +++ b/src/Build/Evaluation/LazyItemEvaluator.IncludeOperation.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.Build.Construction; using Microsoft.Build.Eventing; @@ -33,6 +34,7 @@ public IncludeOperation(IncludeOperationBuilder builder, LazyItemEvaluator SelectItems(OrderedItemDataCollection.Builder listBuilder, ImmutableHashSet globsToIgnore) { ImmutableArray.Builder? itemsToAdd = null; diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs index fe63676c1d2..46a4ac1785d 100644 --- a/src/Build/Instance/ProjectInstance.cs +++ b/src/Build/Instance/ProjectInstance.cs @@ -2768,14 +2768,26 @@ private static ProjectInstance[] GenerateSolutionWrapperUsingOldOM( } } - XmlReaderSettings xrs = new XmlReaderSettings(); - xrs.DtdProcessing = DtdProcessing.Ignore; - - ProjectRootElement projectRootElement = new ProjectRootElement(XmlReader.Create(new StringReader(wrapperProjectXml), xrs), projectRootElementCache, isExplicitlyLoaded, - preserveFormatting: false); - projectRootElement.DirectoryPath = Path.GetDirectoryName(projectFile); - ProjectInstance instance = new ProjectInstance(projectRootElement, globalProperties, toolsVersion, buildParameters, loggingService, projectBuildEventContext, sdkResolverService, submissionId); - return new ProjectInstance[] { instance }; + XmlReaderSettings xrs = new XmlReaderSettings + { + DtdProcessing = DtdProcessing.Ignore + }; + + StringReader sr = new StringReader(wrapperProjectXml); + using (XmlReader xmlReader = XmlReader.Create(sr, xrs)) + { + ProjectRootElement projectRootElement = new( + xmlReader, + projectRootElementCache, + isExplicitlyLoaded, + preserveFormatting: false) + { + DirectoryPath = Path.GetDirectoryName(projectFile) + }; + ProjectInstance instance = new(projectRootElement, globalProperties, toolsVersion, buildParameters, loggingService, projectBuildEventContext, sdkResolverService, submissionId); + + return new[] { instance }; + } } /// diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs index 2c49c17c8a7..72a3629a0e2 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs @@ -20,7 +20,7 @@ namespace Microsoft.Build.Logging { /// - /// Deserializes and returns BuildEventArgs-derived objects from a BinaryReader + /// Deserializes and returns BuildEventArgs-derived objects from a BinaryReader. /// public class BuildEventArgsReader : IBuildEventArgsReaderNotifications, IDisposable { @@ -186,7 +186,6 @@ internal RawRecord ReadRaw() Stream stream = _binaryReader.BaseStream.Slice(serializedEventLength); _lastSubStream = stream as SubStream; - _recordNumber += 1; return new(recordKind, stream); diff --git a/src/Build/Logging/BinaryLogger/Postprocessing/StreamExtensions.cs b/src/Build/Logging/BinaryLogger/Postprocessing/StreamExtensions.cs index 2993b3953c1..d8eca6c3848 100644 --- a/src/Build/Logging/BinaryLogger/Postprocessing/StreamExtensions.cs +++ b/src/Build/Logging/BinaryLogger/Postprocessing/StreamExtensions.cs @@ -5,6 +5,7 @@ using System.Buffers; using System.Diagnostics; using System.IO; +using System.Text; using Microsoft.Build.Shared; namespace Microsoft.Build.Logging @@ -60,7 +61,8 @@ public static byte[] ReadToEnd(this Stream stream) { if (stream.TryGetLength(out long length)) { - BinaryReader reader = new(stream); + using BinaryReader reader = new(stream, Encoding.UTF8, leaveOpen: true); + return reader.ReadBytes((int)length); } diff --git a/src/Deprecated/Engine/Engine/IntrinsicFunctions.cs b/src/Deprecated/Engine/Engine/IntrinsicFunctions.cs index e38ed53576e..53bb173c271 100644 --- a/src/Deprecated/Engine/Engine/IntrinsicFunctions.cs +++ b/src/Deprecated/Engine/Engine/IntrinsicFunctions.cs @@ -197,6 +197,7 @@ internal static object GetRegistryValueFromView(string keyName, string valueName // of that error. RegistryView view = (RegistryView)Enum.Parse(typeof(RegistryView), viewAsString, true); +#pragma warning disable CA2000 // Dispose objects before losing scope is suppressed as a false positive. using (RegistryKey key = GetBaseKeyFromKeyName(keyName, view, out subKeyName)) { if (key != null) @@ -217,6 +218,7 @@ internal static object GetRegistryValueFromView(string keyName, string valueName } } } +#pragma warning restore CA2000 // Dispose objects before losing scope } } diff --git a/src/Deprecated/Engine/LocalProvider/LocalNode.cs b/src/Deprecated/Engine/LocalProvider/LocalNode.cs index 73869190849..ebf9109bf72 100644 --- a/src/Deprecated/Engine/LocalProvider/LocalNode.cs +++ b/src/Deprecated/Engine/LocalProvider/LocalNode.cs @@ -9,9 +9,9 @@ using System.Collections; using System.Diagnostics; using System.IO; +using System.Security.AccessControl; using System.Threading; using Microsoft.Build.BuildEngine.Shared; -using System.Security.AccessControl; namespace Microsoft.Build.BuildEngine { @@ -217,6 +217,7 @@ private static bool CreateGlobalEvents(int nodeNumber) /// This function starts local node when process is launched and shuts it down on time out /// Called by msbuild.exe. /// + [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Agreed not to touch entries from Deprecated folder")] public static void StartLocalNodeServer(int nodeNumber) { // Create global events necessary for handshaking with the parent diff --git a/src/Directory.Build.targets b/src/Directory.Build.targets index fc6affaa7d2..f686521e233 100644 --- a/src/Directory.Build.targets +++ b/src/Directory.Build.targets @@ -13,6 +13,7 @@ $(MSBuildThisFileDirectory)Test.snk 002400000480000094000000060200000024000052534131000400000100010015c01ae1f50e8cc09ba9eac9147cf8fd9fce2cfe9f8dce4f7301c4132ca9fb50ce8cbf1df4dc18dd4d210e4345c744ecb3365ed327efdbc52603faa5e21daa11234c8c4a73e51f03bf192544581ebe107adee3a34928e39d04e524a9ce729d5090bfd7dad9d10c722c0def9ccc08ff0a03790e48bcd1f9b6c476063e1966a1c4 9d77cc7ad39b68eb + $(NoWarn);CA2000 diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index b543973746e..747a065590e 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -1201,14 +1201,13 @@ internal static void KillTree(int processIdToKill) // Grab the process handle. We want to keep this open for the duration of the function so that // it cannot be reused while we are running. - SafeProcessHandle hProcess = OpenProcess(eDesiredAccess.PROCESS_QUERY_INFORMATION, false, processIdToKill); - if (hProcess.IsInvalid) + using (SafeProcessHandle hProcess = OpenProcess(eDesiredAccess.PROCESS_QUERY_INFORMATION, false, processIdToKill)) { - return; - } + if (hProcess.IsInvalid) + { + return; + } - try - { try { // Kill this process, so that no further children can be created. @@ -1239,11 +1238,6 @@ internal static void KillTree(int processIdToKill) } } } - finally - { - // Release the handle. After this point no more children of this process exist and this process has also exited. - hProcess.Dispose(); - } } finally { @@ -1296,11 +1290,9 @@ internal static int GetParentProcessId(int processId) else #endif { - SafeProcessHandle hProcess = OpenProcess(eDesiredAccess.PROCESS_QUERY_INFORMATION, false, processId); - - if (!hProcess.IsInvalid) + using SafeProcessHandle hProcess = OpenProcess(eDesiredAccess.PROCESS_QUERY_INFORMATION, false, processId); { - try + if (!hProcess.IsInvalid) { // UNDONE: NtQueryInformationProcess will fail if we are not elevated and other process is. Advice is to change to use ToolHelp32 API's // For now just return zero and worst case we will not kill some children. @@ -1312,10 +1304,6 @@ internal static int GetParentProcessId(int processId) ParentID = (int)pbi.InheritedFromUniqueProcessId; } } - finally - { - hProcess.Dispose(); - } } } @@ -1337,34 +1325,38 @@ internal static List> GetChildProcessIds(in { // Hold the child process handle open so that children cannot die and restart with a different parent after we've started looking at it. // This way, any handle we pass back is guaranteed to be one of our actual children. +#pragma warning disable CA2000 // Dispose objects before losing scope - caller must dispose returned handles SafeProcessHandle childHandle = OpenProcess(eDesiredAccess.PROCESS_QUERY_INFORMATION, false, possibleChildProcess.Id); - if (childHandle.IsInvalid) +#pragma warning restore CA2000 // Dispose objects before losing scope { - continue; - } + if (childHandle.IsInvalid) + { + continue; + } - bool keepHandle = false; - try - { - if (possibleChildProcess.StartTime > parentStartTime) + bool keepHandle = false; + try { - int childParentProcessId = GetParentProcessId(possibleChildProcess.Id); - if (childParentProcessId != 0) + if (possibleChildProcess.StartTime > parentStartTime) { - if (parentProcessId == childParentProcessId) + int childParentProcessId = GetParentProcessId(possibleChildProcess.Id); + if (childParentProcessId != 0) { - // Add this one - myChildren.Add(new KeyValuePair(possibleChildProcess.Id, childHandle)); - keepHandle = true; + if (parentProcessId == childParentProcessId) + { + // Add this one + myChildren.Add(new KeyValuePair(possibleChildProcess.Id, childHandle)); + keepHandle = true; + } } } } - } - finally - { - if (!keepHandle) + finally { - childHandle.Dispose(); + if (!keepHandle) + { + childHandle.Dispose(); + } } } } diff --git a/src/MSBuild.UnitTests/XMake_Tests.cs b/src/MSBuild.UnitTests/XMake_Tests.cs index 1c3aed6df74..8691917bc02 100644 --- a/src/MSBuild.UnitTests/XMake_Tests.cs +++ b/src/MSBuild.UnitTests/XMake_Tests.cs @@ -1989,9 +1989,7 @@ public void TestProcessFileLoggerSwitch1() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); distributedLoggerRecords.Count.ShouldBe(0); // "Expected no distributed loggers to be attached" loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" } @@ -2010,9 +2008,7 @@ public void TestProcessFileLoggerSwitch2() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); distributedLoggerRecords.Count.ShouldBe(1); // "Expected one distributed loggers to be attached" loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" } @@ -2031,9 +2027,7 @@ public void TestProcessFileLoggerSwitch3() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); distributedLoggerRecords.Count.ShouldBe(0); // "Expected no distributed loggers to be attached" loggers.Count.ShouldBe(0); // "Expected a central loggers to be attached" @@ -2045,9 +2039,7 @@ public void TestProcessFileLoggerSwitch3() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); distributedLoggerRecords.Count.ShouldBe(0); // "Expected no distributed loggers to be attached" loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" @@ -2058,9 +2050,7 @@ public void TestProcessFileLoggerSwitch3() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); distributedLoggerRecords.Count.ShouldBe(0); // "Expected no distributed loggers to be attached" loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" } @@ -2079,9 +2069,7 @@ public void TestProcessFileLoggerSwitch4() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" distributedLoggerRecords.Count.ShouldBe(1); // "Expected a distributed logger to be attached" distributedLoggerRecords[0].ForwardingLoggerDescription.LoggerSwitchParameters.ShouldBe($"logFile={Path.Combine(Directory.GetCurrentDirectory(), "MSBuild.log")}", StringCompareShould.IgnoreCase); // "Expected parameter in logger to match parameter passed in" @@ -2094,9 +2082,7 @@ public void TestProcessFileLoggerSwitch4() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" distributedLoggerRecords.Count.ShouldBe(1); // "Expected a distributed logger to be attached" distributedLoggerRecords[0].ForwardingLoggerDescription.LoggerSwitchParameters.ShouldBe($"{fileLoggerParameters[0]};logFile={Path.Combine(Directory.GetCurrentDirectory(), "MSBuild.log")}", StringCompareShould.IgnoreCase); // "Expected parameter in logger to match parameter passed in" @@ -2109,9 +2095,7 @@ public void TestProcessFileLoggerSwitch4() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" distributedLoggerRecords.Count.ShouldBe(1); // "Expected a distributed logger to be attached" distributedLoggerRecords[0].ForwardingLoggerDescription.LoggerSwitchParameters.ShouldBe($"{fileLoggerParameters[0]};logFile={Path.Combine(Directory.GetCurrentDirectory(), "MSBuild.log")}", StringCompareShould.IgnoreCase); // "Expected parameter in logger to match parameter passed in" @@ -2124,9 +2108,7 @@ public void TestProcessFileLoggerSwitch4() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" distributedLoggerRecords.Count.ShouldBe(1); // "Expected a distributed logger to be attached" distributedLoggerRecords[0].ForwardingLoggerDescription.LoggerSwitchParameters.ShouldBe($";Parameter1;logFile={Path.Combine(Directory.GetCurrentDirectory(), "MSBuild.log")}", StringCompareShould.IgnoreCase); // "Expected parameter in logger to match parameter passed in" @@ -2139,9 +2121,7 @@ public void TestProcessFileLoggerSwitch4() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" distributedLoggerRecords.Count.ShouldBe(1); // "Expected a distributed logger to be attached" distributedLoggerRecords[0].ForwardingLoggerDescription.LoggerSwitchParameters.ShouldBe(fileLoggerParameters[0] + ";" + fileLoggerParameters[1], StringCompareShould.IgnoreCase); // "Expected parameter in logger to match parameter passed in" @@ -2152,9 +2132,7 @@ public void TestProcessFileLoggerSwitch4() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" distributedLoggerRecords.Count.ShouldBe(1); // "Expected a distributed logger to be attached" distributedLoggerRecords[0].ForwardingLoggerDescription.LoggerSwitchParameters.ShouldBe($"Parameter1;verbosity=Normal;logFile={Path.Combine(Directory.GetCurrentDirectory(), "..", "cat.log")};Parameter1", StringCompareShould.IgnoreCase); // "Expected parameter in logger to match parameter passed in" @@ -2165,9 +2143,7 @@ public void TestProcessFileLoggerSwitch4() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 2); + distributedLoggerRecords); distributedLoggerRecords[0].ForwardingLoggerDescription.LoggerSwitchParameters.ShouldBe($"Parameter1;Parameter;;;Parameter;Parameter;logFile={Path.Combine(Directory.GetCurrentDirectory(), "msbuild.log")}", StringCompareShould.IgnoreCase); // "Expected parameter in logger to match parameter passed in" } @@ -2185,9 +2161,7 @@ public void TestProcessFileLoggerSwitch5() MSBuildApp.ProcessDistributedFileLogger( distributedFileLogger, fileLoggerParameters, - distributedLoggerRecords, - loggers, - 1); + distributedLoggerRecords); distributedLoggerRecords.Count.ShouldBe(0); // "Expected no distributed loggers to be attached" loggers.Count.ShouldBe(0); // "Expected no central loggers to be attached" } diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 213b842ae6a..ba45bb4af2f 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -34,8 +34,6 @@ using Microsoft.Build.Shared; using Microsoft.Build.Shared.Debugging; using Microsoft.Build.Shared.FileSystem; -using Microsoft.Build.Utilities; -using static Microsoft.Build.CommandLine.MSBuildApp; using BinaryLogger = Microsoft.Build.Logging.BinaryLogger; using ConsoleLogger = Microsoft.Build.Logging.ConsoleLogger; using FileLogger = Microsoft.Build.Logging.FileLogger; @@ -662,6 +660,9 @@ public static ExitType Execute( ExitType exitType = ExitType.Success; ConsoleCancelEventHandler cancelHandler = Console_CancelKeyPress; + + TextWriter preprocessWriter = null; + TextWriter targetsWriter = null; try { #if FEATURE_GET_COMMANDLINE @@ -701,8 +702,6 @@ public static ExitType Execute( #else bool enableNodeReuse = false; #endif - TextWriter preprocessWriter = null; - TextWriter targetsWriter = null; bool detailedSummary = false; ISet warningsAsErrors = null; ISet warningsNotAsErrors = null; @@ -824,8 +823,18 @@ public static ExitType Execute( using (ProjectCollection collection = new(globalProperties, loggers, ToolsetDefinitionLocations.Default)) { Project project = collection.LoadProject(projectFile, globalProperties, toolsVersion); - TextWriter output = getResultOutputFile.Length > 0 ? new StreamWriter(getResultOutputFile) : Console.Out; - exitType = OutputPropertiesAfterEvaluation(getProperty, getItem, project, output); + + if (getResultOutputFile.Length == 0) + { + exitType = OutputPropertiesAfterEvaluation(getProperty, getItem, project, Console.Out); + } + else + { + using (var streamWriter = new StreamWriter(getResultOutputFile)) + { + exitType = OutputPropertiesAfterEvaluation(getProperty, getItem, project, streamWriter); + } + } collection.LogBuildFinishedEvent(exitType == ExitType.Success); } } @@ -887,8 +896,17 @@ public static ExitType Execute( if (outputPropertiesItemsOrTargetResults && targets?.Length > 0 && result is not null) { - TextWriter outputStream = getResultOutputFile.Length > 0 ? new StreamWriter(getResultOutputFile) : Console.Out; - exitType = OutputBuildInformationInJson(result, getProperty, getItem, getTargetResult, loggers, exitType, outputStream); + if (getResultOutputFile.Length == 0) + { + exitType = OutputBuildInformationInJson(result, getProperty, getItem, getTargetResult, loggers, exitType, Console.Out); + } + else + { + using (var streamWriter = new StreamWriter(getResultOutputFile)) + { + exitType = OutputBuildInformationInJson(result, getProperty, getItem, getTargetResult, loggers, exitType, streamWriter); + } + } } if (!string.IsNullOrEmpty(timerOutputFilename)) @@ -1032,6 +1050,9 @@ public static ExitType Execute( NativeMethodsShared.RestoreConsoleMode(s_originalConsoleMode); + preprocessWriter?.Dispose(); + targetsWriter?.Dispose(); + #if FEATURE_GET_COMMANDLINE MSBuildEventSource.Log.MSBuildExeStop(commandLine); #else @@ -3764,9 +3785,9 @@ private static ILogger[] ProcessLoggingSwitches( ProcessConsoleLoggerSwitch(noConsoleLogger, consoleLoggerParameters, distributedLoggerRecords, verbosity, cpuCount, loggers); } - ProcessDistributedFileLogger(distributedFileLogger, fileLoggerParameters, distributedLoggerRecords, loggers, cpuCount); + ProcessDistributedFileLogger(distributedFileLogger, fileLoggerParameters, distributedLoggerRecords); - ProcessFileLoggers(groupedFileLoggerParameters, distributedLoggerRecords, verbosity, cpuCount, loggers); + ProcessFileLoggers(groupedFileLoggerParameters, distributedLoggerRecords, cpuCount, loggers); verbosity = outVerbosity; @@ -3808,7 +3829,7 @@ internal static string AggregateParameters(string anyPrefixingParameter, string[ /// Add a file logger with the appropriate parameters to the loggers list for each /// non-empty set of file logger parameters provided. /// - private static void ProcessFileLoggers(string[][] groupedFileLoggerParameters, List distributedLoggerRecords, LoggerVerbosity verbosity, int cpuCount, List loggers) + private static void ProcessFileLoggers(string[][] groupedFileLoggerParameters, List distributedLoggerRecords, int cpuCount, List loggers) { for (int i = 0; i < groupedFileLoggerParameters.Length; i++) { @@ -4001,9 +4022,7 @@ private static DistributedLoggerRecord CreateForwardingLoggerRecord(ILogger logg internal static void ProcessDistributedFileLogger( bool distributedFileLogger, string[] fileLoggerParameters, - List distributedLoggerRecords, - List loggers, - int cpuCount) + List distributedLoggerRecords) { if (distributedFileLogger) { diff --git a/src/Shared/NodeEndpointOutOfProcBase.cs b/src/Shared/NodeEndpointOutOfProcBase.cs index 6c12448b1ad..8783318b2e5 100644 --- a/src/Shared/NodeEndpointOutOfProcBase.cs +++ b/src/Shared/NodeEndpointOutOfProcBase.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics.CodeAnalysis; #if CLR2COMPATIBILITY using Microsoft.Build.Shared.Concurrent; #else @@ -14,6 +15,7 @@ using Microsoft.Build.Shared; #if FEATURE_SECURITY_PERMISSIONS || FEATURE_PIPE_SECURITY using System.Security.AccessControl; + #endif #if FEATURE_PIPE_SECURITY && FEATURE_NAMED_PIPE_SECURITY_CONSTRUCTOR using System.Security.Principal; @@ -29,6 +31,7 @@ namespace Microsoft.Build.BackEnd /// /// This is an implementation of INodeEndpoint for the out-of-proc nodes. It acts only as a client. /// + [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "It is expected to keep the stream open for the process lifetime")] internal abstract class NodeEndpointOutOfProcBase : INodeEndpoint { #region Private Data diff --git a/src/StringTools.UnitTests/StringTools.UnitTests.net35.csproj b/src/StringTools.UnitTests/StringTools.UnitTests.net35.csproj index acea1b5a025..b63fa1e4e9e 100644 --- a/src/StringTools.UnitTests/StringTools.UnitTests.net35.csproj +++ b/src/StringTools.UnitTests/StringTools.UnitTests.net35.csproj @@ -1,4 +1,4 @@ - + @@ -14,6 +14,7 @@ true true $(DefineConstants);NET35_UNITTEST + $(NoWarn);CA2000 diff --git a/src/Tasks/AppConfig/AppConfig.cs b/src/Tasks/AppConfig/AppConfig.cs index 7f50e75cc29..0e746a573e7 100644 --- a/src/Tasks/AppConfig/AppConfig.cs +++ b/src/Tasks/AppConfig/AppConfig.cs @@ -34,7 +34,9 @@ internal void Load(string appConfigFilePath) // Need a filestream as the XmlReader doesn't support nonstandard unicode characters in path. // No need to dispose - as 'CloseInput' was passed to XmlReaderSettings FileStream fs = File.OpenRead(appConfigFilePath); +#pragma warning disable CA2000 // Dispose objects before losing scope is suppressed because the reader is disposed in the finally block reader = XmlReader.Create(fs, readerSettings); +#pragma warning restore CA2000 // Dispose objects before losing scope Read(reader); } catch (XmlException e) diff --git a/src/Tasks/BootstrapperUtil/BootstrapperBuilder.cs b/src/Tasks/BootstrapperUtil/BootstrapperBuilder.cs index 0b296e958ca..76e17988b08 100644 --- a/src/Tasks/BootstrapperUtil/BootstrapperBuilder.cs +++ b/src/Tasks/BootstrapperUtil/BootstrapperBuilder.cs @@ -225,7 +225,8 @@ public BuildResults Build(BuildSettings settings) var fi = new FileInfo(de.Value); using (FileStream fs = fi.OpenRead()) { - data = new StreamReader(fs).ReadToEnd(); + using var sr = new StreamReader(fs); + data = sr.ReadToEnd(); } resourceUpdater.AddStringResource(44, de.Key, data); @@ -835,7 +836,7 @@ private XmlDocument LoadAndValidateXmlDocument(string filePath, bool validateFil if (validate) { #pragma warning disable 618 // Using XmlValidatingReader. TODO: We need to switch to using XmlReader.Create() with validation. - var validatingReader = new XmlValidatingReader(xmlReader); + using var validatingReader = new XmlValidatingReader(xmlReader); #pragma warning restore 618 var xrSettings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore, CloseInput = true }; FileStream fs = File.OpenRead(schemaPath); @@ -1657,7 +1658,7 @@ private static string GetFileHash(string filePath) // pre-signed anwyay; this is a fallback in case we ever encounter a bootstrapper that is // not signed. #pragma warning disable SA1111, SA1009 // Closing parenthesis should be on line of last parameter - System.Security.Cryptography.SHA256 sha = System.Security.Cryptography.SHA256.Create( + using System.Security.Cryptography.SHA256 sha = System.Security.Cryptography.SHA256.Create( #if FEATURE_CRYPTOGRAPHIC_FACTORY_ALGORITHM_NAMES "System.Security.Cryptography.SHA256CryptoServiceProvider" #endif @@ -2033,7 +2034,8 @@ private static void DumpXmlToFile(XmlNode node, string fileName) { xmlwriter.Formatting = Formatting.Indented; xmlwriter.Indentation = 4; - xmlwriter.WriteNode(new XmlNodeReader(node), true); + using var xmlReader = new XmlNodeReader(node); + xmlwriter.WriteNode(xmlReader, true); } } catch (IOException) @@ -2194,7 +2196,7 @@ private static string GetPublicKeyOfFile(string fileSource) { try { - var cert = new X509Certificate(fileSource); + using var cert = new X509Certificate(fileSource); string publicKey = cert.GetPublicKeyString(); return publicKey; } diff --git a/src/Tasks/CodeTaskFactory.cs b/src/Tasks/CodeTaskFactory.cs index f863c969cbf..e1923c87f9d 100644 --- a/src/Tasks/CodeTaskFactory.cs +++ b/src/Tasks/CodeTaskFactory.cs @@ -738,7 +738,7 @@ private Assembly CompileInMemoryAssembly() // Horrible code dom / compilation declarations var codeBuilder = new StringBuilder(); - var writer = new StringWriter(codeBuilder, CultureInfo.CurrentCulture); + using var writer = new StringWriter(codeBuilder, CultureInfo.CurrentCulture); var codeGeneratorOptions = new CodeGeneratorOptions { BlankLinesBetweenMembers = true, diff --git a/src/Tasks/DownloadFile.cs b/src/Tasks/DownloadFile.cs index efe54f514ca..71dc72e4c91 100644 --- a/src/Tasks/DownloadFile.cs +++ b/src/Tasks/DownloadFile.cs @@ -146,6 +146,7 @@ private async Task ExecuteAsync() private async Task DownloadAsync(Uri uri, CancellationToken cancellationToken) { // The main reason to use HttpClient vs WebClient is because we can pass a message handler for unit tests to mock +#pragma warning disable CA2000 // Dispose objects before losing scope because HttpClientHandler is disposed by HTTPClient.Dispose() using (var client = new HttpClient(HttpMessageHandler ?? new HttpClientHandler(), disposeHandler: true) { Timeout = TimeSpan.FromMilliseconds(Timeout) }) { // Only get the response without downloading the file so we can determine if the file is already up-to-date @@ -226,6 +227,7 @@ private async Task DownloadAsync(Uri uri, CancellationToken cancellationToken) } } } +#pragma warning restore CA2000 // Dispose objects before losing scope } /// diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index 572a19ea3bb..af988ec51d3 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -974,8 +974,8 @@ private bool IsDangerous(String filename) // XML files are only dangerous if there are unrecognized objects in them dangerous = false; - FileStream stream = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.Read); - XmlTextReader reader = new XmlTextReader(stream); + using FileStream stream = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.Read); + using XmlTextReader reader = new XmlTextReader(stream); reader.DtdProcessing = DtdProcessing.Ignore; reader.XmlResolver = null; try @@ -1622,14 +1622,13 @@ private bool NeedToRebuildSourceFile(string sourceFilePath, DateTime sourceTime, private void GetStronglyTypedResourceToProcess(ref List inputsToProcess, ref List outputsToProcess) { bool needToRebuildSTR = false; + CodeDomProvider provider = null; // The resource file isn't out of date. So check whether the STR class file is. try { if (StronglyTypedFileName == null) { - CodeDomProvider provider = null; - if (ProcessResourceFiles.TryCreateCodeDomProvider(Log, StronglyTypedLanguage, out provider)) { StronglyTypedFileName = ProcessResourceFiles.GenerateDefaultStronglyTypedFilename(provider, OutputResources[0].ItemSpec); @@ -1645,6 +1644,10 @@ private void GetStronglyTypedResourceToProcess(ref List inputsToProce _stronglyTypedResourceSuccessfullyCreated = false; return; } + finally + { + provider?.Dispose(); + } // Now we have the filename, check if it's up to date DateTime sourceTime = NativeMethodsShared.GetLastWriteFileUtcTime(Sources[0].ItemSpec); @@ -2153,11 +2156,18 @@ private void RecordFilesWritten() { if (StronglyTypedFileName == null) { - CodeDomProvider provider; - if (ProcessResourceFiles.TryCreateCodeDomProvider(Log, StronglyTypedLanguage, out provider)) + CodeDomProvider provider = null; + try { - StronglyTypedFileName = ProcessResourceFiles.GenerateDefaultStronglyTypedFilename( - provider, OutputResources[0].ItemSpec); + if (ProcessResourceFiles.TryCreateCodeDomProvider(Log, StronglyTypedLanguage, out provider)) + { + StronglyTypedFileName = ProcessResourceFiles.GenerateDefaultStronglyTypedFilename( + provider, OutputResources[0].ItemSpec); + } + } + finally + { + provider?.Dispose(); } } @@ -3412,61 +3422,69 @@ private bool HaveSystemResourcesExtensionsReference /// The generated strongly typed filename private void CreateStronglyTypedResources(ReaderInfo reader, String outFile, String inputFileName, out String sourceFile) { - CodeDomProvider provider; - if (!TryCreateCodeDomProvider(_logger, _stronglyTypedLanguage, out provider)) - { - sourceFile = null; - return; - } - - // Default the class name if we need to - if (_stronglyTypedClassName == null) - { - _stronglyTypedClassName = Path.GetFileNameWithoutExtension(outFile); - } - - // Default the filename if we need to - if (_stronglyTypedFilename == null) + CodeDomProvider provider = null; + try { - _stronglyTypedFilename = GenerateDefaultStronglyTypedFilename(provider, outFile); - } - sourceFile = this.StronglyTypedFilename; + if (!TryCreateCodeDomProvider(_logger, _stronglyTypedLanguage, out provider)) + { + sourceFile = null; + return; + } - _logger.LogMessageFromResources("GenerateResource.CreatingSTR", _stronglyTypedFilename); - // Generate the STR class - String[] errors; - bool generateInternalClass = !_stronglyTypedClassIsPublic; - // StronglyTypedResourcesNamespace can be null and this is ok. - // If it is null then the default namespace (=stronglyTypedNamespace) is used. - CodeCompileUnit ccu = StronglyTypedResourceBuilder.Create( - reader.resourcesHashTable, - _stronglyTypedClassName, - _stronglyTypedNamespace, - _stronglyTypedResourcesNamespace, - provider, - generateInternalClass, - out errors); + // Default the class name if we need to + if (_stronglyTypedClassName == null) + { + _stronglyTypedClassName = Path.GetFileNameWithoutExtension(outFile); + } - CodeGeneratorOptions codeGenOptions = new CodeGeneratorOptions(); - using (TextWriter output = new StreamWriter(_stronglyTypedFilename)) - { - provider.GenerateCodeFromCompileUnit(ccu, output, codeGenOptions); - } + // Default the filename if we need to + if (_stronglyTypedFilename == null) + { + _stronglyTypedFilename = GenerateDefaultStronglyTypedFilename(provider, outFile); + } + sourceFile = this.StronglyTypedFilename; + + _logger.LogMessageFromResources("GenerateResource.CreatingSTR", _stronglyTypedFilename); + + // Generate the STR class + String[] errors; + bool generateInternalClass = !_stronglyTypedClassIsPublic; + // StronglyTypedResourcesNamespace can be null and this is ok. + // If it is null then the default namespace (=stronglyTypedNamespace) is used. + CodeCompileUnit ccu = StronglyTypedResourceBuilder.Create( + reader.resourcesHashTable, + _stronglyTypedClassName, + _stronglyTypedNamespace, + _stronglyTypedResourcesNamespace, + provider, + generateInternalClass, + out errors); + + CodeGeneratorOptions codeGenOptions = new CodeGeneratorOptions(); + using (TextWriter output = new StreamWriter(_stronglyTypedFilename)) + { + provider.GenerateCodeFromCompileUnit(ccu, output, codeGenOptions); + } - if (errors.Length > 0) - { - _logger.LogErrorWithCodeFromResources("GenerateResource.ErrorFromCodeDom", inputFileName); - foreach (String error in errors) + if (errors.Length > 0) + { + _logger.LogErrorWithCodeFromResources("GenerateResource.ErrorFromCodeDom", inputFileName); + foreach (String error in errors) + { + _logger.LogErrorWithCodeFromResources("GenerateResource.CodeDomError", error); + } + } + else { - _logger.LogErrorWithCodeFromResources("GenerateResource.CodeDomError", error); + // No errors, and no exceptions - we presumably did create the STR class file + // and it should get added to FilesWritten. So set a flag to indicate this. + _stronglyTypedResourceSuccessfullyCreated = true; } } - else + finally { - // No errors, and no exceptions - we presumably did create the STR class file - // and it should get added to FilesWritten. So set a flag to indicate this. - _stronglyTypedResourceSuccessfullyCreated = true; + provider?.Dispose(); } } @@ -3542,15 +3560,16 @@ private void ReadResources(ReaderInfo readerInfo, IResourceReader reader, String #endif // FEATURE_RESXREADER_LIVEDESERIALIZATION /// - /// Read resources from a text format file + /// Read resources from a text format file. /// - /// Reader info - /// Input resources filename + /// Reader info. + /// Input resources filename. private void ReadTextResources(ReaderInfo reader, String fileName) { // Check for byte order marks in the beginning of the input file, but // default to UTF-8. - using (LineNumberStreamReader sr = new LineNumberStreamReader(fileName, new UTF8Encoding(true), true)) + using var fs = File.Open(fileName, FileMode.Open, FileAccess.Read, FileShare.Read); + using (LineNumberStreamReader sr = new LineNumberStreamReader(fs, new UTF8Encoding(true), true)) { StringBuilder name = new StringBuilder(255); StringBuilder value = new StringBuilder(2048); @@ -3876,8 +3895,8 @@ internal sealed class LineNumberStreamReader : StreamReader private int _lineNumber; private int _col; - internal LineNumberStreamReader(String fileName, Encoding encoding, bool detectEncoding) - : base(File.Open(fileName, FileMode.Open, FileAccess.Read, FileShare.Read), encoding, detectEncoding) + internal LineNumberStreamReader(Stream fileStream, Encoding encoding, bool detectEncoding) + : base(fileStream, encoding, detectEncoding) { _lineNumber = 1; _col = 0; diff --git a/src/Tasks/GetInstalledSDKLocations.cs b/src/Tasks/GetInstalledSDKLocations.cs index 011ce919725..e1d4bb966e7 100644 --- a/src/Tasks/GetInstalledSDKLocations.cs +++ b/src/Tasks/GetInstalledSDKLocations.cs @@ -194,7 +194,9 @@ public override bool Execute() object staticCacheDisposer = buildEngine4.GetRegisteredTaskObject(StaticSDKCacheKey, RegisteredTaskObjectLifetime.Build); if (staticCacheDisposer == null) { +#pragma warning disable CA2000 // Dispose objects before losing scope is suppressed because the object is registered with the engine and disposed of at the end of the build. BuildCacheDisposeWrapper staticDisposer = new BuildCacheDisposeWrapper(ToolLocationHelper.ClearSDKStaticCache); +#pragma warning restore CA2000 // Dispose objects before losing scope buildEngine4.RegisterTaskObject(StaticSDKCacheKey, staticDisposer, RegisteredTaskObjectLifetime.Build, allowEarlyCollection: false); } } diff --git a/src/Tasks/ManifestUtil/ManifestFormatter.cs b/src/Tasks/ManifestUtil/ManifestFormatter.cs index a4295b3da34..7171fff8e71 100644 --- a/src/Tasks/ManifestUtil/ManifestFormatter.cs +++ b/src/Tasks/ManifestUtil/ManifestFormatter.cs @@ -17,7 +17,7 @@ public static Stream Format(Stream input) { int t1 = Environment.TickCount; - var r = new XmlTextReader(input) + using var r = new XmlTextReader(input) { DtdProcessing = DtdProcessing.Ignore, WhitespaceHandling = WhitespaceHandling.None @@ -25,7 +25,7 @@ public static Stream Format(Stream input) XmlNamespaceManager nsmgr = XmlNamespaces.GetNamespaceManager(r.NameTable); var m = new MemoryStream(); - var w = new XmlTextWriter(m, Encoding.UTF8) + using var w = new XmlTextWriter(m, Encoding.UTF8) { Formatting = Formatting.Indented, Indentation = 2 diff --git a/src/Tasks/ManifestUtil/ManifestReader.cs b/src/Tasks/ManifestUtil/ManifestReader.cs index fc4afee3919..f10c0d2a963 100644 --- a/src/Tasks/ManifestUtil/ManifestReader.cs +++ b/src/Tasks/ManifestUtil/ManifestReader.cs @@ -224,7 +224,7 @@ public static Manifest ReadManifest(string manifestType, Stream input, bool pres private static Manifest Deserialize(Stream s) { s.Position = 0; - var r = new XmlTextReader(s) { DtdProcessing = DtdProcessing.Ignore }; + using var r = new XmlTextReader(s) { DtdProcessing = DtdProcessing.Ignore }; do { diff --git a/src/Tasks/ManifestUtil/ManifestWriter.cs b/src/Tasks/ManifestUtil/ManifestWriter.cs index 562cc1f1c0f..8da08fbacde 100644 --- a/src/Tasks/ManifestUtil/ManifestWriter.cs +++ b/src/Tasks/ManifestUtil/ManifestWriter.cs @@ -24,7 +24,7 @@ private static Stream Serialize(Manifest manifest) manifest.OnBeforeSave(); var m = new MemoryStream(); var s = new XmlSerializer(manifest.GetType()); - var w = new StreamWriter(m); + using var w = new StreamWriter(m, System.Text.Encoding.UTF8, bufferSize: 1024, leaveOpen: true); int t1 = Environment.TickCount; s.Serialize(w, manifest); @@ -32,6 +32,7 @@ private static Stream Serialize(Manifest manifest) w.Flush(); m.Position = 0; + return m; } diff --git a/src/Tasks/ManifestUtil/SecurityUtil.cs b/src/Tasks/ManifestUtil/SecurityUtil.cs index 9951399b793..b49395de0d6 100644 --- a/src/Tasks/ManifestUtil/SecurityUtil.cs +++ b/src/Tasks/ManifestUtil/SecurityUtil.cs @@ -205,7 +205,7 @@ private static XmlElement GetXmlElement(string targetZone, FrameworkName fn) { try { - var sr = new StreamReader(fs); + using var sr = new StreamReader(fs, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, 1024, leaveOpen: true); string data = sr.ReadToEnd(); if (!string.IsNullOrEmpty(data)) { @@ -610,7 +610,7 @@ public static void SignFile(string certThumbprint, [SupportedOSPlatform("windows")] public static void SignFile(string certPath, SecureString certPassword, Uri timestampUrl, string path) { - X509Certificate2 cert = new X509Certificate2(certPath, certPassword, X509KeyStorageFlags.PersistKeySet); + using X509Certificate2 cert = new X509Certificate2(certPath, certPassword, X509KeyStorageFlags.PersistKeySet); SignFile(cert, timestampUrl, path); } @@ -705,8 +705,9 @@ private static void SignFileInternal(X509Certificate2 cert, CmiManifestSigner2 signer; if (useSha256 && rsa is RSACryptoServiceProvider rsacsp) { - RSACryptoServiceProvider csp = SignedCmiManifest2.GetFixedRSACryptoServiceProvider(rsacsp, useSha256); - signer = new CmiManifestSigner2(csp, cert, useSha256); +#pragma warning disable CA2000 // Dispose objects before losing scope because CmiManifestSigner2 will dispose the RSACryptoServiceProvider + signer = new CmiManifestSigner2(SignedCmiManifest2.GetFixedRSACryptoServiceProvider(rsacsp, useSha256), cert, useSha256); +#pragma warning restore CA2000 // Dispose objects before losing scope } else { diff --git a/src/Tasks/ManifestUtil/TrustInfo.cs b/src/Tasks/ManifestUtil/TrustInfo.cs index ecc02c975a7..8776175eddc 100644 --- a/src/Tasks/ManifestUtil/TrustInfo.cs +++ b/src/Tasks/ManifestUtil/TrustInfo.cs @@ -534,7 +534,8 @@ public override string ToString() var m = new MemoryStream(); Write(m); m.Position = 0; - var r = new StreamReader(m); + using var r = new StreamReader(m); + return r.ReadToEnd(); } diff --git a/src/Tasks/ManifestUtil/Util.cs b/src/Tasks/ManifestUtil/Util.cs index 4d6b6ca09ea..f8bd53d1a44 100644 --- a/src/Tasks/ManifestUtil/Util.cs +++ b/src/Tasks/ManifestUtil/Util.cs @@ -209,7 +209,7 @@ public static Version GetTargetFrameworkVersion(string targetFramework) public static string GetEmbeddedResourceString(string name) { Stream s = GetEmbeddedResourceStream(name); - StreamReader r = new StreamReader(s); + using StreamReader r = new StreamReader(s); return r.ReadToEnd(); } @@ -238,10 +238,10 @@ private static void GetFileInfoImpl(string path, string targetFrameWorkVersion, length = fi.Length; Stream s = null; + HashAlgorithm hashAlg = null; try { s = fi.OpenRead(); - HashAlgorithm hashAlg; if (string.IsNullOrEmpty(targetFrameWorkVersion) || CompareFrameworkVersions(targetFrameWorkVersion, Constants.TargetFrameworkVersion40) <= 0) { @@ -269,6 +269,7 @@ private static void GetFileInfoImpl(string path, string targetFrameWorkVersion, finally { s?.Close(); + hashAlg?.Dispose(); } } @@ -473,7 +474,7 @@ public static void WriteFile(string path, string s) public static void WriteFile(string path, Stream s) { - StreamReader r = new StreamReader(s); + using StreamReader r = new StreamReader(s); WriteFile(path, r.ReadToEnd()); } @@ -520,7 +521,7 @@ public static void WriteLogFile(string filename, Stream s) } string path = Path.Combine(logPath, filename); - StreamReader r = new StreamReader(s); + using var r = new StreamReader(s, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, 1024, leaveOpen: true); string text = r.ReadToEnd(); try { @@ -538,6 +539,7 @@ public static void WriteLogFile(string filename, Stream s) catch (SecurityException) { } + s.Position = 0; } diff --git a/src/Tasks/ManifestUtil/XmlUtil.cs b/src/Tasks/ManifestUtil/XmlUtil.cs index ca35d8090a0..709aaa1e9e3 100644 --- a/src/Tasks/ManifestUtil/XmlUtil.cs +++ b/src/Tasks/ManifestUtil/XmlUtil.cs @@ -114,8 +114,8 @@ public static Stream XslTransform(string resource, Stream input, params Dictiona } } - var m = new MemoryStream(); - var w = new XmlTextWriter(m, Encoding.UTF8); + using var m = new MemoryStream(); + using var w = new XmlTextWriter(m, Encoding.UTF8); w.WriteStartDocument(); int t5 = Environment.TickCount; diff --git a/src/Tasks/ManifestUtil/mansign2.cs b/src/Tasks/ManifestUtil/mansign2.cs index dfc7d8e46c2..1e98ca0ec72 100644 --- a/src/Tasks/ManifestUtil/mansign2.cs +++ b/src/Tasks/ManifestUtil/mansign2.cs @@ -511,7 +511,8 @@ private static void ReplacePublicKeyToken(XmlDocument manifestDom, AsymmetricAlg if (snKey is RSACryptoServiceProvider rsacsp) { - cspPublicKeyBlob = (GetFixedRSACryptoServiceProvider(rsacsp, useSha256)).ExportCspBlob(false); + using var cryptoProvider = GetFixedRSACryptoServiceProvider(rsacsp, useSha256); + cspPublicKeyBlob = cryptoProvider.ExportCspBlob(false); if (cspPublicKeyBlob == null || cspPublicKeyBlob.Length == 0) { throw new CryptographicException(Win32.NTE_BAD_KEY); diff --git a/src/Tasks/ResGen.cs b/src/Tasks/ResGen.cs index 5bce63d6e60..614ed571685 100644 --- a/src/Tasks/ResGen.cs +++ b/src/Tasks/ResGen.cs @@ -289,10 +289,11 @@ public override bool Execute() // Default the filename if we need to - regardless of whether the STR was successfully generated if (StronglyTypedFileName == null) { - CodeDomProvider provider; + CodeDomProvider provider = null; try { provider = CodeDomProvider.CreateProvider(StronglyTypedLanguage); + StronglyTypedFileName = ProcessResourceFiles.GenerateDefaultStronglyTypedFilename(provider, outputFile.ItemSpec); } catch (System.Configuration.ConfigurationException) { @@ -306,8 +307,10 @@ public override bool Execute() // logged an appropriate error. return false; } - - StronglyTypedFileName = ProcessResourceFiles.GenerateDefaultStronglyTypedFilename(provider, outputFile.ItemSpec); + finally + { + provider?.Dispose(); + } } } diff --git a/src/Tasks/ResolveKeySource.cs b/src/Tasks/ResolveKeySource.cs index deade89479b..2fcfbe94245 100644 --- a/src/Tasks/ResolveKeySource.cs +++ b/src/Tasks/ResolveKeySource.cs @@ -216,7 +216,7 @@ private bool ResolveManifestKey() { bool imported = false; // first try it with no password - var cert = new X509Certificate2(); + using var cert = new X509Certificate2(); var personalStore = new X509Store(StoreName.My, StoreLocation.CurrentUser); try { diff --git a/src/Tasks/ResourceHandling/FileStreamResource.cs b/src/Tasks/ResourceHandling/FileStreamResource.cs index ea29bf5b772..39117f25e70 100644 --- a/src/Tasks/ResourceHandling/FileStreamResource.cs +++ b/src/Tasks/ResourceHandling/FileStreamResource.cs @@ -12,8 +12,11 @@ namespace Microsoft.Build.Tasks.ResourceHandling internal class FileStreamResource : IResource { public string Name { get; } + public string TypeAssemblyQualifiedName { get; } + public string OriginatingFile { get; } + public string FileName { get; } public string TypeFullName => NameUtilities.FullNameFromAssemblyQualifiedName(TypeAssemblyQualifiedName); @@ -37,7 +40,9 @@ public void AddTo(IResourceWriter writer) { if (writer is PreserializedResourceWriter preserializedResourceWriter) { +#pragma warning disable CA2000 // Dispose objects before losing scope the stream is expected to be disposed by the PreserializedResourceWriter.ResourceDataRecord FileStream fileStream = new FileStream(FileName, FileMode.Open, FileAccess.Read, FileShare.Read); +#pragma warning restore CA2000 // Dispose objects before losing scope preserializedResourceWriter.AddActivatorResource(Name, fileStream, TypeAssemblyQualifiedName, closeAfterWrite: true); } diff --git a/src/Tasks/Unzip.cs b/src/Tasks/Unzip.cs index 53ad3198125..6590a161c43 100644 --- a/src/Tasks/Unzip.cs +++ b/src/Tasks/Unzip.cs @@ -116,6 +116,7 @@ public override bool Execute() { using (FileStream stream = new FileStream(sourceFile.ItemSpec, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 0x1000, useAsync: false)) { +#pragma warning disable CA2000 // Dispose objects before losing scope because ZipArchive will dispose the stream when it is disposed. using (ZipArchive zipArchive = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: false)) { try @@ -129,6 +130,7 @@ public override bool Execute() return false; } } +#pragma warning restore CA2000 // Dispose objects before losing scope } } catch (OperationCanceledException) diff --git a/src/Tasks/WriteCodeFragment.cs b/src/Tasks/WriteCodeFragment.cs index 77128537b7a..81b2c4d9497 100644 --- a/src/Tasks/WriteCodeFragment.cs +++ b/src/Tasks/WriteCodeFragment.cs @@ -140,145 +140,151 @@ private string GenerateCode(out string extension) extension = null; bool haveGeneratedContent = false; - CodeDomProvider provider; - + CodeDomProvider provider = null; try { - provider = CodeDomProvider.CreateProvider(Language); - } - catch (SystemException e) when + try + { + provider = CodeDomProvider.CreateProvider(Language); + } + catch (SystemException e) when #if FEATURE_SYSTEM_CONFIGURATION - (e is ConfigurationException || e is SecurityException) + (e is ConfigurationException || e is SecurityException) #else (e.GetType().Name == "ConfigurationErrorsException") // TODO: catch specific exception type once it is public https://github.com/dotnet/corefx/issues/40456 #endif - { - Log.LogErrorWithCodeFromResources("WriteCodeFragment.CouldNotCreateProvider", Language, e.Message); - return null; - } - - extension = provider.FileExtension; + { + Log.LogErrorWithCodeFromResources("WriteCodeFragment.CouldNotCreateProvider", Language, e.Message); + return null; + } - var unit = new CodeCompileUnit(); + extension = provider.FileExtension; - var globalNamespace = new CodeNamespace(); - unit.Namespaces.Add(globalNamespace); + var unit = new CodeCompileUnit(); - // Declare authorship. Unfortunately CodeDOM puts this comment after the attributes. - string comment = ResourceUtilities.GetResourceString("WriteCodeFragment.Comment"); - globalNamespace.Comments.Add(new CodeCommentStatement(comment)); + var globalNamespace = new CodeNamespace(); + unit.Namespaces.Add(globalNamespace); - if (AssemblyAttributes == null) - { - return String.Empty; - } + // Declare authorship. Unfortunately CodeDOM puts this comment after the attributes. + string comment = ResourceUtilities.GetResourceString("WriteCodeFragment.Comment"); + globalNamespace.Comments.Add(new CodeCommentStatement(comment)); - // For convenience, bring in the namespaces, where many assembly attributes lie - foreach (string name in NamespaceImports) - { - globalNamespace.Imports.Add(new CodeNamespaceImport(name)); - } + if (AssemblyAttributes == null) + { + return String.Empty; + } - foreach (ITaskItem attributeItem in AssemblyAttributes) - { - // Some attributes only allow positional constructor arguments, or the user may just prefer them. - // To set those, use metadata names like "_Parameter1", "_Parameter2" etc. - // If a parameter index is skipped, it's an error. - IDictionary customMetadata = attributeItem.CloneCustomMetadata(); + // For convenience, bring in the namespaces, where many assembly attributes lie + foreach (string name in NamespaceImports) + { + globalNamespace.Imports.Add(new CodeNamespaceImport(name)); + } - // Some metadata may indicate the types of parameters. Use that metadata to determine - // the parameter types. Those metadata items will be removed from the dictionary. - IReadOnlyDictionary parameterTypes = ExtractParameterTypes(customMetadata); + foreach (ITaskItem attributeItem in AssemblyAttributes) + { + // Some attributes only allow positional constructor arguments, or the user may just prefer them. + // To set those, use metadata names like "_Parameter1", "_Parameter2" etc. + // If a parameter index is skipped, it's an error. + IDictionary customMetadata = attributeItem.CloneCustomMetadata(); - var orderedParameters = new List(new AttributeParameter?[customMetadata.Count + 1] /* max possible slots needed */); - var namedParameters = new List(); + // Some metadata may indicate the types of parameters. Use that metadata to determine + // the parameter types. Those metadata items will be removed from the dictionary. + IReadOnlyDictionary parameterTypes = ExtractParameterTypes(customMetadata); - foreach (DictionaryEntry entry in customMetadata) - { - string name = (string)entry.Key; - string value = (string)entry.Value; + var orderedParameters = new List(new AttributeParameter?[customMetadata.Count + 1] /* max possible slots needed */); + var namedParameters = new List(); - // Get the declared type information for this parameter. - // If a type is not declared, then we infer the type. - if (!parameterTypes.TryGetValue(name, out ParameterType type)) + foreach (DictionaryEntry entry in customMetadata) { - type = new ParameterType { Kind = ParameterTypeKind.Inferred }; + string name = (string)entry.Key; + string value = (string)entry.Value; + + // Get the declared type information for this parameter. + // If a type is not declared, then we infer the type. + if (!parameterTypes.TryGetValue(name, out ParameterType type)) + { + type = new ParameterType { Kind = ParameterTypeKind.Inferred }; + } + + if (name.StartsWith("_Parameter", StringComparison.OrdinalIgnoreCase)) + { + if (!Int32.TryParse(name.Substring("_Parameter".Length), out int index)) + { + Log.LogErrorWithCodeFromResources("General.InvalidValue", name, "WriteCodeFragment"); + return null; + } + + if (index > orderedParameters.Count || index < 1) + { + Log.LogErrorWithCodeFromResources("WriteCodeFragment.SkippedNumberedParameter", index); + return null; + } + + // "_Parameter01" and "_Parameter1" would overwrite each other + orderedParameters[index - 1] = new AttributeParameter { Type = type, Value = value }; + } + else + { + namedParameters.Add(new AttributeParameter { Name = name, Type = type, Value = value }); + } } - if (name.StartsWith("_Parameter", StringComparison.OrdinalIgnoreCase)) + bool encounteredNull = false; + List providedOrderedParameters = new(); + for (int i = 0; i < orderedParameters.Count; i++) { - if (!Int32.TryParse(name.Substring("_Parameter".Length), out int index)) + if (!orderedParameters[i].HasValue) { - Log.LogErrorWithCodeFromResources("General.InvalidValue", name, "WriteCodeFragment"); - return null; + // All subsequent args should be null, else a slot was missed + encounteredNull = true; + continue; } - if (index > orderedParameters.Count || index < 1) + if (encounteredNull) { - Log.LogErrorWithCodeFromResources("WriteCodeFragment.SkippedNumberedParameter", index); + Log.LogErrorWithCodeFromResources("WriteCodeFragment.SkippedNumberedParameter", i + 1 /* back to 1 based */); return null; } - // "_Parameter01" and "_Parameter1" would overwrite each other - orderedParameters[index - 1] = new AttributeParameter { Type = type, Value = value }; - } - else - { - namedParameters.Add(new AttributeParameter { Name = name, Type = type, Value = value }); + providedOrderedParameters.Add(orderedParameters[i].Value); } - } - bool encounteredNull = false; - List providedOrderedParameters = new(); - for (int i = 0; i < orderedParameters.Count; i++) - { - if (!orderedParameters[i].HasValue) - { - // All subsequent args should be null, else a slot was missed - encounteredNull = true; - continue; - } + var attribute = new CodeAttributeDeclaration(new CodeTypeReference(attributeItem.ItemSpec)); + + // We might need the type of the attribute if we need to infer the + // types of the parameters. Search for it by the given type name, + // as well as within the namespaces that we automatically import. + Lazy attributeType = new( + () => Type.GetType(attribute.Name, throwOnError: false) ?? NamespaceImports.Select(x => Type.GetType($"{x}.{attribute.Name}", throwOnError: false)).FirstOrDefault(), + System.Threading.LazyThreadSafetyMode.None); - if (encounteredNull) + if ( + !AddArguments(attribute, attributeType, providedOrderedParameters, isPositional: true) + || !AddArguments(attribute, attributeType, namedParameters, isPositional: false)) { - Log.LogErrorWithCodeFromResources("WriteCodeFragment.SkippedNumberedParameter", i + 1 /* back to 1 based */); return null; } - providedOrderedParameters.Add(orderedParameters[i].Value); + unit.AssemblyCustomAttributes.Add(attribute); + haveGeneratedContent = true; } - var attribute = new CodeAttributeDeclaration(new CodeTypeReference(attributeItem.ItemSpec)); - - // We might need the type of the attribute if we need to infer the - // types of the parameters. Search for it by the given type name, - // as well as within the namespaces that we automatically import. - Lazy attributeType = new( - () => Type.GetType(attribute.Name, throwOnError: false) ?? NamespaceImports.Select(x => Type.GetType($"{x}.{attribute.Name}", throwOnError: false)).FirstOrDefault(), - System.Threading.LazyThreadSafetyMode.None); - - if ( - !AddArguments(attribute, attributeType, providedOrderedParameters, isPositional: true) - || !AddArguments(attribute, attributeType, namedParameters, isPositional: false)) + var generatedCode = new StringBuilder(); + using (var writer = new StringWriter(generatedCode, CultureInfo.CurrentCulture)) { - return null; + provider.GenerateCodeFromCompileUnit(unit, writer, new CodeGeneratorOptions()); } - unit.AssemblyCustomAttributes.Add(attribute); - haveGeneratedContent = true; - } + string code = generatedCode.ToString(); - var generatedCode = new StringBuilder(); - using (var writer = new StringWriter(generatedCode, CultureInfo.CurrentCulture)) + // If we just generated infrastructure, don't bother returning anything + // as there's no point writing the file + return haveGeneratedContent ? code : String.Empty; + } + finally { - provider.GenerateCodeFromCompileUnit(unit, writer, new CodeGeneratorOptions()); + provider?.Dispose(); } - - string code = generatedCode.ToString(); - - // If we just generated infrastructure, don't bother returning anything - // as there's no point writing the file - return haveGeneratedContent ? code : String.Empty; } /// diff --git a/src/Tasks/XamlTaskFactory/TaskParser.cs b/src/Tasks/XamlTaskFactory/TaskParser.cs index bd9f1aa2185..626a1a92587 100644 --- a/src/Tasks/XamlTaskFactory/TaskParser.cs +++ b/src/Tasks/XamlTaskFactory/TaskParser.cs @@ -144,7 +144,9 @@ private bool ParseAsContentOrFile(string contentOrFile, string desiredRule) throw new ArgumentException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("Xaml.RuleFileNotFound", contentOrFile)); } - return ParseXamlDocument(new StreamReader(contentOrFile), desiredRule); + using var sr = new StreamReader(contentOrFile); + + return ParseXamlDocument(sr, desiredRule); } // On Windows, xml content string is not a valid path, so, maybeFullPath == null @@ -158,7 +160,9 @@ private bool ParseAsContentOrFile(string contentOrFile, string desiredRule) if (FileSystems.Default.FileExists(maybeFullPath)) { // file found, parse as a file - return ParseXamlDocument(new StreamReader(maybeFullPath), desiredRule); + using var sr = new StreamReader(maybeFullPath); + + return ParseXamlDocument(sr, desiredRule); } // @maybeFullPath is either: diff --git a/src/Tasks/XamlTaskFactory/XamlTaskFactory.cs b/src/Tasks/XamlTaskFactory/XamlTaskFactory.cs index 4094d25b62e..342fb1f30b2 100644 --- a/src/Tasks/XamlTaskFactory/XamlTaskFactory.cs +++ b/src/Tasks/XamlTaskFactory/XamlTaskFactory.cs @@ -131,7 +131,7 @@ public bool Initialize(string taskName, IDictionary ta }; // create the code provider - var codegenerator = CodeDomProvider.CreateProvider("cs"); + using var codegenerator = CodeDomProvider.CreateProvider("cs"); CompilerResults results; bool debugXamlTask = Environment.GetEnvironmentVariable("MSBUILDWRITEXAMLTASK") == "1"; if (debugXamlTask) diff --git a/src/Tasks/XslTransformation.cs b/src/Tasks/XslTransformation.cs index 2f255f143cc..f55532b9546 100644 --- a/src/Tasks/XslTransformation.cs +++ b/src/Tasks/XslTransformation.cs @@ -451,8 +451,12 @@ public XslCompiledTransform LoadXslt(bool useTrustedSettings) switch (_xslMode) { case XslModes.Xslt: - xslct.Load(XmlReader.Create(new StringReader(_data)), settings, new XmlUrlResolver()); - break; + { + using var sr = new StringReader(_data); + using var xmlReader = XmlReader.Create(sr); + xslct.Load(xmlReader, settings, new XmlUrlResolver()); + break; + } case XslModes.XsltFile: if (useTrustedSettings) { diff --git a/src/UnitTests.Shared/ObjectModelHelpers.cs b/src/UnitTests.Shared/ObjectModelHelpers.cs index ce51be22785..e5cf81e5fe3 100644 --- a/src/UnitTests.Shared/ObjectModelHelpers.cs +++ b/src/UnitTests.Shared/ObjectModelHelpers.cs @@ -1971,7 +1971,8 @@ public static void VerifyAssertLineByLine(string expected, string actual, bool i /// public static void ClearDirtyFlag(ProjectRootElement project) { - project.Save(new StringWriter()); + using var sw = new StringWriter(); + project.Save(sw); Assert.False(project.HasUnsavedChanges); }