Remove 'open' from list of valid rule configuration default values. (#…

…1158)
microsoft · Dec 6, 2018 · 7c40bc5 · 7c40bc5
1 parent c75aa9b
commit 7c40bc5
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 23 deletions.
diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md
@@ -204,25 +204,26 @@
 ## **v2.0.0-csd.2.beta.2018.10.10** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.0.0-csd.2.beta.2018.10.10) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.0.0-csd.2.beta.2018.10.10) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.0.0-csd.2.beta.2018.10.10)) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.0.0-csd.2.beta.2018.10.10))
 * FEATURE:Add --sarif-version command to driver (to transform SARIF output to v1 format)
 * BUGFIX: Drop erroneous persistence of redaction tokens as files objects.
-* API NON-BREAKING: Add result.occurrenceCount (denotes # of occurrences of an identical results within an analysisRun)
-* API NON-BREAKING: Add run.externalFiles object to schema. Sync generally to OASIS TC schema.
-* API BREAKING: originalUriBaseIds is now a dictionary of file locations, not strings.
-* API BREAKING: Suffix invocation.startTime, invocation.endTime, file.lastModifiedTime and notification.time with Utc (startTimeUtc, endTimeUtc, etc.).
-* API BREAKING: threadflowLocation.timestamp renamed to 'executionTimeUtc'.
-* API BREAKING: versionControlDetails.timestamp renamed to 'asOfTimeUtc'.
-* API BREAKING: versionControlDetails.uri renamed to 'repositoryUri'.
-* API BREAKING: versionControlDetails.tag renamed to 'revisionTag'
-* API BREAKING: exception.message type converted from string to message object.
-* API BREAKING: file.hashes is now a string/string dictionary, not an array of 'hash' objects (the type for which is deleted)
-* API BREAKING: run.instanceGuid, run.correlationGuid, run.logicalId, run.description combined into new run.id 'runAutomationDetails' object instance.
-* API BREAKING: run.automationLogicalId subsumed by run.aggregateIds, an array of 'runAutomationDetails' objects.
-* API BREAKING: Remove threadFlowLocation.step
-* API BREAKING: invocation.workingDirectory is now a FileLocation object (and not a URI expressed as a string)
+* API NON-BREAKING: Add 'result.occurrenceCount' (denotes # of occurrences of an identical results within an analysisRun)
+* API NON-BREAKING: Add 'run.externalFiles' object to schema. Sync generally to OASIS TC schema.
+* API BREAKING: 'originalUriBaseIds' is now a dictionary of file locations, not strings.
+* API BREAKING: Suffix 'invocation.startTime', 'invocation.endTime', 'file.lastModifiedTime' and 'notification.time' with Utc ('startTimeUtc', 'endTimeUtc', etc.).
+* API BREAKING: 'threadflowLocation.timestamp' renamed to 'executionTimeUtc'.
+* API BREAKING: 'versionControlDetails.timestamp' renamed to 'asOfTimeUtc'.
+* API BREAKING: 'versionControlDetails.uri' renamed to 'repositoryUri'.
+* API BREAKING: 'versionControlDetails.tag' renamed to 'revisionTag'
+* API BREAKING: 'exception.message' type converted from string to message object.
+* API BREAKING: 'file.hashes' is now a string/string dictionary, not an array of 'hash' objects (the type for which is deleted)
+* API BREAKING: 'run.instanceGuid', 'run.correlationGuid', 'run.logicalId', 'run.description' combined into new 'runAutomationDetails' object instance defined at 'run.id'.
+* API BREAKING: 'run.automationLogicalId' subsumed by 'run.aggregateIds', an array of 'runAutomationDetails' objects.
+* API BREAKING: Remove 'threadFlowLocation.step'
+* API BREAKING: 'invocation.workingDirectory' is now a FileLocation object (and not a URI expressed as a string)
 
 ## **v2.0.0-csd.2.beta.2018.11.28** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.0.0-csd.2.beta.2018.11.28) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.0.0-csd.2.beta.2018.11.28) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.0.0-csd.2.beta.2018.11.28)) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.0.0-csd.2.beta.2018.11.28))
 * BUGFIX: Result matching improvements in properties persistence
 * FEATURE: Fortify FPR converter improvements
-* API NON-BREAKING: Add run.newlineSequences to schema. https://github.com/oasis-tcs/sarif-spec/issues/169
-* API BREAKING: remove run.architecture https://github.com/oasis-tcs/sarif-spec/issues/262
-* API BREAKING: result.message is now a required property https://github.com/oasis-tcs/sarif-spec/issues/283
-* API BREAKING: rename tool.fileVersion to tool.dottedQuadFileVersion https://github.com/oasis-tcs/sarif-spec/issues/274
+* API NON-BREAKING: Add 'run.newlineSequences' to schema. https://github.com/oasis-tcs/sarif-spec/issues/169
+* API BREAKING: remove 'run.architecture' https://github.com/oasis-tcs/sarif-spec/issues/262
+* API BREAKING: 'result.message' is now a required property https://github.com/oasis-tcs/sarif-spec/issues/283
+* API BREAKING: rename 'tool.fileVersion' to 'tool.dottedQuadFileVersion' https://github.com/oasis-tcs/sarif-spec/issues/274
+* API BREAKING: remove 'open' from valid rule default configuration levels. https://github.com/oasis-tcs/sarif-spec/issues/288. The transformer remaps this value to 'note'.
diff --git a/src/Sarif.FunctionalTests/v2/ObsoleteFormats/ComprehensivePrereleaseTwoZeroZero.sarif b/src/Sarif.FunctionalTests/v2/ObsoleteFormats/ComprehensivePrereleaseTwoZeroZero.sarif
@@ -429,6 +429,7 @@
         "rules": {
           "C2001": {
             "id": "C2001",
+            "configuration": {"defaultLevel": "open"},
             "shortDescription": {
               "text": "A variable was used without being initialized."
             },

diff --git a/src/Sarif/Autogenerated/RuleConfigurationDefaultLevel.cs b/src/Sarif/Autogenerated/RuleConfigurationDefaultLevel.cs
@@ -14,7 +14,6 @@ public enum RuleConfigurationDefaultLevel
         None,
         Note,
         Warning,
-        Error,
-        Open
+        Error
     }
 }
diff --git a/src/Sarif/CodeGenHints.json b/src/Sarif/CodeGenHints.json
@@ -609,8 +609,7 @@
         "memberNames": [
           "Note",
           "Warning",
-          "Error",
-          "Open"
+          "Error"
         ]
       }
     },

diff --git a/src/Sarif/Schemata/sarif-schema.json b/src/Sarif/Schemata/sarif-schema.json
@@ -1440,7 +1440,7 @@
         "defaultLevel": {
           "description": "Specifies the default severity level of the result.",
           "default": "warning",
-          "enum": [ "note", "warning", "error", "open" ]
+          "enum": [ "note", "warning", "error" ]
         },
 
         "parameters": {

diff --git a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs
@@ -116,12 +116,45 @@ private static bool ApplyChangesFromTC25ThroughTC28(JObject sarifLog)
                     {
                         modifiedLog |= RenameProperty((JObject)conversion["tool"], previousName: "fileVersion", newName: "dottedQuadFileVersion");
                     }
+
+                    // Remove 'open' from rule configuration default level enumeration
+                    // https://github.com/oasis-tcs/sarif-spec/issues/288
+                    JObject resources = (JObject)run["resources"];
+                    modifiedLog |= RemapRuleDefaultLevelFromOpenToNote(resources);
+
                 }
             }
 
             return modifiedLog;
         }
 
+        private static bool RemapRuleDefaultLevelFromOpenToNote(JObject resources)
+        {
+            bool modifiedResources = false;
+
+            if (resources == null) { return modifiedResources; }
+
+            JObject rules = (JObject)resources["rules"];
+            if (rules == null ) { return modifiedResources; }
+
+            foreach (JProperty rule in rules.Values<JProperty>())
+            {
+                JObject configuration = (JObject)rule.Value["configuration"];
+                if (configuration == null) { continue; }
+
+                if ("open".Equals((string)configuration["defaultLevel"]))
+                {
+                    // We remap 'open' to 'note'. 'open' is an indicator that analysis is unresolved, i.e., 
+                    // the question of whether a weakness exists is not yet determined. 'note' is the most
+                    // reasonable level to associate with this class of report, if it is emitted. In 
+                    // practice, we don't expect that a current producer exists who is in this condition.
+                    configuration["defaultLevel"] = "note";
+                }
+            }
+
+            return modifiedResources;
+        }
+
         private static bool ApplyCoreTransformations(JObject sarifLog)
         {
             bool modifiedLog = UpdateSarifLogVersion(sarifLog);