diff --git a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs index dae21f7d5..5b96d6720 100644 --- a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs +++ b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs @@ -27,7 +27,7 @@ public abstract class AnalyzeCommandBase : PluginDriverComma private Run _run; private Tool _tool; - private TContext _rootContext; + internal TContext _rootContext; private CacheByFileHashLogger _cacheByFileHashLogger; private IDictionary _pathToHashDataMap; @@ -55,87 +55,94 @@ public string DefaultConfigurationPath public override int Run(TOptions options) { - // To correctly initialize the logger, we must first add Hashes to dataToInsert -#pragma warning disable CS0618 // Type or member is obsolete - if (options.ComputeFileHashes) -#pragma warning restore CS0618 - { - OptionallyEmittedData dataToInsert = options.DataToInsert.ToFlags(); - dataToInsert |= OptionallyEmittedData.Hashes; - - options.DataToInsert = Enum.GetValues(typeof(OptionallyEmittedData)).Cast() - .Where(oed => dataToInsert.HasFlag(oed)).ToList(); - } - - // 0. Initialize an common logger that drives all outputs. This - // object drives logging for console, statistics, etc. - using (AggregatingLogger logger = InitializeLogger(options)) + try { - // Once the logger has been correctly initialized, we can raise a warning - _rootContext = CreateContext(options, logger, RuntimeErrors); + // To correctly initialize the logger, we must first add Hashes to dataToInsert #pragma warning disable CS0618 // Type or member is obsolete if (options.ComputeFileHashes) #pragma warning restore CS0618 { - Warnings.LogObsoleteOption(_rootContext, "--hashes", SdkResources.ComputeFileHashes_ReplaceInsertHashes); - } + OptionallyEmittedData dataToInsert = options.DataToInsert.ToFlags(); + dataToInsert |= OptionallyEmittedData.Hashes; - try - { - Analyze(options, logger); - } - catch (ExitApplicationException ex) - { - // These exceptions have already been logged - ExecutionException = ex; - return FAILURE; - } - catch (Exception ex) - { - // These exceptions escaped our net and must be logged here - RuntimeErrors |= Errors.LogUnhandledEngineException(_rootContext, ex); - ExecutionException = ex; - return FAILURE; + options.DataToInsert = Enum.GetValues(typeof(OptionallyEmittedData)).Cast() + .Where(oed => dataToInsert.HasFlag(oed)).ToList(); } - finally + + // 0. Initialize an common logger that drives all outputs. This + // object drives logging for console, statistics, etc. + using (AggregatingLogger logger = InitializeLogger(options)) { - logger.AnalysisStopped(RuntimeErrors); + // Once the logger has been correctly initialized, we can raise a warning + _rootContext = CreateContext(options, logger, RuntimeErrors); +#pragma warning disable CS0618 // Type or member is obsolete + if (options.ComputeFileHashes) +#pragma warning restore CS0618 + { + Warnings.LogObsoleteOption(_rootContext, "--hashes", SdkResources.ComputeFileHashes_ReplaceInsertHashes); + } + + try + { + Analyze(options, logger); + } + catch (ExitApplicationException ex) + { + // These exceptions have already been logged + ExecutionException = ex; + return FAILURE; + } + catch (Exception ex) + { + // These exceptions escaped our net and must be logged here + RuntimeErrors |= Errors.LogUnhandledEngineException(_rootContext, ex); + ExecutionException = ex; + return FAILURE; + } + finally + { + logger.AnalysisStopped(RuntimeErrors); + } } - } - bool succeeded = (RuntimeErrors & ~RuntimeConditions.Nonfatal) == RuntimeConditions.None; + bool succeeded = (RuntimeErrors & ~RuntimeConditions.Nonfatal) == RuntimeConditions.None; - if (succeeded) - { - try + if (succeeded) { - ProcessBaseline(_rootContext, options, FileSystem); - } - catch (Exception ex) - { - RuntimeErrors |= RuntimeConditions.ExceptionProcessingBaseline; - ExecutionException = ex; - return FAILURE; - } + try + { + ProcessBaseline(_rootContext, options, FileSystem); + } + catch (Exception ex) + { + RuntimeErrors |= RuntimeConditions.ExceptionProcessingBaseline; + ExecutionException = ex; + return FAILURE; + } - try - { - PostLogFile(options.PostUri, options.OutputFilePath, FileSystem); + try + { + PostLogFile(options.PostUri, options.OutputFilePath, FileSystem); + } + catch (Exception ex) + { + RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile; + ExecutionException = ex; + return FAILURE; + } } - catch (Exception ex) + + if (options.RichReturnCode) { - RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile; - ExecutionException = ex; - return FAILURE; + return (int)RuntimeErrors; } - } - if (options.RichReturnCode) + return succeeded ? SUCCESS : FAILURE; + } + finally { - return (int)RuntimeErrors; + _rootContext?.Dispose(); } - - return succeeded ? SUCCESS : FAILURE; } private void Analyze(TOptions options, AggregatingLogger logger) diff --git a/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs index db658d171..744645ae1 100644 --- a/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs +++ b/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs @@ -33,7 +33,7 @@ public abstract class MultithreadedAnalyzeCommandBase : Plug private Run _run; private Tool _tool; private bool _computeHashes; - private TContext _rootContext; + internal TContext _rootContext; private int _fileContextsCount; private Channel _hashChannel; private OptionallyEmittedData _dataToInsert; @@ -66,71 +66,78 @@ public string DefaultConfigurationPath public override int Run(TOptions options) { - // Initialize an common logger that drives all outputs. This - // object drives logging for console, statistics, etc. - using (AggregatingLogger logger = InitializeLogger(options)) + try { - try - { - Analyze(options, logger); - } - catch (ExitApplicationException ex) + // Initialize an common logger that drives all outputs. This + // object drives logging for console, statistics, etc. + using (AggregatingLogger logger = InitializeLogger(options)) { - // These exceptions have already been logged - ExecutionException = ex; - return FAILURE; - } - catch (Exception ex) - { - ex = ex.InnerException ?? ex; + try + { + Analyze(options, logger); + } + catch (ExitApplicationException ex) + { + // These exceptions have already been logged + ExecutionException = ex; + return FAILURE; + } + catch (Exception ex) + { + ex = ex.InnerException ?? ex; - if (!(ex is ExitApplicationException)) + if (!(ex is ExitApplicationException)) + { + // These exceptions escaped our net and must be logged here + RuntimeErrors |= Errors.LogUnhandledEngineException(_rootContext, ex); + } + ExecutionException = ex; + return FAILURE; + } + finally { - // These exceptions escaped our net and must be logged here - RuntimeErrors |= Errors.LogUnhandledEngineException(_rootContext, ex); + logger.AnalysisStopped(RuntimeErrors); } - ExecutionException = ex; - return FAILURE; } - finally - { - logger.AnalysisStopped(RuntimeErrors); - } - } - bool succeeded = (RuntimeErrors & ~RuntimeConditions.Nonfatal) == RuntimeConditions.None; + bool succeeded = (RuntimeErrors & ~RuntimeConditions.Nonfatal) == RuntimeConditions.None; - if (succeeded) - { - try + if (succeeded) { - ProcessBaseline(_rootContext, options, FileSystem); - } - catch (Exception ex) - { - RuntimeErrors |= RuntimeConditions.ExceptionProcessingBaseline; - ExecutionException = ex; - return FAILURE; - } + try + { + ProcessBaseline(_rootContext, options, FileSystem); + } + catch (Exception ex) + { + RuntimeErrors |= RuntimeConditions.ExceptionProcessingBaseline; + ExecutionException = ex; + return FAILURE; + } - try - { - PostLogFile(options.PostUri, options.OutputFilePath, FileSystem); + try + { + PostLogFile(options.PostUri, options.OutputFilePath, FileSystem); + } + catch (Exception ex) + { + RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile; + ExecutionException = ex; + return FAILURE; + } } - catch (Exception ex) + + if (options.RichReturnCode) { - RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile; - ExecutionException = ex; - return FAILURE; + return (int)RuntimeErrors; } - } - if (options.RichReturnCode) + return succeeded ? SUCCESS : FAILURE; + } + finally { - return (int)RuntimeErrors; + _rootContext?.Dispose(); } - - return succeeded ? SUCCESS : FAILURE; } private void Analyze(TOptions analyzeOptions, AggregatingLogger logger) @@ -805,10 +812,9 @@ private SupportedPlatform GetCurrentRunningOS() #endif } - protected virtual void AnalyzeTargets( - TOptions options, - TContext rootContext, - IEnumerable> skimmers) + protected virtual void AnalyzeTargets(TOptions options, + TContext rootContext, + IEnumerable> skimmers) { var disabledSkimmers = new SortedSet(); diff --git a/src/Sarif/Core/SarifLog.cs b/src/Sarif/Core/SarifLog.cs index 3f8fcc4d6..bf5aecd74 100644 --- a/src/Sarif/Core/SarifLog.cs +++ b/src/Sarif/Core/SarifLog.cs @@ -87,7 +87,7 @@ public static async Task Post(Uri postUri, if (!fileSystem.FileExists(filePath)) { - throw new ArgumentException(nameof(filePath)); + throw new ArgumentException($"File path does not exist: '{filePath}'", nameof(filePath)); } using Stream fileStream = fileSystem.FileOpenRead(filePath); diff --git a/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs b/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs index 39b5fd0cb..834dfa9d2 100644 --- a/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs +++ b/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs @@ -40,6 +40,19 @@ public AnalyzeCommandBaseTests(ITestOutputHelper output) this.Output = output; } + [Fact] + public void AnalyzeCommandBase_RootContextIsDisposed() + { + var options = new TestAnalyzeOptions(); + var singleThreadedCommand = new TestAnalyzeCommand(); + int result = singleThreadedCommand.Run(options); + singleThreadedCommand._rootContext.Disposed.Should().BeTrue(); + + var multithreadedAnalyzeCommand = new TestMultithreadedAnalyzeCommand(); + result = singleThreadedCommand.Run(options); + singleThreadedCommand._rootContext.Disposed.Should().BeTrue(); + } + private void ExceptionTestHelper( RuntimeConditions runtimeConditions, ExitReason expectedExitReason = ExitReason.None, @@ -1237,10 +1250,10 @@ private void AnalyzeScenarios(int[] scenarios) testCase.FileSystem = null; testCase.Files = multiThreadTargets; - Run runMultiThread = RunAnalyzeCommand(options, testCase, multithreaded: true); + Run runMultithreaded = RunAnalyzeCommand(options, testCase, multithreaded: true); - runMultiThread.Results.Should().BeEquivalentTo(runSingleThread.Results); - runMultiThread.Artifacts.Should().BeEquivalentTo(runSingleThread.Artifacts); + runMultithreaded.Results.Should().BeEquivalentTo(runSingleThread.Results); + runMultithreaded.Artifacts.Should().BeEquivalentTo(runSingleThread.Artifacts); } } diff --git a/src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs b/src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs index f500a8078..05ef9db7f 100644 --- a/src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs +++ b/src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs @@ -5,6 +5,8 @@ using System.Collections.Generic; using System.Reflection; +using FluentAssertions; + namespace Microsoft.CodeAnalysis.Sarif.Driver { public class TestAnalyzeCommand : AnalyzeCommandBase, ITestAnalyzeCommand @@ -70,5 +72,12 @@ protected override void ProcessBaseline(IAnalysisContext context, TestAnalyzeOpt base.ProcessBaseline(context, options, fileSystem); } + + public override int Run(TestAnalyzeOptions options) + { + int result = base.Run(options); + this._rootContext?.Disposed.Should().BeTrue(); + return result; + } } } diff --git a/src/Test.UnitTests.Sarif.Driver/TestMultithreadedAnalyzeCommand.cs b/src/Test.UnitTests.Sarif.Driver/TestMultithreadedAnalyzeCommand.cs index 2ea8bd7f4..dbf105a82 100644 --- a/src/Test.UnitTests.Sarif.Driver/TestMultithreadedAnalyzeCommand.cs +++ b/src/Test.UnitTests.Sarif.Driver/TestMultithreadedAnalyzeCommand.cs @@ -5,6 +5,8 @@ using System.Collections.Generic; using System.Reflection; +using FluentAssertions; + using Microsoft.CodeAnalysis.Test.Utilities.Sarif; namespace Microsoft.CodeAnalysis.Sarif.Driver @@ -59,7 +61,9 @@ protected override void ValidateOptions(TestAnalyzeOptions options, TestAnalysis public int Run(AnalyzeOptionsBase options) { - return base.Run((TestAnalyzeOptions)options); + int result = base.Run((TestAnalyzeOptions)options); + this._rootContext?.Disposed.Should().BeTrue(); + return result; } protected override TestAnalysisContext DetermineApplicabilityAndAnalyze(TestAnalysisContext context, IEnumerable> skimmers, ISet disabledSkimmers) diff --git a/src/Test.UnitTests.Sarif.Multitool.Library/SuppressCommandTests.cs b/src/Test.UnitTests.Sarif.Multitool.Library/SuppressCommandTests.cs index 63f4ddb08..291176f2f 100644 --- a/src/Test.UnitTests.Sarif.Multitool.Library/SuppressCommandTests.cs +++ b/src/Test.UnitTests.Sarif.Multitool.Library/SuppressCommandTests.cs @@ -165,7 +165,7 @@ private static void VerifySuppressCommand(SuppressOptions options) if (options.Timestamps && suppression.TryGetProperty("timeUtc", out DateTime timeUtc)) { - timeUtc.Should().BeCloseTo(DateTime.UtcNow); + timeUtc.Should().BeCloseTo(DateTime.UtcNow, precision: 500); } if (options.ExpiryInDays > 0 && suppression.TryGetProperty("expiryUtc", out DateTime expiryUtc)) diff --git a/src/Test.Utilities.Sarif/TestAnalysisContext.cs b/src/Test.Utilities.Sarif/TestAnalysisContext.cs index 3729e12b0..69c4bcddc 100644 --- a/src/Test.Utilities.Sarif/TestAnalysisContext.cs +++ b/src/Test.Utilities.Sarif/TestAnalysisContext.cs @@ -33,8 +33,11 @@ public class TestAnalysisContext : IAnalysisContext public DefaultTraces Traces { get; set; } + public bool Disposed { get; private set; } + public void Dispose() { + Disposed = true; } } }