Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for git blame. #2358

Merged
merged 10 commits into from
May 27, 2021
Merged

Adding support for git blame. #2358

merged 10 commits into from
May 27, 2021

Conversation

suvamM
Copy link
Collaborator

@suvamM suvamM commented May 21, 2021

This PR adds support for enriching a SARIF log with git blame information. The high level algorithm is as follows:

  1. For each result, if the corresponding artifact is a file, retrieve blame information associated with the file.
  2. Parse the blame information to extract relevant information (such as author name, email, etc).
  3. If the code location of the SARIF result overlaps with the blame hunk, enrich the result node with the extracted blame information.

The code changes have been manually tested, but the PR currently does not introduce unit tests for the feature. The problem lies with mocking appropriate external services, since the relevant code creates local objects to interact with these external services (such as GitHelper).

Also, the OptionallyEmittedData.All flag now assumes slightly different semantics, where it implies all flags except the overwrite and git blame flags. This was necessary since the existing test for the OptionallyEmittedData.All flag does not support the preconditions for git blame.

@suvamM suvamM requested a review from eddynaka May 21, 2021 13:45
@lgtm-com
Copy link

lgtm-com bot commented May 22, 2021

This pull request introduces 1 alert when merging c2cb3f3 into 2f3eb01 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@suvamM suvamM added the area-multitool Multitool command implementations label May 23, 2021
@suvamM suvamM marked this pull request as ready for review May 24, 2021 05:29
@suvamM
Copy link
Collaborator Author

suvamM commented May 24, 2021

@eddynaka Made a commit incorporating your suggestions. The unit test for the git blame parser uncovered a couple of bugs! Things are fixed now. Look forward to your feedback.

src/Sarif/BlameHunk.cs Outdated Show resolved Hide resolved
src/Sarif/BlameHunk.cs Outdated Show resolved Hide resolved
@eddynaka eddynaka merged commit ecf33f8 into main May 27, 2021
@eddynaka eddynaka deleted the users/suvam/git-blame-support branch May 27, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-multitool Multitool command implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants