Skip to content

Commit

Permalink
Support Incompatible rules (#2597)
Browse files Browse the repository at this point in the history
* Adding support for Incompatible rules

* Address review feedbacks

* Update ERR997_IncompatibleRulesDetected string
  • Loading branch information
yongyan-gh authored Dec 28, 2022
1 parent 0e842c2 commit 80cb48e
Show file tree
Hide file tree
Showing 12 changed files with 280 additions and 3 deletions.
35 changes: 35 additions & 0 deletions src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -568,6 +569,8 @@ protected virtual void AnalyzeTargets(
}
}

this.CheckIncompatibleRules(skimmers, rootContext, disabledSkimmers);

if (disabledSkimmers.Count == skimmers.Count())
{
Errors.LogAllRulesExplicitlyDisabled(rootContext);
Expand Down Expand Up @@ -792,6 +795,38 @@ protected void ThrowExitApplicationException(TContext context, ExitReason exitRe
};
}

internal void CheckIncompatibleRules(IEnumerable<Skimmer<TContext>> skimmers, TContext context, ISet<string> disabledSkimmers)
{
var availableRules = new Dictionary<string, Skimmer<TContext>>();

foreach (Skimmer<TContext> skimmer in skimmers)
{
if (disabledSkimmers.Contains(skimmer.Id))
{
continue;
}

availableRules[skimmer.Id] = skimmer;
}

foreach (KeyValuePair<string, Skimmer<TContext>> 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<Skimmer<TContext>> InitializeSkimmers(ISet<Skimmer<TContext>> skimmers, TContext context)
{
SortedSet<Skimmer<TContext>> disabledSkimmers = new SortedSet<Skimmer<TContext>>(SkimmerIdComparer<TContext>.Instance);
Expand Down
4 changes: 3 additions & 1 deletion src/Sarif.Driver/Sdk/ExitReason.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -14,6 +15,7 @@ public enum ExitReason
NoValidAnalysisTargets,
InvalidCommandLineOption,
ExceptionProcessingBaseline,
ExceptionPostingLogFile
ExceptionPostingLogFile,
IncompatibleRulesDetected
}
}
34 changes: 34 additions & 0 deletions src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,8 @@ protected virtual void AnalyzeTargets(TOptions options,
}
}

this.CheckIncompatibleRules(skimmers, rootContext, disabledSkimmers);

if (disabledSkimmers.Count == skimmers.Count())
{
Errors.LogAllRulesExplicitlyDisabled(rootContext);
Expand Down Expand Up @@ -1071,6 +1073,38 @@ protected void ThrowExitApplicationException(TContext context, ExitReason exitRe
};
}

internal void CheckIncompatibleRules(IEnumerable<Skimmer<TContext>> skimmers, TContext context, ISet<string> disabledSkimmers)
{
var availableRules = new Dictionary<string, Skimmer<TContext>>();

foreach (Skimmer<TContext> skimmer in skimmers)
{
if (disabledSkimmers.Contains(skimmer.Id))
{
continue;
}

availableRules[skimmer.Id] = skimmer;
}

foreach (KeyValuePair<string, Skimmer<TContext>> 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<Skimmer<TContext>> InitializeSkimmers(ISet<Skimmer<TContext>> skimmers, TContext context)
{
SortedSet<Skimmer<TContext>> disabledSkimmers = new SortedSet<Skimmer<TContext>>(SkimmerIdComparer<TContext>.Instance);
Expand Down
4 changes: 2 additions & 2 deletions src/Sarif.Driver/Sdk/Skimmer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ public Skimmer()

protected virtual IEnumerable<string> MessageResourceNames => throw new NotImplementedException();

protected virtual ISet<string> ConflictSkimmerSet => null;

public virtual bool EnabledByDefault => true;

public virtual ISet<string> IncompatibleRuleIds { get; internal set; }

public override IDictionary<string, MultiformatMessageString> MessageStrings
{
get
Expand Down
27 changes: 27 additions & 0 deletions src/Sarif/Errors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/Sarif/RuntimeConditions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public enum RuntimeConditions : uint
OutputFileAlreadyExists = 0x10000,
ExceptionProcessingBaseline = 0x20000,
ExceptionPostingLogFile = 0x40000,
OneOrMoreRulesAreIncompatible = 0x80000,

// Non-fatal conditions
UnassignedNonfatal = 0x00F00000,
Expand Down
9 changes: 9 additions & 0 deletions src/Sarif/SdkResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Sarif/SdkResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -284,4 +284,7 @@ You can also refer to properties in the result's property bag with 'properties.&
<data name="WRN997_OneOrMoreFilesSkippedDueToSize" xml:space="preserve">
<value>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.</value>
</data>
<data name="ERR997_IncompatibleRulesDetected" xml:space="preserve">
<value>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).</value>
</data>
</root>
2 changes: 2 additions & 0 deletions src/Test.UnitTests.Sarif.Driver/ITestAnalyzeCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,7 @@ internal interface ITestAnalyzeCommand
RuntimeConditions RuntimeErrors { get; set; }

int Run(AnalyzeOptionsBase options);

void CheckIncompatibleRules(IEnumerable<Skimmer<TestAnalysisContext>> skimmers, TestAnalysisContext context, ISet<string> disabledSkimmers);
}
}
154 changes: 154 additions & 0 deletions src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Data.Common;
using System.IO;
using System.Linq;
using System.Reflection;
Expand Down Expand Up @@ -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<string> { "TEST1003" } },
new TestRule { Id = "TEST1003" },
};

var disabledSkimmers = new HashSet<string>();

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<string>();

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<string> { "NA9999" } },
new TestRule { Id = "TEST1002" },
new TestRule { Id = "TEST1003" },
};

var disabledSkimmers = new HashSet<string>();

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<string> { "TEST1001" } },
new TestRule { Id = "TEST1003" },
};

var disabledSkimmers = new HashSet<string>() { "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<string> { "TEST1003" } },
new TestRule { Id = "TEST1003", IncompatibleRuleIds = new HashSet<string> { "TEST1001", "TEST1002" } },
};

var disabledSkimmers = new HashSet<string>();

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<TestRule> skimmers, HashSet<string> 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<ExitReason> exception = Assert.Throws<ExitApplicationException<ExitReason>>(
() => 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();
Expand Down
5 changes: 5 additions & 0 deletions src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,10 @@ public override int Run(TestAnalyzeOptions options)
this._rootContext?.Disposed.Should().BeTrue();
return result;
}

public new void CheckIncompatibleRules(IEnumerable<Skimmer<TestAnalysisContext>> skimmers, TestAnalysisContext context, ISet<string> disabledSkimmers)
{
base.CheckIncompatibleRules(skimmers, context, disabledSkimmers);
}
}
}
Loading

0 comments on commit 80cb48e

Please sign in to comment.