Fix #1668: SarifLogger doesn't handle sub-rules. (#1669)

src/ReleaseHistory.md

@@ -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

src/Sarif/ExtensionMethods.cs

@@ -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(SarifConstants.HierarchicalComponentSeparator);
+            string[] parentComponents = parent.Split(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 };

src/Sarif/OptionallyEmittedData.cs

@@ -49,7 +49,7 @@ public enum OptionallyEmittedData : int
         ContextRegionSnippets = 0x20,
 
         // Some SARIF data, such as timestamps that denote the start and end of analysis,
-        // are not determinitic run-over-run. In order to support caching mechanisms of 
+        // are not deterministic run-over-run. In order to support caching mechanisms of 
         // modern build systems, it is sometimes helpful to be able to reliably remove this
         // non-deterministic data.
         NondeterministicProperties = 0x40,

src/Sarif/SarifConstants.cs

@@ -5,6 +5,17 @@ namespace Microsoft.CodeAnalysis.Sarif
 {
     public static class SarifConstants
     {
+        // <summary>
+        // When used with Guid.ToString(string format), this format produces the
+        // GUID format required by SARIF: 32 digits separated by hyphens.
+        // </summary>
+        public const string GuidFormat = "D";
+
         public const string RedactedMarker = "[REDACTED]";
+
+        // <summary>
+        // The character that separates the components of a SARIF hierarchical string.
+        // </summary>
+        public const char HierarchicalComponentSeparator = '/';
     }
 }

src/Sarif/Writers/SarifLogger.cs

@@ -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);
             }

src/Test.UnitTests.Sarif/Writers/SarifLoggerTests.cs

@@ -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" };