diff --git a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs index 203366aee..a5182286b 100644 --- a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs +++ b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Runtime.InteropServices; @@ -568,6 +569,8 @@ protected virtual void AnalyzeTargets( } } + this.CheckIncompatibleRules(skimmers, rootContext, disabledSkimmers); + if (disabledSkimmers.Count == skimmers.Count()) { Errors.LogAllRulesExplicitlyDisabled(rootContext); @@ -792,6 +795,38 @@ protected void ThrowExitApplicationException(TContext context, ExitReason exitRe }; } + internal void CheckIncompatibleRules(IEnumerable> skimmers, TContext context, ISet disabledSkimmers) + { + var availableRules = new Dictionary>(); + + foreach (Skimmer skimmer in skimmers) + { + if (disabledSkimmers.Contains(skimmer.Id)) + { + continue; + } + + availableRules[skimmer.Id] = skimmer; + } + + foreach (KeyValuePair> entry in availableRules) + { + if (entry.Value.IncompatibleRuleIds?.Any() != true) + { + continue; + } + + foreach (string incompatibleRuleId in entry.Value.IncompatibleRuleIds) + { + if (availableRules.ContainsKey(incompatibleRuleId)) + { + Errors.LogIncompatibleRules(context, entry.Key, incompatibleRuleId); + ThrowExitApplicationException(context, ExitReason.IncompatibleRulesDetected); + } + } + } + } + protected virtual ISet> InitializeSkimmers(ISet> skimmers, TContext context) { SortedSet> disabledSkimmers = new SortedSet>(SkimmerIdComparer.Instance); diff --git a/src/Sarif.Driver/Sdk/ExitReason.cs b/src/Sarif.Driver/Sdk/ExitReason.cs index ae6c6cd41..36f68034e 100644 --- a/src/Sarif.Driver/Sdk/ExitReason.cs +++ b/src/Sarif.Driver/Sdk/ExitReason.cs @@ -3,6 +3,7 @@ namespace Microsoft.CodeAnalysis.Sarif.Driver { + // Be sure to append new values for this enum to avoid changing existing member values. public enum ExitReason { None, @@ -14,6 +15,7 @@ public enum ExitReason NoValidAnalysisTargets, InvalidCommandLineOption, ExceptionProcessingBaseline, - ExceptionPostingLogFile + ExceptionPostingLogFile, + IncompatibleRulesDetected } } diff --git a/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs index 9974d2eec..3311b4b5b 100644 --- a/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs +++ b/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs @@ -865,6 +865,8 @@ protected virtual void AnalyzeTargets(TOptions options, } } + this.CheckIncompatibleRules(skimmers, rootContext, disabledSkimmers); + if (disabledSkimmers.Count == skimmers.Count()) { Errors.LogAllRulesExplicitlyDisabled(rootContext); @@ -1071,6 +1073,38 @@ protected void ThrowExitApplicationException(TContext context, ExitReason exitRe }; } + internal void CheckIncompatibleRules(IEnumerable> skimmers, TContext context, ISet disabledSkimmers) + { + var availableRules = new Dictionary>(); + + foreach (Skimmer skimmer in skimmers) + { + if (disabledSkimmers.Contains(skimmer.Id)) + { + continue; + } + + availableRules[skimmer.Id] = skimmer; + } + + foreach (KeyValuePair> entry in availableRules) + { + if (entry.Value.IncompatibleRuleIds?.Any() != true) + { + continue; + } + + foreach (string incompatibleRuleId in entry.Value.IncompatibleRuleIds) + { + if (availableRules.ContainsKey(incompatibleRuleId)) + { + Errors.LogIncompatibleRules(context, entry.Key, incompatibleRuleId); + ThrowExitApplicationException(context, ExitReason.IncompatibleRulesDetected); + } + } + } + } + protected virtual ISet> InitializeSkimmers(ISet> skimmers, TContext context) { SortedSet> disabledSkimmers = new SortedSet>(SkimmerIdComparer.Instance); diff --git a/src/Sarif.Driver/Sdk/Skimmer.cs b/src/Sarif.Driver/Sdk/Skimmer.cs index 471f19261..075ef3bde 100644 --- a/src/Sarif.Driver/Sdk/Skimmer.cs +++ b/src/Sarif.Driver/Sdk/Skimmer.cs @@ -25,10 +25,10 @@ public Skimmer() protected virtual IEnumerable MessageResourceNames => throw new NotImplementedException(); - protected virtual ISet ConflictSkimmerSet => null; - public virtual bool EnabledByDefault => true; + public virtual ISet IncompatibleRuleIds { get; internal set; } + public override IDictionary MessageStrings { get diff --git a/src/Sarif/Errors.cs b/src/Sarif/Errors.cs index 2d2992d2f..794b46cf7 100644 --- a/src/Sarif/Errors.cs +++ b/src/Sarif/Errors.cs @@ -26,6 +26,7 @@ public static class Errors private const string ERR997_ExceptionLoadingAnalysisTarget = "ERR997.ExceptionLoadingAnalysisTarget"; private const string ERR997_ExceptionInstantiatingSkimmers = "ERR997.ExceptionInstantiatingSkimmers"; private const string ERR997_OutputFileAlreadyExists = "ERR997.OutputFileAlreadyExists"; + internal const string ERR997_IncompatibleRulesDetected = "ERR997.IncompatibleRulesDetected"; // Rule disabling tool errors: private const string ERR998_ExceptionInCanAnalyze = "ERR998.ExceptionInCanAnalyze"; @@ -509,6 +510,32 @@ public static RuntimeConditions LogUnhandledEngineException(IAnalysisContext con return RuntimeConditions.ExceptionInEngine; } + public static void LogIncompatibleRules(IAnalysisContext context, string ruleId, string incompatibleRuleId) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + // The current configuration enables rules that are not compatible + // ('{0}' has declared that it is not compatible with '{1}'). You + // can selectively disable one of the rules using an updated XML + // configuration (passed by the --config argument). + context.Logger.LogConfigurationNotification( + CreateNotification( + context.TargetUri, + ERR997_IncompatibleRulesDetected, + ruleId: null, + FailureLevel.Error, + exception: null, + persistExceptionStack: false, + messageFormat: SdkResources.ERR997_IncompatibleRulesDetected, + ruleId, + incompatibleRuleId)); + + context.RuntimeErrors |= RuntimeConditions.OneOrMoreRulesAreIncompatible; + } + public static Notification CreateNotification( Uri uri, string notificationId, diff --git a/src/Sarif/RuntimeConditions.cs b/src/Sarif/RuntimeConditions.cs index 7df7d9128..6d07cb06b 100644 --- a/src/Sarif/RuntimeConditions.cs +++ b/src/Sarif/RuntimeConditions.cs @@ -50,6 +50,7 @@ public enum RuntimeConditions : uint OutputFileAlreadyExists = 0x10000, ExceptionProcessingBaseline = 0x20000, ExceptionPostingLogFile = 0x40000, + OneOrMoreRulesAreIncompatible = 0x80000, // Non-fatal conditions UnassignedNonfatal = 0x00F00000, diff --git a/src/Sarif/SdkResources.Designer.cs b/src/Sarif/SdkResources.Designer.cs index a249235c9..52ec43970 100644 --- a/src/Sarif/SdkResources.Designer.cs +++ b/src/Sarif/SdkResources.Designer.cs @@ -177,6 +177,15 @@ public static string ERR997_ExceptionLoadingPlugIn { } } + /// + /// Looks up a localized string similar to The current configuration enables rules that are not compatible ('{0}' has declared that it is not compatible with '{1}'). You can selectively disable one of the rules using an updated XML configuration (passed by the --config argument).. + /// + public static string ERR997_IncompatibleRulesDetected { + get { + return ResourceManager.GetString("ERR997_IncompatibleRulesDetected", resourceCulture); + } + } + /// /// Looks up a localized string similar to '{0}' is not a property of the Invocation object.. /// diff --git a/src/Sarif/SdkResources.resx b/src/Sarif/SdkResources.resx index 076b7c3a7..737fe06be 100644 --- a/src/Sarif/SdkResources.resx +++ b/src/Sarif/SdkResources.resx @@ -284,4 +284,7 @@ You can also refer to properties in the result's property bag with 'properties.& One or more files were skipped for analysis due to exceeding size limits (currently configured as {0} KB). The 'max-file-size-in-kb' command-line argument can be used to increase this threshold. + + The current configuration enables rules that are not compatible ('{0}' has declared that it is not compatible with '{1}'). You can selectively disable one of the rules using an updated XML configuration (passed by the --config argument). + \ No newline at end of file diff --git a/src/Test.UnitTests.Sarif.Driver/ITestAnalyzeCommand.cs b/src/Test.UnitTests.Sarif.Driver/ITestAnalyzeCommand.cs index cf80931c6..36d0af903 100644 --- a/src/Test.UnitTests.Sarif.Driver/ITestAnalyzeCommand.cs +++ b/src/Test.UnitTests.Sarif.Driver/ITestAnalyzeCommand.cs @@ -16,5 +16,7 @@ internal interface ITestAnalyzeCommand RuntimeConditions RuntimeErrors { get; set; } int Run(AnalyzeOptionsBase options); + + void CheckIncompatibleRules(IEnumerable> skimmers, TestAnalysisContext context, ISet disabledSkimmers); } } diff --git a/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs b/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs index 4aa5cb227..50b253fbc 100644 --- a/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs +++ b/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Data.Common; using System.IO; using System.Linq; using System.Reflection; @@ -2069,6 +2070,159 @@ public IFileSystem FileSystem #endregion ResultsCachingTestsAndHelpers + [Fact] + public void CheckIncompatibleRules_ExitAnalysis() + { + TestRule[] skimmers = new[] + { + new TestRule { Id = "TEST1001" }, + new TestRule { Id = "TEST1002", IncompatibleRuleIds = new HashSet { "TEST1003" } }, + new TestRule { Id = "TEST1003" }, + }; + + var disabledSkimmers = new HashSet(); + + var consoleLogger = new ConsoleLogger(false, "TestTool") { CaptureOutput = true }; + var context = new TestAnalysisContext(); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, true, + ExitReason.IncompatibleRulesDetected, RuntimeConditions.OneOrMoreRulesAreIncompatible, + Errors.ERR997_IncompatibleRulesDetected, multipleThreadsCommand: false); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, true, + ExitReason.IncompatibleRulesDetected, RuntimeConditions.OneOrMoreRulesAreIncompatible, + Errors.ERR997_IncompatibleRulesDetected, multipleThreadsCommand: true); + } + + [Fact] + public void CheckIncompatibleRules_NoIncompatibleRules() + { + TestRule[] skimmers = new[] + { + new TestRule { Id = "TEST1001" }, + new TestRule { Id = "TEST1002" }, + new TestRule { Id = "TEST1003" }, + }; + + var disabledSkimmers = new HashSet(); + + var consoleLogger = new ConsoleLogger(false, "TestTool") { CaptureOutput = true }; + var context = new TestAnalysisContext(); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, + false, ExitReason.None, RuntimeConditions.None, null, multipleThreadsCommand: false); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, + false, ExitReason.None, RuntimeConditions.None, null, multipleThreadsCommand: false); + } + + [Fact] + public void CheckIncompatibleRules_IncompatibleRuleDoesNotExist() + { + TestRule[] skimmers = new[] + { + new TestRule { Id = "TEST1001", IncompatibleRuleIds = new HashSet { "NA9999" } }, + new TestRule { Id = "TEST1002" }, + new TestRule { Id = "TEST1003" }, + }; + + var disabledSkimmers = new HashSet(); + + var consoleLogger = new ConsoleLogger(false, "TestTool") { CaptureOutput = true }; + var context = new TestAnalysisContext(); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, + false, ExitReason.None, RuntimeConditions.None, null, false); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, + false, ExitReason.None, RuntimeConditions.None, null, true); + } + + [Fact] + public void CheckIncompatibleRules_RulesAlreadyDisabled() + { + TestRule[] skimmers = new[] + { + new TestRule { Id = "TEST1001" }, + new TestRule { Id = "TEST1002", IncompatibleRuleIds = new HashSet { "TEST1001" } }, + new TestRule { Id = "TEST1003" }, + }; + + var disabledSkimmers = new HashSet() { "TEST1002" }; + + var consoleLogger = new ConsoleLogger(false, "TestTool") { CaptureOutput = true }; + var context = new TestAnalysisContext(); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, + false, ExitReason.None, RuntimeConditions.None, null, false); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, + false, ExitReason.None, RuntimeConditions.None, null, true); + } + + [Fact] + public void CheckIncompatibleRules_MultipleIncompatibleRules() + { + TestRule[] skimmers = new[] + { + new TestRule { Id = "TEST1001" }, + new TestRule { Id = "TEST1002", IncompatibleRuleIds = new HashSet { "TEST1003" } }, + new TestRule { Id = "TEST1003", IncompatibleRuleIds = new HashSet { "TEST1001", "TEST1002" } }, + }; + + var disabledSkimmers = new HashSet(); + + var consoleLogger = new ConsoleLogger(false, "TestTool") { CaptureOutput = true }; + var context = new TestAnalysisContext(); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, + true, ExitReason.IncompatibleRulesDetected, RuntimeConditions.OneOrMoreRulesAreIncompatible, + Errors.ERR997_IncompatibleRulesDetected, false); + + this.RunCheckIncompatibleRulesTests(skimmers, disabledSkimmers, context, consoleLogger, + true, ExitReason.IncompatibleRulesDetected, RuntimeConditions.OneOrMoreRulesAreIncompatible, + Errors.ERR997_IncompatibleRulesDetected, true); + } + + private void RunCheckIncompatibleRulesTests(IEnumerable skimmers, HashSet disabledSkimmers, + TestAnalysisContext context, ConsoleLogger consoleLogger, bool expectExpcetion, ExitReason expectedExitReason, + RuntimeConditions expectedRuntimeConditions, string expectedErrorCode, bool multipleThreadsCommand) + { + ITestAnalyzeCommand command = this.CreateTestCommand(context, consoleLogger, multipleThreadsCommand); + + if (expectExpcetion) + { + ExitApplicationException exception = Assert.Throws>( + () => command.CheckIncompatibleRules(skimmers, context, disabledSkimmers)); + + exception.ExitReason.Should().Be(expectedExitReason); + } + + context.RuntimeErrors.Should().Be(expectedRuntimeConditions); + + if (expectedErrorCode == null) + { + consoleLogger.CapturedOutput.Should().BeNull(); + } + else + { + consoleLogger.CapturedOutput.Contains(expectedErrorCode); + } + } + + private ITestAnalyzeCommand CreateTestCommand(TestAnalysisContext context, ConsoleLogger consoleLogger, bool multiThreadsCommand = false) + { + ITestAnalyzeCommand command = multiThreadsCommand ? + new TestMultithreadedAnalyzeCommand() : + (ITestAnalyzeCommand)new TestAnalyzeCommand(); + + var logger = new AggregatingLogger(); + logger.Loggers.Add(consoleLogger); + context.Logger = logger; + + return command; + } + private void PostUriTestHelper(string postUri, int expectedReturnCode, RuntimeConditions runtimeConditions) { string location = GetThisTestAssemblyFilePath(); diff --git a/src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs b/src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs index 05ef9db7f..0a6418a2a 100644 --- a/src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs +++ b/src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs @@ -79,5 +79,10 @@ public override int Run(TestAnalyzeOptions options) this._rootContext?.Disposed.Should().BeTrue(); return result; } + + public new void CheckIncompatibleRules(IEnumerable> skimmers, TestAnalysisContext context, ISet disabledSkimmers) + { + base.CheckIncompatibleRules(skimmers, context, disabledSkimmers); + } } } diff --git a/src/Test.UnitTests.Sarif.Driver/TestMultithreadedAnalyzeCommand.cs b/src/Test.UnitTests.Sarif.Driver/TestMultithreadedAnalyzeCommand.cs index dbf105a82..8e9b2bf7a 100644 --- a/src/Test.UnitTests.Sarif.Driver/TestMultithreadedAnalyzeCommand.cs +++ b/src/Test.UnitTests.Sarif.Driver/TestMultithreadedAnalyzeCommand.cs @@ -85,5 +85,10 @@ protected override void ProcessBaseline(IAnalysisContext context, TestAnalyzeOpt base.ProcessBaseline(context, options, fileSystem); } + + public new void CheckIncompatibleRules(IEnumerable> skimmers, TestAnalysisContext context, ISet disabledSkimmers) + { + base.CheckIncompatibleRules(skimmers, context, disabledSkimmers); + } } }