diff --git a/src/Sarif.Multitool/Rules/RuleResources.Designer.cs b/src/Sarif.Multitool/Rules/RuleResources.Designer.cs index bc3c4dff8..5862afec7 100644 --- a/src/Sarif.Multitool/Rules/RuleResources.Designer.cs +++ b/src/Sarif.Multitool/Rules/RuleResources.Designer.cs @@ -351,7 +351,7 @@ internal static string SARIF1011_ReferenceFinalSchema_FullDescription_Text { } /// - /// Looks up a localized string similar to Placeholder_SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_MessageIdMustExist_Text. + /// Looks up a localized string similar to {0}: Placeholder '{1}' '{2}'. /// internal static string SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_MessageIdMustExist_Text { get { @@ -360,7 +360,7 @@ internal static string SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_ } /// - /// Looks up a localized string similar to Placeholder_SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_SupplyEnoughMessageArguments_Text. + /// Looks up a localized string similar to {0}: Placeholder '{1}' '{2}' '{3}' '{4}' '{5}'. /// internal static string SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_SupplyEnoughMessageArguments_Text { get { @@ -370,7 +370,7 @@ internal static string SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_ } /// - /// Looks up a localized string similar to Placeholder_SARIF1012_MessageArgumentsMustBeConsistentWithRule_FullDescription_Text. + /// Looks up a localized string similar to Placeholder. /// internal static string SARIF1012_MessageArgumentsMustBeConsistentWithRule_FullDescription_Text { get { @@ -644,33 +644,6 @@ internal static string SARIF2011_ProvideContextRegion_Note_Default_Text { } } - /// - /// Looks up a localized string similar to Placeholder_SARIF2012_ProvideHelpUris_FullDescription_Text. - /// - internal static string SARIF2012_ProvideHelpUris_FullDescription_Text { - get { - return ResourceManager.GetString("SARIF2012_ProvideHelpUris_FullDescription_Text", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Placeholder_SARIF2012_ProvideHelpUris_Note_Default_Text. - /// - internal static string SARIF2012_ProvideHelpUris_Note_Default_Text { - get { - return ResourceManager.GetString("SARIF2012_ProvideHelpUris_Note_Default_Text", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Placeholder_SARIF2013_ProvideEmbeddedFileContent_FullDescription_Text. - /// - internal static string SARIF2013_ProvideEmbeddedFileContent_FullDescription_Text { - get { - return ResourceManager.GetString("SARIF2013_ProvideEmbeddedFileContent_FullDescription_Text", resourceCulture); - } - } - /// /// Looks up a localized string similar to Placeholder_SARIF2013_ProvideEmbeddedFileContent_Note_Default_Text. /// diff --git a/src/Sarif.Multitool/Rules/RuleResources.resx b/src/Sarif.Multitool/Rules/RuleResources.resx index a96c1c8ef..7c60be2f8 100644 --- a/src/Sarif.Multitool/Rules/RuleResources.resx +++ b/src/Sarif.Multitool/Rules/RuleResources.resx @@ -283,15 +283,6 @@ Many tool use similar names for 'uriBaseId' symbols. We suggest 'REPOROOT' for t {0}: Placeholder_SARIF2004_OptimizeFileSize_Warning_EliminateLocationOnlyArtifacts_Text - - Placeholder_SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_MessageIdMustExist_Text - - - Placeholder_SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_SupplyEnoughMessageArguments_Text - - - Placeholder_SARIF1012_MessageArgumentsMustBeConsistentWithRule_FullDescription_Text - Placeholder_SARIF2002_ProvideMessageArguments_FullDescription_Text @@ -331,14 +322,14 @@ Many tool use similar names for 'uriBaseId' symbols. We suggest 'REPOROOT' for t Placeholder_SARIF2011_ProvideContextRegion_Note_Default_Text - - Placeholder_SARIF2012_ProvideHelpUris_FullDescription_Text + + {0}: Placeholder '{1}' '{2}' - - Placeholder_SARIF2012_ProvideHelpUris_Note_Default_Text + + {0}: Placeholder '{1}' '{2}' '{3}' '{4}' '{5}' - - Placeholder_SARIF2013_ProvideEmbeddedFileContent_FullDescription_Text + + Placeholder Placeholder_SARIF2013_ProvideEmbeddedFileContent_Note_Default_Text diff --git a/src/Sarif.Multitool/Rules/SARIF1012.MessageArgumentsMustBeConsistentWithRule.cs b/src/Sarif.Multitool/Rules/SARIF1012.MessageArgumentsMustBeConsistentWithRule.cs new file mode 100644 index 000000000..8a5fca540 --- /dev/null +++ b/src/Sarif.Multitool/Rules/SARIF1012.MessageArgumentsMustBeConsistentWithRule.cs @@ -0,0 +1,89 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; + +namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules +{ + public class MessageArgumentsMustBeConsistentWithRule : SarifValidationSkimmerBase + { + /// + /// SARIF1012 + /// + public override string Id => RuleId.MessageArgumentsMustBeConsistentWithRule; + + /// + /// Placeholder + /// + public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_FullDescription_Text }; + + protected override IEnumerable MessageResourceNames => new string[] { + nameof(RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_MessageIdMustExist_Text), + nameof(RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_SupplyEnoughMessageArguments_Text) + }; + + public override FailureLevel DefaultLevel => FailureLevel.Error; + + private static readonly Regex s_replacementSequenceRegex = new Regex(@"\{(\d+)\}", RegexOptions.Compiled | RegexOptions.CultureInvariant); + private IList currentRules; + private Run run; + + protected override void Analyze(Run run, string runPointer) + { + this.run = run; + this.currentRules = run.Tool.Driver?.Rules; + } + + protected override void Analyze(Result result, string resultPointer) + { + // If message.id is present, check that a message with that id exists in the rule. + if (!string.IsNullOrEmpty(result.Message.Id)) + { + ReportingDescriptor rule = result.GetRule(this.run); + + if (this.currentRules == null + || rule.MessageStrings?.ContainsKey(result.Message.Id) == false) + { + // {0}: Placeholder {1} {2} + LogResult( + resultPointer, + nameof(RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_MessageIdMustExist_Text), + result.Message.Id, + result.ResolvedRuleId(run) ?? "null"); + return; + } + + // A message with the specified key is present in the rule. Check if the result supplied enough arguments. + string messageText = rule.MessageStrings[result.Message.Id].Text; + int placeholderMaxPosition = PlaceholderMaxPosition(messageText); + if (placeholderMaxPosition > (result.Message.Arguments?.Count ?? 0)) + { + // {0}: Placeholder {1} {2} {3} {4} {5} + LogResult( + resultPointer, + nameof(RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_SupplyEnoughMessageArguments_Text), + result.Message.Arguments.Count.ToString(), + result.Message.Id, + result.ResolvedRuleId(run) ?? "null", + placeholderMaxPosition.ToString(), + messageText); + } + } + } + + private int PlaceholderMaxPosition(string text) + { + int max = -1; + foreach (Match match in s_replacementSequenceRegex.Matches(text)) + { + int index = int.Parse(match.Groups["index"].Value); + max = Math.Max(max, index); + } + + return max++; + } + } +} diff --git a/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs b/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs index 7be9f2a50..bb998ac5a 100644 --- a/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs +++ b/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs @@ -123,6 +123,14 @@ public void SARIF1011_ReferenceFinalSchema_Valid() public void SARIF1011_ReferenceFinalSchema_Invalid() => RunTest(MakeInvalidTestFileName(RuleId.ReferenceFinalSchema, nameof(RuleId.ReferenceFinalSchema))); + [Fact] + public void SARIF1012_MessageArgumentsMustBeConsistentWithRule_Valid() + => RunTest(MakeValidTestFileName(RuleId.MessageArgumentsMustBeConsistentWithRule, nameof(RuleId.MessageArgumentsMustBeConsistentWithRule))); + + [Fact] + public void SARIF1012_MessageArgumentsMustBeConsistentWithRule_Invalid() + => RunTest(MakeInvalidTestFileName(RuleId.MessageArgumentsMustBeConsistentWithRule, nameof(RuleId.MessageArgumentsMustBeConsistentWithRule))); + [Fact] public void SARIF2001_AuthorHighQualityMessages_Valid() => RunTest(MakeValidTestFileName(RuleId.AuthorHighQualityMessages, nameof(RuleId.AuthorHighQualityMessages))); diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Invalid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Invalid.sarif new file mode 100644 index 000000000..8e3dee912 --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Invalid.sarif @@ -0,0 +1,76 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing", + "rules": [ + { + "id": "SARIF1012", + "name": "MessageArgumentsMustBeConsistentWithRule", + "shortDescription": { + "text": "Placeholder." + }, + "fullDescription": { + "text": "Placeholder" + }, + "messageStrings": { + "Error_MessageIdMustExist": { + "text": "{0}: Placeholder '{1}' '{2}'" + }, + "Error_SupplyEnoughMessageArguments": { + "text": "{0}: Placeholder '{1}' '{2}' '{3}' '{4}' '{5}'" + } + }, + "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html" + } + ] + } + }, + "invocations": [ + { + "executionSuccessful": true + } + ], + "artifacts": [ + { + "location": { + "uri": "FunctionalTestOutput.ValidateCommand/Inputs.SARIF1012.MessageArgumentsMustBeConsistentWithRule_Invalid.sarif", + "uriBaseId": "TEST_DIR" + } + } + ], + "results": [ + { + "ruleId": "SARIF1012", + "ruleIndex": 0, + "level": "error", + "message": { + "id": "Error_MessageIdMustExist", + "arguments": [ + "runs[0].results[1]", + "DoesNotExist", + "TEST1001" + ] + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0 + }, + "region": { + "startLine": 36, + "startColumn": 9 + } + } + } + ] + } + ], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Valid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Valid.sarif new file mode 100644 index 000000000..1f9513761 --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Valid.sarif @@ -0,0 +1,28 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing" + } + }, + "invocations": [ + { + "executionSuccessful": true + } + ], + "artifacts": [ + { + "location": { + "uri": "FunctionalTestOutput.ValidateCommand/Inputs.SARIF1012.MessageArgumentsMustBeConsistentWithRule_Valid.sarif", + "uriBaseId": "TEST_DIR" + } + } + ], + "results": [], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Invalid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Invalid.sarif new file mode 100644 index 000000000..82c78fc4b --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Invalid.sarif @@ -0,0 +1,50 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing", + "version": "1.2.3", + "rules": [ + { + "id": "TEST1001", + "fullDescription": { + "text": "Test 1001 full description." + }, + "messageStrings": { + "DoesExist": { + "text": "'{0}': Placeholder '{1}'." + } + } + } + ] + } + }, + "results": [ + { + "ruleId": "TEST1001", + "ruleIndex": 0, + "message": { + "id": "DoesExist", + "arguments": [ + "runs[0].originalUriBaseIds.SRCINVALID" + ] + } + }, + { + "ruleId": "TEST1001", + "ruleIndex": 0, + "message": { + "id": "DoesNotExist", + "arguments": [ + "runs[0].originalUriBaseIds.SRCINVALID" + ] + } + } + ], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Valid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Valid.sarif new file mode 100644 index 000000000..1e5cefa32 --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1012.MessageArgumentsMustBeConsistentWithRule_Valid.sarif @@ -0,0 +1,41 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing", + "version": "1.2.3", + "rules": [ + { + "id": "TEST1001", + "fullDescription": { + "text": "Test 1001 full description." + }, + "messageStrings": { + "DoesExist": { + "text": "'{0}': Placeholder '{1}'." + } + } + } + ] + } + }, + "results": [ + { + "ruleId": "TEST1001", + "ruleIndex": 0, + "message": { + "id": "DoesExist", + "arguments": [ + "runs[0].originalUriBaseIds.SRCINVALID", + "SRCINVALID" + ] + } + } + ], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file