Get v2 -> v1 transform working with files array #1211
Merged
Conversation
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
added 8 commits
January 13, 2019 14:12
... but OneRunWithBasicInvocation regressed.
The output was actually correct. The converter now, as permitted by the v1 spec, omits file.uri if it matches the key.
... but there is still commented out code, showing that we have test holes.
ghost
requested a review
from
ghost
commented
Jan 14, 2019
| "collections.rsp": { | ||
| "uri": "collections.rsp" | ||
| } | ||
| "collections.rsp": {} |
There was a problem hiding this comment.
The transformer now refrains from emitting uri if it matches the key, as permitted by the v1 spec. We could go further and omit the whole object if it's empty but that's more work that I'm willing to do, and would cause problems if the empty object were in a nested file chain. #ByDesign
ghost
commented
Jan 14, 2019
| @@ -27,7 +27,7 @@ | |||
| } | |||
| }, | |||
| "file:///home/buildAgent/bin/app.zip#/docs/intro.docx": { | |||
| "uri": "file:///docs/intro.docx", | |||
There was a problem hiding this comment.
file [](start = 18, length = 4)
I fixed this existing bug in the transformer. #Resolved
There was a problem hiding this comment.
yes good catch, can you please add another test to this file that nests a second zip in this one, and another file object that is a nested file within that nested zip? this guarantees completeness in nested files testing.
In reply to: 247374942 [](ancestors = 247374942)
There was a problem hiding this comment.
ghost
commented
Jan 14, 2019
| @@ -11,16 +11,13 @@ | |||
| "invocation": {}, | |||
| "files": { | |||
| "file:///src/base/driver.cs": { | |||
| "uri": "file:///src/base/driver.cs", | |||
There was a problem hiding this comment.
Again, the transformer now doesn't bother to emit these. #ByDesign
ghost
commented
Jan 14, 2019
| @@ -69,9 +69,7 @@ private void VerifyCurrentToVersionOneTransformationFromResource(string v2InputR | |||
| File.WriteAllText(expectedFilePath, v1ExpectedLogText); | |||
| File.WriteAllText(actualFilePath, v1ActualLogText); | |||
|
|
|||
|
|
|||
| string errorMessage = string.Format(@"V2 conversion from V1 produced unexpected diffs for test: '{0}'.", v2InputResourceName); | |||
There was a problem hiding this comment.
errorMessage [](start = 23, length = 12)
This variable was never used, so this message never appeared. #Resolved
There was a problem hiding this comment.
ok. this code needs to be converted to actually use the helpers in the filediff base class. maybe good for a followon change.
In reply to: 247374990 [](ancestors = 247374990)
There was a problem hiding this comment.
There was a problem hiding this comment.
Done! This SarifCurrentToVersionOneVisitorTests.cs is now 65 lines long :-) Take a look at the changes to FileDiffingTests.cs to see how it works.
In reply to: 247582997 [](ancestors = 247582997,247377138,247374990)
ghost
commented
Jan 14, 2019
| // To deal with Case #2, we would follow the chain or uriBaseIds, prepending | ||
| // the value of each one to the URI. | ||
| // | ||
| // WE DO NOT YET HANDLE CASE #2. |
There was a problem hiding this comment.
This is the missing code and test case. #Pending
There was a problem hiding this comment.
I will handle this in a separate PR, after lunch, after I review your PR.
In reply to: 247375067 [](ancestors = 247375067)
| FileData fileData; | ||
| if (_currentV2Run.Files.TryGetValue(uri.OriginalString, out fileData)) | ||
| if (_fileKeyToIndexDictionary.TryGetValue(uri.OriginalString, out int index) | ||
| && index < files.Count) |
There was a problem hiding this comment.
&& index < files.Count [](start = 19, length = 23)
when would this index < files.Count not be true? that seems like an adverse conition #Resolved
There was a problem hiding this comment.
You're right: it's guaranteed by the way the dictionary is constructed. Removed this condition.
In reply to: 247376623 [](ancestors = 247376623)
| @@ -22,6 +22,10 @@ public class SarifCurrentToVersionOneVisitor : SarifRewritingVisitor | |||
| private Run _currentV2Run = null; | |||
| private RunVersionOne _currentRun = null; | |||
|
|
|||
| // To understand the purpose of these fields, see the comment on CreateFileKeyIndexMappings. | |||
| private IDictionary<string, int> _fileKeyToIndexDictionary; | |||
There was a problem hiding this comment.
_fileKeyToIndexDictionary [](start = 41, length = 25)
can you specify the version-ness of these things in the name? e.g, v1FileKeytoV2Index dictionary? Helps with clarity. For both members please. #Resolved
There was a problem hiding this comment.
| // The convention for building the key is as follows: | ||
| // The root file URI is separated from the chain of nested files by '#'; | ||
| // The nested file URIs are separated from each other by '/'. | ||
| string separator = parentFile.ParentIndex == -1 ? "#" : "/"; |
There was a problem hiding this comment.
string separator = parentFile.ParentIndex [](start = 16, length = 41)
are you sure this code doesn't double slashes at the point where we jump from a nested archive to an element in it?
i.e., need a test case for myarchive.zip#myarchive2.zip/foo.cpp
in this three-part file object, the most nested item will have a URL of /foo.cpp
does your code double that slash? like so myarchive.zip#myarchive2.zip//foo.cpp? #Resolved
There was a problem hiding this comment.
Good catch! I forgot that nested file URIs already start with a slash. I'll add the test case you asked for to expose this bug, and then fix it.
In reply to: 247376946 [](ancestors = 247376946)
There was a problem hiding this comment.
| string key = entry.Key; | ||
| int index = entry.Value; | ||
| FileData v2File = _currentV2Run.Files[index]; | ||
| FileDataVersionOne fileDataVersionOne = CreateFileData(v2File); |
There was a problem hiding this comment.
CreateFileData [](start = 60, length = 14)
Again prefer to version these things, e.g., CreateFileDataVersionOne #Resolved
There was a problem hiding this comment.
Yes, I agree: I always found this code confusing for that reason, and all the CreateXxx methods follow this bad pattern. I'll change them all.
In reply to: 247376992 [](ancestors = 247376992)
There was a problem hiding this comment.
|
I am mostly authoring docs tomorrow. Please update this PR based on my feedback according to your judgment and merge. After your update, I am it will LGTM. #Resolved |
|
Ok, thanks for the review. In reply to: 453900063 [](ancestors = 453900063) |
michaelcfanning
added a commit
that referenced
this pull request
Jan 28, 2019
* Fix up tests * Conversion to files array. WIP. Core SARIF component build complete except for SarifLogger tail. * Add fileIndex property to file object (#1186) * Fix up tests * PR feedback to improve schema comment * Logical locations notes (#1185) (#1187) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * Reduced files array build (#1191) * Sarif and Sarif.Converters now building * Files array (#1188) * Add fileIndex property to file object (#1186) * Fix up tests * PR feedback to improve schema comment * Logical locations notes (#1185) (#1187) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * DRY out converters to isolate shared code. * Restore essential change in schema that converts files dictionary to an array. * Simplify ShouldSerialize logic * Remove unused namespaces * Respond to PR feedback. * Respond to PR feedback * End-to-end build works. Now we can author core transformation and fix tests. (#1192) * Fix up merge from 'develop' branch. * Update supporting test code for processing checked in files. (#1202) * Update supporting test code for processing checked in files. * Update nested files test to contain single file. * Files array basic transform (#1205) * Update supporting test code for processing checked in files. * Update nested files test to contain single file. * WIP. Furhter progress * Fix up samples build * Fix up merge from basic transform branch * Mime type validation (#1206) * Fix up merge from basic transform branch * Fix up mime test * Start work on v1 <-> v2 transformation (#1208) * Restore TransformCommand and first (unaffected) unit test * Restore "minimal prerelease v2 to current v2" test. * estore "minimal v1 to current v2" test. * Restore remaining TransformCommand unit tests. * Uncomment v2 => v1 tests to observe failures. * Uncomment 'transform' command. * Restore MakeUrisAbsoluteVisitor tests (#1210) This change updates the visitor that expands URIs in the presence of `originalUriBaseIds`. Turns out there was technical debt here, because our tests provided `originalUriBaseIds` equivalents in the property bag (because we had no official SARIF slot for them). I did not notice this gap when we made the schema change to add `originalUriBaseIds`. * Get v2 -> v1 transform working with files array (#1211) Test failure count is down to 32; will be 28 when you merge your fix. There is not -- and never was -- a test case for fileLocations that use uriBaseId (never was one). I know for a fact that there is no code to support that case. You’ll see a comment to that effect in the code. I will take care of that next. Then I will move on to v1 -> v2 transform. As part of this change, the `SarifCurrentToVersionOneVisitorTests` are now based on the `RunTest` helper method from the base class `FileDiffingTests`. * Convert debug assert to exception to make test execution more deterministic (#1214) * Update insert optional data tests and update indices visitor. (#1212) * Update insert optional data tests and update indices visitor. * Delete speculatively needed file * Remove erroneous override of base visit method. * Rework summary comment on DeletesOutputsDirectoryOnClassInitializationFixture. * Update clang analyzer name. Flow converter log verification through JToken comparison. (#1213) * The multiool, core sarif, and validation test binaries now all pass (#1215) * The multiool, core sarif, and validation test binaries now all pass completely. * Remove unwanted assert that may fire during unit testing. * Merge from files-array * PR feedback. * PR feedback tweaks * Accept PR feedback from previous change. (#1216) Use LINQ IEnuemrable.Except in the unit test, which improves readability without compromising efficiency (because Except uses a Set to do its work in O(N) time). * Fix Sarif.Driver and Sarif.Functional tests. (#1217) * Fix Sarif.Driver and Sarif.Functional tests. * Restore test file * Fix Semmle tests and Fortify converter: all tests now pass. (#1218) * Sarif converters fixups (#1219) * Fix semmle tests and fority. * Final test fixups * Invoke appveyor for files-array branch.: (#1220) * Update SarifVersionOneToCurrentVisitor for run.files array (#1221) * Uncomment v1 -> v2 tests; 3/14 fail. * Move test data to locations expected by FileDiffingTests. * Fix up some IDE C#7 code cleanups. * Use FileDiffingTests helper. * Fix bug in FileDiffingTests that caused a test failure. * Remove default-valued argument from a call to RunTest. * Create basic files array Does not yet have fileIndex, parentIndex, or response file handling. * Revert incorrect change in FileDiffingTests. * Fix one unit test with spot fix to "expected" file. * Fix up some C#7 IDE warnings * Force update in FileDiffing tests to avoid deserialization errors from out of date "expected" files. * Fix missing "modified" flag sets in PreRelCompatTransformer. * Populate fileIndex in run.files array. * Fix unit test by fixing fileLocation creation. * Restore response file handling. * Populate fileIndex on fileLocations as appropriate. * Fix last test failure by reworking response file handling. * Feedback: Introduce transformer helper PopulatePropertyIfAbsent. * Feedback: Tighten platform-independent string compare. Also: - Reformat unit test lines. * Feedbakc: Revert FileDiffingTest change; downgrade affected test files to provoke transform * Basic rules transformation (except for v1 <-> v2 conversion) (#1223) * Basic rules transformation (except for v1 <-> v2 conversion) * Respond to very excellent PR feedback. * PR feedback * Add files array tests for nested files and uriBaseIds (#1226) * Add failing v1 -> v2 nested files test * Fix existing mis-handling of analysisTarget and resultFile. * Get nested files test case working. * Add failing v1 => v2 uriBaseId test. * Populate v2 uriBaseId. * Fix up expected v2 fileLocation.properties: test passes. * Enhance uriBaseIds test case. * Implement v2->v1 conversion for rules dictionary (#1228) * Notification rule index (#1229) * Add notification.ruleIndex and increase flatten messages testing * Notification message tests + add notification.ruleIndex to schema * Notification rule index (#1230) * Add notification.ruleIndex and increase flatten messages testing * Notification message tests + add notification.ruleIndex to schema * Missed feedback from previous PR (#1231) * Implement v1->v2 conversion for rules dictionary (#1232) * Partial implementation * Get last test working. * Somebody missed checking in a generated file. * Schema changes from TC #30 (#1233) * Add source language, fix rank default. * Adjust rank minimum to accommoodate default. * Fix broken test. * Remove unnecessary None items from project file. * PR feedback * Files array results matching (#1234) * WIP preliminary results matching * Restore results matching for files array * Add back autogenerated (unused) namespace directive
michaelcfanning
pushed a commit
that referenced
this pull request
Feb 6, 2019
…#1264) * Fix tests that are broken in appveyor (#1134) * Properly persist run level property bags (#1136) * Fix #1138: Add validation rule: contextRegion requires region (#1142) Also: - Enhance the "new-style" verification so that we no longer require the file "Invalid_Expected.sarif". Each file can now contain a property that specifies the expected locations of all the validation results. * Prep for 2018-11-28 schema update. Remove run.architecture. (#1145) * Add run.newlineSequences to schema (#1146) * Mark result.message as required in the schema (#1147) * Mark result.message as required in the schema * Update release history with result.message breaking change. * Fix typo in testoutput. * Rename tool.fileVersion to tool.dottedQuadFileVersion (#1148) * Upgrade more validator functional tests (#1149) We apply the new functional test pattern to four more rules: - `EndColumnMustNotBeLessThanStartColumn` - `EndLineMustNotBeLessThanStartLine` - `EndTimeMustBeAfterStartTime` (which is misnamed, and in a future PR we will rename it to `EndTimeMustNotBeBeforeStartTime`) - `MessageShouldEndWithPeriod` In addition, we remove the test data for a rule that no longer exists, `HashAlgorithmsMustBeUnique` (which no longer applies because `file.hashes` is now an object keyed by the hash algorithm). Because there are so many properties of type `message`, exhaustively testing the rule `MessageShouldEndWithPeriod` revealed many holes in the visitor class `SarifValidationSkimmerBase`, which I plugged. As we have discussed, we should generate this class from the schema. After this, there are only two more rules to convert: - `UriBaseIdRequiresRelativeUri` - `UseAbsolutePathsForNestedFileUriFragments` ... but this PR is already large enough. * Remove 'open' from list of valid rule configuration default values. (#1158) * Emit column kind default explicitly for Windows SDK SARIF emit. (#1160) * Emit column kind default explicitly for Windows SDK SARIF emit. * Update release notes * More column kind test fixes * Change behavior to always serialize column kind. * Always serialize column kind * Finish validator functional test upgrade (#1159) * Rename rule EndTimeMustBeAfterStartTime => ...MustNotBeBefore... * Upgrade UriBaseIdRequiresRelativeUri tests. * Remove obsolete rule UseAbsolutePathsForNestedFileUriFragments. * Remove support for old test design. * Remove 'package' as a documented logical location kind in the schema. Documentation only change. (#1162) * Fortify FPR converter improvements + unit tests (#1161) * Improvements and corrections Populate originalUriBaseIds from <SourceBasePath> Populate tFL.kind from <Action type="..."> Add default node to result.locations * Add location annotation for Action elements with no type attribute * Support node annotations + uribasepath + test updates * Update FortifyTest.fpr.sarif * Add converter tests & assets + opportunistic code cleanup * PR feedback * Logical locations dictionaries to arrays (#1170) The core change here is the transformation of `run.logicalLocations` from a dictionary (which is keyed, generally, by the fully qualified name of a logical location) to an array of logical locations. Result locations now reference logical locations by a logical location index. This changes removes the necessity of resolving key name collisions for logical locations that differ only by type (a namespace that collides with the fully qualified name of a type being the classic example). In addition to making the core change, we have also authored a transformation that converts existing pre-release SARIF v2 files to the new design. We accomplish this by creating dictionaries, with value type comparison for keys, that are keyed by logical locations. This processing requires that any parent keys already exist in the array (so that a logical location can populate its parent logical location index, if any). In addition to the core functionality and any transformation of individual log files, result matching presents special complications. In a result matching scenario, the logical location index of a result is not relevant to its identify: only the contents of the logical location this index points to are relevant. Furthermore, when merging a baseline file (which can contain results that are exclusive to a single log file within the matching domain), logical location indices are subject to change and must be updated. For this scenario and at least one other, we use a visitor pattern to update indices. The reason is that locations are pervasive in the format and the visitor provides a convenient mechanism to put common location processing logical. This visitor uses puts additional pressure on the transformation logic, as it entails additional deserialization of the JSON. With more time/effort, we could have exhaustively updated all locations using the JParser/JObject/etc. API domain. Oh well. Finally, we must update the logical that transforms v1 to v2 and vice versa. Whew. If that was not already sufficiently intrusive, this work revealed some minor flaws in various converters (the ones that handle logical locations): AndroidStudio, FxCop and PREfast. This change is complex but valuable. Logical locations are now expressed as coherent objects in their table. In the main, I have preferred to leave `result.fullyQualifiedName` populated (in addition to `result.logicalLocationIndex`, to support easily looking up matching logical locations). * Add result.rank and ruleConfiguration.defaultRank (#1167) As we discussed offline with @fishoak, the design is good as it stands. The only change is that the default should be -1. I filed oasis-tcs/sarif-spec#303 for that, and put it on the agenda for TC #30. * Logical locations notes (#1184) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Logical locations notes (#1185) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * Incorporate "provenance" schema changes and fix package licenses (#1193) * Add autogenerated RuleConfiguration.cs missed from earlier commit. * Upgrade to NuGet.exe 4.9.2 to handle new license tag. * Remove unused 'Owners' element from build.props. * Add package Title. * Use packageLicenseExpression to specify package license. * Suppress NU5105 (about SemVer 2.0.0 strings) for "dotnet pack" packages. NuGet.exe still warns for ".nuspec" packages. * Incorporate latest "provenance" schema changes. * Address PR feedback. * External property files (#1194) * Update spec for externalPropertiesFile object. * Add external property files to schema. * Finish merge of provenance changes. * Update release notes. * Remove vertical whitespace. * PR feedback. Fix 'properties' to refer to an external file not an actual properties bag. * Remove code gen hint that makes external property files a property bag holder. * Introduce missing brace. Fix up code emit for 'properties' property that isn't a property bag. * Incorporate schema changes for versionControlDetails.mappedTo and rule.deprecatedIds (#1198) * Incorporate "versionControlDetails.mappedTo" schema change. * Incorporate "rule.deprecatedIds" schema change. * Revert updates to comprehensive.sarif (to allow transformer to continue to use this as test content). * Array scrub part 1: everything but anyOf-or-null properties. (#1201) NOTE: For explicitness, I added schema attributes even when they matched the JSON schema defaults, namely: `"minItems": 0` and `"uniqueItems": false`. * Fix v1->v2 hash transformation (#1203) CreateHash must be called to handle algorithm names that aren't in our translation table. Also updated a unit test to cover this case. * Integrate jschema 0.61.0 into SDK (#1204) * Merging arrays transformations back into 'develop' branch (#1236) * Fix up tests * Conversion to files array. WIP. Core SARIF component build complete except for SarifLogger tail. * Add fileIndex property to file object (#1186) * Fix up tests * PR feedback to improve schema comment * Logical locations notes (#1185) (#1187) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * Reduced files array build (#1191) * Sarif and Sarif.Converters now building * Files array (#1188) * Add fileIndex property to file object (#1186) * Fix up tests * PR feedback to improve schema comment * Logical locations notes (#1185) (#1187) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * DRY out converters to isolate shared code. * Restore essential change in schema that converts files dictionary to an array. * Simplify ShouldSerialize logic * Remove unused namespaces * Respond to PR feedback. * Respond to PR feedback * End-to-end build works. Now we can author core transformation and fix tests. (#1192) * Fix up merge from 'develop' branch. * Update supporting test code for processing checked in files. (#1202) * Update supporting test code for processing checked in files. * Update nested files test to contain single file. * Files array basic transform (#1205) * Update supporting test code for processing checked in files. * Update nested files test to contain single file. * WIP. Furhter progress * Fix up samples build * Fix up merge from basic transform branch * Mime type validation (#1206) * Fix up merge from basic transform branch * Fix up mime test * Start work on v1 <-> v2 transformation (#1208) * Restore TransformCommand and first (unaffected) unit test * Restore "minimal prerelease v2 to current v2" test. * estore "minimal v1 to current v2" test. * Restore remaining TransformCommand unit tests. * Uncomment v2 => v1 tests to observe failures. * Uncomment 'transform' command. * Restore MakeUrisAbsoluteVisitor tests (#1210) This change updates the visitor that expands URIs in the presence of `originalUriBaseIds`. Turns out there was technical debt here, because our tests provided `originalUriBaseIds` equivalents in the property bag (because we had no official SARIF slot for them). I did not notice this gap when we made the schema change to add `originalUriBaseIds`. * Get v2 -> v1 transform working with files array (#1211) Test failure count is down to 32; will be 28 when you merge your fix. There is not -- and never was -- a test case for fileLocations that use uriBaseId (never was one). I know for a fact that there is no code to support that case. You’ll see a comment to that effect in the code. I will take care of that next. Then I will move on to v1 -> v2 transform. As part of this change, the `SarifCurrentToVersionOneVisitorTests` are now based on the `RunTest` helper method from the base class `FileDiffingTests`. * Convert debug assert to exception to make test execution more deterministic (#1214) * Update insert optional data tests and update indices visitor. (#1212) * Update insert optional data tests and update indices visitor. * Delete speculatively needed file * Remove erroneous override of base visit method. * Rework summary comment on DeletesOutputsDirectoryOnClassInitializationFixture. * Update clang analyzer name. Flow converter log verification through JToken comparison. (#1213) * The multiool, core sarif, and validation test binaries now all pass (#1215) * The multiool, core sarif, and validation test binaries now all pass completely. * Remove unwanted assert that may fire during unit testing. * Merge from files-array * PR feedback. * PR feedback tweaks * Accept PR feedback from previous change. (#1216) Use LINQ IEnuemrable.Except in the unit test, which improves readability without compromising efficiency (because Except uses a Set to do its work in O(N) time). * Fix Sarif.Driver and Sarif.Functional tests. (#1217) * Fix Sarif.Driver and Sarif.Functional tests. * Restore test file * Fix Semmle tests and Fortify converter: all tests now pass. (#1218) * Sarif converters fixups (#1219) * Fix semmle tests and fority. * Final test fixups * Invoke appveyor for files-array branch.: (#1220) * Update SarifVersionOneToCurrentVisitor for run.files array (#1221) * Uncomment v1 -> v2 tests; 3/14 fail. * Move test data to locations expected by FileDiffingTests. * Fix up some IDE C#7 code cleanups. * Use FileDiffingTests helper. * Fix bug in FileDiffingTests that caused a test failure. * Remove default-valued argument from a call to RunTest. * Create basic files array Does not yet have fileIndex, parentIndex, or response file handling. * Revert incorrect change in FileDiffingTests. * Fix one unit test with spot fix to "expected" file. * Fix up some C#7 IDE warnings * Force update in FileDiffing tests to avoid deserialization errors from out of date "expected" files. * Fix missing "modified" flag sets in PreRelCompatTransformer. * Populate fileIndex in run.files array. * Fix unit test by fixing fileLocation creation. * Restore response file handling. * Populate fileIndex on fileLocations as appropriate. * Fix last test failure by reworking response file handling. * Feedback: Introduce transformer helper PopulatePropertyIfAbsent. * Feedback: Tighten platform-independent string compare. Also: - Reformat unit test lines. * Feedbakc: Revert FileDiffingTest change; downgrade affected test files to provoke transform * Basic rules transformation (except for v1 <-> v2 conversion) (#1223) * Basic rules transformation (except for v1 <-> v2 conversion) * Respond to very excellent PR feedback. * PR feedback * Add files array tests for nested files and uriBaseIds (#1226) * Add failing v1 -> v2 nested files test * Fix existing mis-handling of analysisTarget and resultFile. * Get nested files test case working. * Add failing v1 => v2 uriBaseId test. * Populate v2 uriBaseId. * Fix up expected v2 fileLocation.properties: test passes. * Enhance uriBaseIds test case. * Implement v2->v1 conversion for rules dictionary (#1228) * Notification rule index (#1229) * Add notification.ruleIndex and increase flatten messages testing * Notification message tests + add notification.ruleIndex to schema * Notification rule index (#1230) * Add notification.ruleIndex and increase flatten messages testing * Notification message tests + add notification.ruleIndex to schema * Missed feedback from previous PR (#1231) * Implement v1->v2 conversion for rules dictionary (#1232) * Partial implementation * Get last test working. * Somebody missed checking in a generated file. * Schema changes from TC #30 (#1233) * Add source language, fix rank default. * Adjust rank minimum to accommoodate default. * Fix broken test. * Remove unnecessary None items from project file. * PR feedback * Files array results matching (#1234) * WIP preliminary results matching * Restore results matching for files array * Add back autogenerated (unused) namespace directive * Updated release notes for TC30 changes. (#1240) * Mention rules array change in release history. (#1243) * Baseline states (#1245) * Add 'updated' state to baselineState and rename 'existing' to 'unchanged' * Update prerelease transformer * Enable appveyor build + test. Correct version constant. * Update test. Respond to PR feedback. * Fix #1251 #1252 #1253 (#1254) * Fixes + test coverage + cleanup * Update SDK version * Update version in test assets * Fix multitool nuspec (#1256) * Revert unintentional change to BaselineState (#1262) The `develop` branch should match TC <span>#</span>30, but we inadvertently introduced a change from TC <span>#</span>31: replacing `BaselineState.Existing` with `.Unchanged` and `Updated`. I did not revert the entire change. Some things (like having AppVeyor build the `tc-31` branch instead of the defunct `files-array` branch, and some C# 7 updates to the `PrereleaseCompatibilityTransformer`) were good, and I kept them. Also: - Update the version to `2019-01-09` in preparation for merge to `master`. * Transfer Bogdan's point fix (analysisTarget handling) from master to develop (#1263) In preparation for merging `develop` to `master` for the publication of version 2019-01-09 (TC <span>#</span>30), we apply the recent changes in `master` to the `develop` branch. These changes fixed two bugs in the handling of `analysisTarget` in the v1-to-v2 converter (`SarifVersionOneToCurrentVisitor`). Now `develop` is completely up to date, and when we merge `develop` to `master`, we _should_ be able to simply take the "incoming" changes on all conflicting files. * Cherry-pick: v1 transformer analysis target region persistence fix. (#1238) * Mention NuGet publishing changes in release history. * Cherry pick: Don't emit v2 analysisTarget if there is no v1 resultFile. (#1247)
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Test failure count is down to 32; will be 28 when you merge your fix.
There is not -- and never was -- a test case for fileLocations that use uriBaseId (never was one). I know for a fact that there is no code to support that case. You’ll see a comment to that effect in the code. I will take care of that next. Then I will move on to v1 -> v2 transform.