Merging artifacts during merge command #2285
Conversation
src/Sarif/Core/Result.cs
Outdated
| @@ -117,6 +123,39 @@ public bool TryIsSuppressed(out bool isSuppressed) | |||
| return true; | |||
| } | |||
|
|
|||
| public void InlineArtifacts(Run run = null) | |||
There was a problem hiding this comment.
InlineArtifacts [](start = 20, length = 15)
Remember that a result has an optional run object on it. And so if we find that, we should use it. Otherwise, this looks good.
We have another inlining problem, which is that a result needs to have a 'flattened' result message in cases where it otherwise has only messge args and an index to the rules table (which actually holds the message string). We have helpers elsewhere that provide this flattening, there's a visitor dedicated to it. Should make sure useful helper exists on Rule. btw - all of this stuff is helpful to enable optionally in the rewrite command. #Closed
| public bool ShouldSerializeLevel() { return this.Level != FailureLevel.Warning; } | ||
| public bool ShouldSerializeLevel() | ||
| { | ||
| return this.Level != FailureLevel.Warning; |
There was a problem hiding this comment.
FailureLevel.Warning [](start = 33, length = 20)
This is wrong, we should only set this if level doesn't correspond to the default rule level. Also, this value relates to kind! So, if this isn't a failure result, level == 0. #2077
| } | ||
|
|
||
| if (!_ruleIdToResultsMap[key].Contains(result)) | ||
| { | ||
| _ruleIdToResultsMap[key].Add(result); | ||
| splitRun.Results.Add(result); | ||
| _ruleIdToMergeVisitorsMap[key].CurrentRun = result.Run; |
There was a problem hiding this comment.
ruleIdToMergeVisitorsMap[key] [](start = 37, length = 29)
use a variable (of RunMergingVisitor type) for this.
| "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", | ||
| "version": "2.1.0", |
There was a problem hiding this comment.
version [](start = 3, length = 7)
Looks good! Please get some example PREfast files and let's test them explicitly.
Fixes #2284