{"action":"created","comment":{"url":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/comments/243719244","pull_request_review_id":187572567,"id":243719244,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDI0MzcxOTI0NA==","diff_hunk":"@@ -38,6 +38,11 @@ public bool Equals(Location left, Location right)\n                 return false;\n             }\n \n+            if (left.LogicalLocationIndex != right.LogicalLocationIndex)\n+            {\n+                return false;\n+            }","path":"src/Sarif/Autogenerated/LocationEqualityComparer.cs","position":7,"original_position":7,"commit_id":"37b9568f74eb44d7d86e0f0628f62df7b3fe17d4","original_commit_id":"37b9568f74eb44d7d86e0f0628f62df7b3fe17d4","user":{"login":"lgolding","id":11762653,"node_id":"MDQ6VXNlcjExNzYyNjUz","avatar_url":"https://avatars0.githubusercontent.com/u/11762653?v=4","gravatar_id":"","url":"https://api.github.com/users/lgolding","html_url":"https://github.com/lgolding","followers_url":"https://api.github.com/users/lgolding/followers","following_url":"https://api.github.com/users/lgolding/following{/other_user}","gists_url":"https://api.github.com/users/lgolding/gists{/gist_id}","starred_url":"https://api.github.com/users/lgolding/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lgolding/subscriptions","organizations_url":"https://api.github.com/users/lgolding/orgs","repos_url":"https://api.github.com/users/lgolding/repos","events_url":"https://api.github.com/users/lgolding/events{/privacy}","received_events_url":"https://api.github.com/users/lgolding/received_events","type":"User","site_admin":false},"body":">} [](start = 12, length = 1)\r\n\r\nThis is relevant to your comment in the PR description: that two locations with different indices in different log files might actually turn out to be the same. As I continue the review I expect to see how we avoid this potentially incorrect generated behavior.","created_at":"2018-12-22T00:49:09Z","updated_at":"2018-12-22T00:49:09Z","html_url":"https://github.com/Microsoft/sarif-sdk/pull/1170#discussion_r243719244","pull_request_url":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/1170","author_association":"COLLABORATOR","_links":{"self":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/comments/243719244"},"html":{"href":"https://github.com/Microsoft/sarif-sdk/pull/1170#discussion_r243719244"},"pull_request":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/1170"}}},"pull_request":{"url":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/1170","id":240582787,"node_id":"MDExOlB1bGxSZXF1ZXN0MjQwNTgyNzg3","html_url":"https://github.com/Microsoft/sarif-sdk/pull/1170","diff_url":"https://github.com/Microsoft/sarif-sdk/pull/1170.diff","patch_url":"https://github.com/Microsoft/sarif-sdk/pull/1170.patch","issue_url":"https://api.github.com/repos/Microsoft/sarif-sdk/issues/1170","number":1170,"state":"open","locked":false,"title":"Logical locations dictionaries to arrays","user":{"login":"michaelcfanning","id":13526455,"node_id":"MDQ6VXNlcjEzNTI2NDU1","avatar_url":"https://avatars3.githubusercontent.com/u/13526455?v=4","gravatar_id":"","url":"https://api.github.com/users/michaelcfanning","html_url":"https://github.com/michaelcfanning","followers_url":"https://api.github.com/users/michaelcfanning/followers","following_url":"https://api.github.com/users/michaelcfanning/following{/other_user}","gists_url":"https://api.github.com/users/michaelcfanning/gists{/gist_id}","starred_url":"https://api.github.com/users/michaelcfanning/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/michaelcfanning/subscriptions","organizations_url":"https://api.github.com/users/michaelcfanning/orgs","repos_url":"https://api.github.com/users/michaelcfanning/repos","events_url":"https://api.github.com/users/michaelcfanning/events{/privacy}","received_events_url":"https://api.github.com/users/michaelcfanning/received_events","type":"User","site_admin":false},"body":"The core change here is the transformation of logical locations from dictionaries (which are 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).\r\nIn 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).\r\nIn 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.\r\nFor 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.\r\nFinally, we must update the logical that transforms v1 to v2 and vice versa.\r\nWhew. 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.\r\nThis 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).\r\n\r\n@lgolding @EasyRhinoMSFT  @rtaket ","created_at":"2018-12-21T21:37:08Z","updated_at":"2018-12-22T00:49:09Z","closed_at":null,"merged_at":null,"merge_commit_sha":"38c8cf3a0a38d510bab483df386cb8cc23f8955a","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/1170/commits","review_comments_url":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/1170/comments","review_comment_url":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Microsoft/sarif-sdk/issues/1170/comments","statuses_url":"https://api.github.com/repos/Microsoft/sarif-sdk/statuses/37b9568f74eb44d7d86e0f0628f62df7b3fe17d4","head":{"label":"Microsoft:dictionaries-to-arrays","ref":"dictionaries-to-arrays","sha":"37b9568f74eb44d7d86e0f0628f62df7b3fe17d4","user":{"login":"Microsoft","id":6154722,"node_id":"MDEyOk9yZ2FuaXphdGlvbjYxNTQ3MjI=","avatar_url":"https://avatars2.githubusercontent.com/u/6154722?v=4","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"repo":{"id":45640472,"node_id":"MDEwOlJlcG9zaXRvcnk0NTY0MDQ3Mg==","name":"sarif-sdk","full_name":"Microsoft/sarif-sdk","private":false,"owner":{"login":"Microsoft","id":6154722,"node_id":"MDEyOk9yZ2FuaXphdGlvbjYxNTQ3MjI=","avatar_url":"https://avatars2.githubusercontent.com/u/6154722?v=4","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/Microsoft/sarif-sdk","description":".NET code and supporting files for working with the 'Static Analysis Results Interchange Format' (SARIF, see https://github.com/sarif-standard/sarif-spec)","fork":false,"url":"https://api.github.com/repos/Microsoft/sarif-sdk","forks_url":"https://api.github.com/repos/Microsoft/sarif-sdk/forks","keys_url":"https://api.github.com/repos/Microsoft/sarif-sdk/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Microsoft/sarif-sdk/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Microsoft/sarif-sdk/teams","hooks_url":"https://api.github.com/repos/Microsoft/sarif-sdk/hooks","issue_events_url":"https://api.github.com/repos/Microsoft/sarif-sdk/issues/events{/number}","events_url":"https://api.github.com/repos/Microsoft/sarif-sdk/events","assignees_url":"https://api.github.com/repos/Microsoft/sarif-sdk/assignees{/user}","branches_url":"https://api.github.com/repos/Microsoft/sarif-sdk/branches{/branch}","tags_url":"https://api.github.com/repos/Microsoft/sarif-sdk/tags","blobs_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/refs{/sha}","trees_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Microsoft/sarif-sdk/statuses/{sha}","languages_url":"https://api.github.com/repos/Microsoft/sarif-sdk/languages","stargazers_url":"https://api.github.com/repos/Microsoft/sarif-sdk/stargazers","contributors_url":"https://api.github.com/repos/Microsoft/sarif-sdk/contributors","subscribers_url":"https://api.github.com/repos/Microsoft/sarif-sdk/subscribers","subscription_url":"https://api.github.com/repos/Microsoft/sarif-sdk/subscription","commits_url":"https://api.github.com/repos/Microsoft/sarif-sdk/commits{/sha}","git_commits_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/commits{/sha}","comments_url":"https://api.github.com/repos/Microsoft/sarif-sdk/comments{/number}","issue_comment_url":"https://api.github.com/repos/Microsoft/sarif-sdk/issues/comments{/number}","contents_url":"https://api.github.com/repos/Microsoft/sarif-sdk/contents/{+path}","compare_url":"https://api.github.com/repos/Microsoft/sarif-sdk/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Microsoft/sarif-sdk/merges","archive_url":"https://api.github.com/repos/Microsoft/sarif-sdk/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Microsoft/sarif-sdk/downloads","issues_url":"https://api.github.com/repos/Microsoft/sarif-sdk/issues{/number}","pulls_url":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls{/number}","milestones_url":"https://api.github.com/repos/Microsoft/sarif-sdk/milestones{/number}","notifications_url":"https://api.github.com/repos/Microsoft/sarif-sdk/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Microsoft/sarif-sdk/labels{/name}","releases_url":"https://api.github.com/repos/Microsoft/sarif-sdk/releases{/id}","deployments_url":"https://api.github.com/repos/Microsoft/sarif-sdk/deployments","created_at":"2015-11-05T21:26:46Z","updated_at":"2018-12-08T00:56:05Z","pushed_at":"2018-12-21T21:40:13Z","git_url":"git://github.com/Microsoft/sarif-sdk.git","ssh_url":"git@github.com:Microsoft/sarif-sdk.git","clone_url":"https://github.com/Microsoft/sarif-sdk.git","svn_url":"https://github.com/Microsoft/sarif-sdk","homepage":"","size":83824,"stargazers_count":32,"watchers_count":32,"language":"C#","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":26,"mirror_url":null,"archived":false,"open_issues_count":166,"license":{"key":"other","name":"Other","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":26,"open_issues":166,"watchers":32,"default_branch":"develop"}},"base":{"label":"Microsoft:develop","ref":"develop","sha":"ebbcbc75f77306e5518a48c717b88605ec19299e","user":{"login":"Microsoft","id":6154722,"node_id":"MDEyOk9yZ2FuaXphdGlvbjYxNTQ3MjI=","avatar_url":"https://avatars2.githubusercontent.com/u/6154722?v=4","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"repo":{"id":45640472,"node_id":"MDEwOlJlcG9zaXRvcnk0NTY0MDQ3Mg==","name":"sarif-sdk","full_name":"Microsoft/sarif-sdk","private":false,"owner":{"login":"Microsoft","id":6154722,"node_id":"MDEyOk9yZ2FuaXphdGlvbjYxNTQ3MjI=","avatar_url":"https://avatars2.githubusercontent.com/u/6154722?v=4","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/Microsoft/sarif-sdk","description":".NET code and supporting files for working with the 'Static Analysis Results Interchange Format' (SARIF, see https://github.com/sarif-standard/sarif-spec)","fork":false,"url":"https://api.github.com/repos/Microsoft/sarif-sdk","forks_url":"https://api.github.com/repos/Microsoft/sarif-sdk/forks","keys_url":"https://api.github.com/repos/Microsoft/sarif-sdk/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Microsoft/sarif-sdk/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Microsoft/sarif-sdk/teams","hooks_url":"https://api.github.com/repos/Microsoft/sarif-sdk/hooks","issue_events_url":"https://api.github.com/repos/Microsoft/sarif-sdk/issues/events{/number}","events_url":"https://api.github.com/repos/Microsoft/sarif-sdk/events","assignees_url":"https://api.github.com/repos/Microsoft/sarif-sdk/assignees{/user}","branches_url":"https://api.github.com/repos/Microsoft/sarif-sdk/branches{/branch}","tags_url":"https://api.github.com/repos/Microsoft/sarif-sdk/tags","blobs_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/refs{/sha}","trees_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Microsoft/sarif-sdk/statuses/{sha}","languages_url":"https://api.github.com/repos/Microsoft/sarif-sdk/languages","stargazers_url":"https://api.github.com/repos/Microsoft/sarif-sdk/stargazers","contributors_url":"https://api.github.com/repos/Microsoft/sarif-sdk/contributors","subscribers_url":"https://api.github.com/repos/Microsoft/sarif-sdk/subscribers","subscription_url":"https://api.github.com/repos/Microsoft/sarif-sdk/subscription","commits_url":"https://api.github.com/repos/Microsoft/sarif-sdk/commits{/sha}","git_commits_url":"https://api.github.com/repos/Microsoft/sarif-sdk/git/commits{/sha}","comments_url":"https://api.github.com/repos/Microsoft/sarif-sdk/comments{/number}","issue_comment_url":"https://api.github.com/repos/Microsoft/sarif-sdk/issues/comments{/number}","contents_url":"https://api.github.com/repos/Microsoft/sarif-sdk/contents/{+path}","compare_url":"https://api.github.com/repos/Microsoft/sarif-sdk/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Microsoft/sarif-sdk/merges","archive_url":"https://api.github.com/repos/Microsoft/sarif-sdk/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Microsoft/sarif-sdk/downloads","issues_url":"https://api.github.com/repos/Microsoft/sarif-sdk/issues{/number}","pulls_url":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls{/number}","milestones_url":"https://api.github.com/repos/Microsoft/sarif-sdk/milestones{/number}","notifications_url":"https://api.github.com/repos/Microsoft/sarif-sdk/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Microsoft/sarif-sdk/labels{/name}","releases_url":"https://api.github.com/repos/Microsoft/sarif-sdk/releases{/id}","deployments_url":"https://api.github.com/repos/Microsoft/sarif-sdk/deployments","created_at":"2015-11-05T21:26:46Z","updated_at":"2018-12-08T00:56:05Z","pushed_at":"2018-12-21T21:40:13Z","git_url":"git://github.com/Microsoft/sarif-sdk.git","ssh_url":"git@github.com:Microsoft/sarif-sdk.git","clone_url":"https://github.com/Microsoft/sarif-sdk.git","svn_url":"https://github.com/Microsoft/sarif-sdk","homepage":"","size":83824,"stargazers_count":32,"watchers_count":32,"language":"C#","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":26,"mirror_url":null,"archived":false,"open_issues_count":166,"license":{"key":"other","name":"Other","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":26,"open_issues":166,"watchers":32,"default_branch":"develop"}},"_links":{"self":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/1170"},"html":{"href":"https://github.com/Microsoft/sarif-sdk/pull/1170"},"issue":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/issues/1170"},"comments":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/issues/1170/comments"},"review_comments":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/1170/comments"},"review_comment":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/pulls/1170/commits"},"statuses":{"href":"https://api.github.com/repos/Microsoft/sarif-sdk/statuses/37b9568f74eb44d7d86e0f0628f62df7b3fe17d4"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"lgolding"}}