Implement v1->v2 conversion for rules dictionary (#1232)

* Partial implementation * Get last test working. * Somebody missed checking in a generated file.
microsoft · Jan 23, 2019 · 1608bfa · 1608bfa
1 parent b691212
commit 1608bfa
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 72 deletions.
diff --git a/....UnitTests/TestData/SarifVersionOneToCurrentVisitor/ExpectedOutputs/OneRunWithRules.sarif b/....UnitTests/TestData/SarifVersionOneToCurrentVisitor/ExpectedOutputs/OneRunWithRules.sarif
@@ -1,6 +1,6 @@
 {
-  "$schema": "http://json.schemastore.org/sarif-2.0.0-csd.2.beta.2018-10-10",
-  "version": "2.0.0-csd.2.beta.2018-10-10",
+  "$schema": "http://json.schemastore.org/sarif-2.0.0-csd.2.beta.2018-11-28",
+  "version": "2.0.0-csd.2.beta.2018-11-28",
   "runs": [
     {
       "tool": {
@@ -10,30 +10,32 @@
       "results": [
         {
           "ruleId": "C2001",
+          "ruleIndex": 0,
           "message": {
             "arguments": [ "someVariable" ],
             "messageId": "default"
           }
         },
         {
           "ruleId": "C2001",
+          "ruleIndex": 0,
           "message": {
             "arguments": [ "anotherVariable" ],
             "messageId": "default"
           }
         },
         {
-          "ruleId": "C2002-1",
+          "ruleId": "C2002",
           "message": { "text": "Some testing occurred." }
         },
         {
-          "ruleId": "C2003",
+          "ruleIndex": 2,
           "message": { "text": "Some testing occurred." }
         }
       ],
       "resources": {
-        "rules": {
-          "C2001": {
+        "rules": [
+          {
             "id": "C2001",
             "shortDescription": {
               "text": "A variable was used without being initialized."
@@ -45,18 +47,19 @@
               "some_key": "FoxForceFive"
             }
           },
-          "C2002": {
+          {
             "id": "C2002",
             "fullDescription": {
               "text": "Catfish season continuous hen lamb include dose copy grant."
             },
             "configuration": {
               "enabled": true,
-              "defaultLevel": "error"
+              "defaultLevel": "error",
+              "defaultRank": 0.0
             },
             "helpUri": "http://www.domain.com/rules/c2002.html"
           },
-          "C2003": {
+          {
             "id": "C2003",
             "name": {
               "text": "Rule C2003"
@@ -68,14 +71,13 @@
               "text": "Rent internal rebellion competence biography photograph."
             },
             "configuration": {
-              "defaultLevel": "note"
+              "defaultLevel": "note",
+              "defaultRank": 0.0
             }
-          },
-          "C2002-1": {
-            "id": "C2002"
           }
-        }
-      }
+        ]
+      },
+      "columnKind": "utf16CodeUnits"
     }
   ]
 }
diff --git a/...tTests/TestData/SarifVersionOneToCurrentVisitor/ExpectedOutputs/TwoResultsWithFixes.sarif b/...tTests/TestData/SarifVersionOneToCurrentVisitor/ExpectedOutputs/TwoResultsWithFixes.sarif
@@ -1,6 +1,6 @@
 {
-  "$schema": "http://json.schemastore.org/sarif-2.0.0-csd.2.beta.2018-11-28",
-  "version": "2.0.0-csd.2.beta.2018-11-28",
+  "$schema": "http://json.schemastore.org/sarif-2.0.0",
+  "version": "2.0.0",
   "runs": [
     {
       "tool": {

diff --git a/src/Sarif.UnitTests/Visitors/SarifVersionOneToCurrentVisitorTests.cs b/src/Sarif.UnitTests/Visitors/SarifVersionOneToCurrentVisitorTests.cs
@@ -51,11 +51,11 @@ public void SarifTransformerTests_ToCurrent_OneRunWithLogicalLocations()
         [Fact]
         public void SarifTransformerTests_ToCurrent_OneRunWithFiles()
             => RunTest("OneRunWithFiles.sarif");
-#if TRANSFORM_CODE_AUTHORED
+
         [Fact]
         public void SarifTransformerTests_ToCurrent_OneRunWitRules()
             => RunTest("OneRunWithRules.sarif");
-#endif
+
         [Fact]
         public void SarifTransformerTests_ToCurrent_OneRunWithBasicInvocation()
             => RunTest("OneRunWithBasicInvocation.sarif");
@@ -71,15 +71,15 @@ public void SarifTransformerTests_ToCurrent_OneRunWithNotificationsButNoInvocati
         [Fact]
         public void SarifTransformerTests_ToCurrent_NotificationExceptionWithStack()
             => RunTest("NotificationExceptionWithStack.sarif");
-#if TRANSFORM_CODE_AUTHORED
+
         [Fact]
         public void SarifTransformerTests_ToCurrent_BasicResult()
             => RunTest("BasicResult.sarif");
 
         [Fact]
         public void SarifTransformerTests_ToCurrent_TwoResultsWithFixes()
             => RunTest("TwoResultsWithFixes.sarif");
-#endif
+
         [Fact]
         public void SarifTransformerTests_ToCurrent_UriBaseId()
             => RunTest("UriBaseId.sarif");

diff --git a/src/Sarif/Autogenerated/Notification.cs b/src/Sarif/Autogenerated/Notification.cs
@@ -41,13 +41,13 @@ public SarifNodeKind SarifNodeKind
         public string Id { get; set; }
 
         /// <summary>
-        /// The stable, unique identifier of the rule (if any) to which this notification is relevant. This member can be used to retrieve rule metadata from the rules dictionary, if it exists.
+        /// The stable, unique identifier of the rule, if any, to which this notification is relevant.
         /// </summary>
         [DataMember(Name = "ruleId", IsRequired = false, EmitDefaultValue = false)]
         public string RuleId { get; set; }
 
         /// <summary>
-        /// The index within the run resources array of the rule object associated with this notification.
+        /// The index within the run resources array of the rule object, if any, associated with this notification.
         /// </summary>
         [DataMember(Name = "ruleIndex", IsRequired = false, EmitDefaultValue = false)]
         [DefaultValue(-1)]

diff --git a/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs b/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs
@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Globalization;
 using System.Linq;
 using Microsoft.CodeAnalysis.Sarif.VersionOne;
@@ -22,6 +21,7 @@ public class SarifVersionOneToCurrentVisitor : SarifRewritingVisitorVersionOne
         private RunVersionOne _currentV1Run;
         private int _threadFlowLocationNestingLevel;
         private IDictionary<string, int> _v1FileKeytoV2IndexMap;
+        private IDictionary<string, int> _v1RuleKeyToV2IndexMap;
 
         private IDictionary<string, string> _v1KeyToFullyQualifiedNameMap;
         private IDictionary<LogicalLocation, int> _v2LogicalLocationToIndexMap;
@@ -732,51 +732,11 @@ internal Result CreateResult(ResultVersionOne v1Result)
                     result.AnalysisTarget = CreateFileLocation(v1Result.Locations[0].AnalysisTarget);
                 }
 
-                if (v1Result.RuleKey == null)
-                {
-                    result.RuleId = v1Result.RuleId;
-                }
-                else
-                {
-                    if (v1Result.RuleId == null)
-                    {
-                        result.RuleId = v1Result.RuleKey;
-                    }
-                    else
-                    {
-                        if (v1Result.RuleId == v1Result.RuleKey)
-                        {
-                            result.RuleId = v1Result.RuleId;
-                        }
-                        else
-                        {
-                            result.RuleId = v1Result.RuleKey;
-
-                            if (_currentRun.Resources == null)
-                            {
-                                _currentRun.Resources = new Resources();
-                            }
-
-#if TRANSFORM_CODE_AUTHORED
-                            if (_currentRun.Resources.Rules == null)
-                            {
-                                _currentRun.Resources.Rules = new List<string, Rule>();
-                            }
-
-                            IDictionary<string, Rule> rules = _currentRun.Resources.Rules;
+                result.RuleId = v1Result.RuleId;
 
-                            if (!rules.ContainsKey(v1Result.RuleKey))
-                            {
-                                Rule rule = new Rule() { Id = v1Result.RuleId };
-                                rules.Add(v1Result.RuleKey, rule);
-                            }
+                string ruleKey = v1Result.RuleKey ?? v1Result.RuleId;
+                result.RuleIndex = GetRuleIndexForRuleKey(ruleKey, _v1RuleKeyToV2IndexMap);
 
-                            Debug.Assert(rules[v1Result.RuleKey].Id == v1Result.RuleId);
-#endif
-                        }
-                    }
-                }
-
                 if (v1Result.FormattedRuleMessage != null)
                 {
                     if (result.Message == null)
@@ -875,6 +835,7 @@ internal Run CreateRun(RunVersionOne v1Run)
                     _currentV1Run = v1Run;
 
                     _v1FileKeytoV2IndexMap = CreateFileKeyToIndexMapping(v1Run.Files);
+                    _v1RuleKeyToV2IndexMap = CreateRuleKeyToIndexMapping(v1Run.Rules);
 
                     RunAutomationDetails id = null;
                     RunAutomationDetails[] aggregateIds = null;
@@ -905,20 +866,18 @@ internal Run CreateRun(RunVersionOne v1Run)
 
                     _currentRun = run;
 
-#if TRANSFORM_CODE_AUTHORED
                     if (v1Run.Rules != null)
                     {
                         run.Resources = new Resources
                         {
-                            Rules = new Dictionary<string, Rule>()
+                            Rules = new List<Rule>()
                         };
 
                         foreach (var pair in v1Run.Rules)
                         {
-                            run.Resources.Rules.Add(pair.Key, CreateRule(pair.Value));
+                            run.Resources.Rules.Add(CreateRule(pair.Value));
                         }
                     }
-#endif
 
                     if (v1Run.Files != null)
                     {
@@ -1019,6 +978,32 @@ private static IDictionary<string, int> CreateFileKeyToIndexMapping(IDictionary<
             return v1FileKeyToV2IndexMap;
         }
 
+        private static IDictionary<string, int> CreateRuleKeyToIndexMapping(IDictionary<string, RuleVersionOne> v1Rules)
+        {
+            var v1RuleKeyToV2IndexMap = new Dictionary<string, int>();
+
+            if (v1Rules != null)
+            {
+                int index = 0;
+                foreach (KeyValuePair<string, RuleVersionOne> entry in v1Rules)
+                {
+                    v1RuleKeyToV2IndexMap[entry.Key] = index++;
+                }
+            }
+
+            return v1RuleKeyToV2IndexMap;
+        }
+
+        private int GetRuleIndexForRuleKey(string ruleKey, IDictionary<string, int> v1RuleKeyToV2IndexMap)
+        {
+            if (ruleKey == null || !v1RuleKeyToV2IndexMap.TryGetValue(ruleKey, out int index))
+            {
+                index = -1;
+            }
+
+            return index;
+        }
+
         private void PopulateLogicalLocation(
             Run v2Run, 
             IDictionary<string, LogicalLocationVersionOne> v1LogicalLocations,

diff --git a/src/Sarif/Visitors/UpdateIndicesVisitor.cs b/src/Sarif/Visitors/UpdateIndicesVisitor.cs
@@ -27,7 +27,7 @@ public override Result VisitResult(Result node)
         {
             if (_ruleKeyToIndexMap != null)
             {
-                if (_ruleKeyToIndexMap.TryGetValue(node.RuleId, out int ruleIndex))
+                if (node.RuleId != null &&_ruleKeyToIndexMap.TryGetValue(node.RuleId, out int ruleIndex))
                 {
                     node.RuleIndex = ruleIndex;