Skip to content

Commit

Permalink
Fix #1668: SarifLogger doesn't handle sub-rules.
Browse files Browse the repository at this point in the history
  • Loading branch information
Larry Golding committed Sep 4, 2019
1 parent 6dca34a commit 64ab7e2
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 3 deletions.
1 change: 1 addition & 0 deletions src/ReleaseHistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -461,3 +461,4 @@
## **v2.1.16** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.1.16) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.1.16) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.1.16) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.1.16)
* BUGFIX, BREAKING: In the Multitool `page` command, the default for `--force` was `true` and it could not be changed. https://github.com/microsoft/sarif-sdk/issues/1630
* BUGFIX: The Multitool `match-results-forward` command failed if results included logical locations. https://github.com/microsoft/sarif-sdk/issues/1656
* BUGFIX: `SarifLogger(ReportingDescriptor rule, Result result)` failed if it tried to log a result whose `ruleId` was a sub-rule; for example, `rule.Id == "TEST0001"` but `result.ruleId == "TEST0001/1"`. https://github.com/microsoft/sarif-sdk/issues/1668
17 changes: 17 additions & 0 deletions src/Sarif/ExtensionMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,23 @@ public static string InstanceIdLogicalComponent(this RunAutomationDetails runAut
return instanceId.Substring(0, instanceId.LastIndexOf('/'));
}

public static bool IsEqualToOrHierarchicalDescendantOf(this string child, string parent)
{
if (child == parent) { return true; }

string[] childComponents = child.Split(new[] { SarifConstants.HierarchicalComponentSeparator });
string[] parentComponents = parent.Split(new[] { SarifConstants.HierarchicalComponentSeparator });

if (childComponents.Length < parentComponents.Length) { return false; }

for (int i = 0; i < parentComponents.Length; ++i)
{
if (childComponents[i] != parentComponents[i]) { return false; }
}

return true;
}

public static Message ToMessage(this string text)
{
return new Message { Text = text };
Expand Down
1 change: 1 addition & 0 deletions src/Sarif/SarifConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ namespace Microsoft.CodeAnalysis.Sarif
public static class SarifConstants
{
public const string RedactedMarker = "[REDACTED]";
public const char HierarchicalComponentSeparator = '/';
}
}
6 changes: 3 additions & 3 deletions src/Sarif/Writers/SarifLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,12 @@ public void Log(ReportingDescriptor rule, Result result)
throw new ArgumentNullException(nameof(result));
}

if (rule.Id != result.RuleId)
if (!result.RuleId.IsEqualToOrHierarchicalDescendantOf(rule.Id))
{
//The rule id '{0}' specified by the result does not match the actual id of the rule '{1}'
string message = string.Format(CultureInfo.InvariantCulture, SdkResources.ResultRuleIdDoesNotMatchRule,
result.RuleId.ToString(),
rule.Id.ToString());
result.RuleId,
rule.Id);

throw new ArgumentException(message);
}
Expand Down
18 changes: 18 additions & 0 deletions src/Test.UnitTests.Sarif/Writers/SarifLoggerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,24 @@ public void SarifLogger_TreatsInvocationPropertiesCaseInsensitively()
invocation.ProcessId.Should().NotBe(0);
}

[Fact]
public void SarifLogger_AcceptsSubrulesInResultRuleId()
{
var sb = new StringBuilder();

using (var textWriter = new StringWriter(sb))
{
using (var sarifLogger = new SarifLogger(textWriter))
{
var rule = new ReportingDescriptor { Id = "RuleId" };
var result = new Result { RuleId = "RuleId/1" };

Action action = () => sarifLogger.Log(rule, result);
action.Should().NotThrow();
}
}
}

private void LogSimpleResult(SarifLogger sarifLogger)
{
ReportingDescriptor rule = new ReportingDescriptor { Id = "RuleId" };
Expand Down

0 comments on commit 64ab7e2

Please sign in to comment.