Adding rule SARIF1012 #1944
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| // 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 System.Linq; | ||
| using System.Text.RegularExpressions; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules | ||
| { | ||
| public class MessageArgumentsMustBeConsistentWithRule : SarifValidationSkimmerBase | ||
| { | ||
| /// <summary> | ||
| /// SARIF1012 | ||
| /// </summary> | ||
| public override string Id => RuleId.MessageArgumentsMustBeConsistentWithRule; | ||
|
|
||
| /// <summary> | ||
| /// Placeholder | ||
| /// </summary> | ||
| public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_FullDescription_Text }; | ||
|
|
||
| protected override IEnumerable<string> MessageResourceNames => new string[] { | ||
| nameof(RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_MessageIdMustExist_Text), | ||
| nameof(RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_SupplyEnoughMessageArguments_Text) | ||
| }; | ||
|
|
||
| public override FailureLevel DefaultLevel => FailureLevel.Error; | ||
|
|
||
| private IList<ReportingDescriptor> 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 not null, we have to check if that exists in rules | ||
| if (!string.IsNullOrEmpty(result.Message.Id)) | ||
| { | ||
| ReportingDescriptor rule = result.GetRule(this.run); | ||
|
|
||
| if (this.currentRules == null | ||
| || rule.MessageStrings == null | ||
| || !rule.MessageStrings.ContainsKey(result.Message.Id)) | ||
| { | ||
| // {0}: Placeholder {1} {2} | ||
| LogResult( | ||
| resultPointer, | ||
| nameof(RuleResources.SARIF1012_MessageArgumentsMustBeConsistentWithRule_Error_MessageIdMustExist_Text), | ||
| result.Message.Id, | ||
| result.ResolvedRuleId(run) ?? "null"); | ||
| return; | ||
| } | ||
|
|
||
| // we have rules, we find it but, the count of parameters are invalid | ||
|
There was a problem hiding this comment.
"A message with the specified key is present in the rule. Check if the result supplied enough arguments." #Closed |
||
| string messageText = rule.MessageStrings[result.Message.Id].Text; | ||
| int countOfPlaceholders = CountOfPlaceholders(messageText); | ||
|
There was a problem hiding this comment.
It's not the number of placeholders that matters. It's the highest numbered placeholder. If the highest numbered placeholder is There was a problem hiding this comment. |
||
| if (countOfPlaceholders > (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(), | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. There was a problem hiding this comment. |
||
| result.Message.Id, | ||
| result.ResolvedRuleId(run) ?? "null", | ||
| countOfPlaceholders.ToString(), | ||
| messageText); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private int CountOfPlaceholders(string text) | ||
| { | ||
| MatchCollection matchCollection = Regex.Matches(text, "{\\d+}"); | ||
|
There was a problem hiding this comment.
You can use a regex "capture" to get just the integer part: There was a problem hiding this comment. Also, In reply to: 447722378 [](ancestors = 447722378) There was a problem hiding this comment. Finally, note that "verbatim strings" ( In reply to: 447723010 [](ancestors = 447723010,447722378) |
||
| HashSet<string> matches = new HashSet<string>(); | ||
| foreach (Match match in matchCollection) | ||
| { | ||
| matches.Add(match.Value); | ||
| } | ||
|
|
||
| return matches.Count; | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| { | ||
| "$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" | ||
| }, | ||
| { | ||
| "id": "SARIF2001", | ||
| "name": "AuthorHighQualityMessages", | ||
| "shortDescription": { | ||
| "text": "Follow authoring practices that make your rule messages readable, understandable, and actionable." | ||
| }, | ||
| "fullDescription": { | ||
| "text": "Follow authoring practices that make your rule messages readable, understandable, and actionable.\r\n\r\nIncluding \"dynamic content\" (information that varies among results from the same rule) makes your messages more specific. It avoids the \"wall of bugs\" phenomenon, where hundreds of occurrences of the same message appear unapproachable.\r\n\r\nPlacing dynamic content in quotes sets it off from the static text, making it easier to spot. It's especially helpful when the dynamic content is a string that might contain spaces, and most especially when the string might be empty (and so would be invisible if it weren't for the quotes). We recommend single quotes for a less cluttered appearance, even though English usage would require double quotes.\r\n\r\nFinally, write in complete sentences and end each sentence with a period. This guidance does not apply to Markdown messages, which might include formatting that makes the punctuation unnecessary." | ||
| }, | ||
| "messageStrings": { | ||
| "Warning_EnquoteDynamicContent": { | ||
| "text": "{0}: In rule '{1}', the message with id '{2}' includes dynamic content that is not enclosed in single quotes. Enquoting dynamic content makes it easier to spot, and single quotes give a less cluttered appearance." | ||
| }, | ||
| "Warning_IncludeDynamicContent": { | ||
| "text": "{0}: In rule '{1}', the message with id '{2}' does not include any dynamic content. Dynamic content makes your messages more specific and avoids the \"wall of bugs\" phenomenon." | ||
| }, | ||
| "Warning_TerminateWithPeriod": { | ||
| "text": "{0}: In rule '{1}', the message with id '{2}' does not end in a period. Write rule messages as complete sentences." | ||
| } | ||
| }, | ||
| "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[0]", | ||
| "DoesNotExist", | ||
| "0" | ||
| ] | ||
| }, | ||
| "locations": [ | ||
| { | ||
| "physicalLocation": { | ||
| "artifactLocation": { | ||
| "index": 0 | ||
| }, | ||
| "region": { | ||
| "startLine": 44, | ||
| "startColumn": 9 | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "ruleId": "SARIF1012", | ||
| "ruleIndex": 0, | ||
| "level": "error", | ||
| "message": { | ||
| "id": "Error_SupplyEnoughMessageArguments", | ||
| "arguments": [ | ||
| "runs[0].results[1]", | ||
| "1", | ||
| "DoesExist", | ||
| "0", | ||
| "3", | ||
| "{0}: Placeholder {1} {2}." | ||
| ] | ||
| }, | ||
| "locations": [ | ||
| { | ||
| "physicalLocation": { | ||
| "artifactLocation": { | ||
| "index": 0 | ||
| }, | ||
| "region": { | ||
| "startLine": 69, | ||
| "startColumn": 9 | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "ruleId": "SARIF2001", | ||
| "ruleIndex": 1, | ||
| "message": { | ||
| "id": "Warning_EnquoteDynamicContent", | ||
| "arguments": [ | ||
| "runs[0].tool.driver.rules[0].messageStrings.DoesExist.text", | ||
| "TEST1001", | ||
| "DoesExist" | ||
| ] | ||
| }, | ||
| "locations": [ | ||
| { | ||
| "physicalLocation": { | ||
| "artifactLocation": { | ||
| "index": 0 | ||
| }, | ||
| "region": { | ||
| "startLine": 22, | ||
| "startColumn": 53 | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "columnKind": "utf16CodeUnits" | ||
| } | ||
| ] | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use full sentences for comments (capitalize first letter, end with period). Also I suggest this phrasing:
If message.id is present, check that a message with that id exists in the rule.#Closed