[BUG] Large File Deletion: linesOfCode Reports Zero Changes

[nhanvt-eh]

Describe the bug
I'm utilizing DangerJS within our GitHub Actions workflow to enforce code change limits. Specifically, I'm using danger.git.linesOfCode to calculate the number of lines modified, with the intention of ignoring changes matching certain patterns.

However, I have discovered that danger.git.linesOfCode is producing inaccurate results when large files are deleted in a pull request (PR). This is due to the GitHub API not consistently returning the patch data for these deleted files, which danger.git.linesOfCode relies upon for its calculations.

To Reproduce
Steps to reproduce the behavior:

1. Use danger.git.linesOfCode in your Dangerfile to track line changes.
2. Create a PR that deletes a large file (e.g., for me, it was a file with 10,000 lines of code).
3. Observe that danger.git.linesOfCode incorrectly reports zero lines changed for the removed file (which contained 10,000+ lines). Smaller files report correct line changes

Expected behavior
danger.git.linesOfCode should accurately report the number of deleted lines for large files

Screenshots
Upon investigation, the issue appears to stem from the GitHub API's response when retrieving file changes for a PR. The API call repos/${repo}/pulls/${prID}/files?page=${page}&per_page=${perPage} is used to fetch file data. For large file deletions, the patch field is missing from the API response.

This missing patch data directly impacts the calculation performed by danger.git.linesOfCode, as shown in the following code flow:

• danger-js/source/platforms/git/gitJSONToGitDSL.ts

  Line 158 in 1cdf428

  const linesOfCode = async (pattern?: string) => {

  -

  danger-js/source/platforms/git/gitJSONToGitDSL.ts

  Line 221 in 1cdf428

  const diffForFile = async (filename: string) => {

  -

  danger-js/source/platforms/git/gitJSONToGitDSL.ts

  Line 195 in 1cdf428

  const structuredDiffForFile = async (filename: string): Promise<StructuredDiff | null> => {

  -

  danger-js/source/platforms/github/GitHubGit.ts

  Line 58 in 1cdf428

  getFullDiff: ghAPI.getPullRequestDiff,

  -

  danger-js/source/platforms/github/GitHubAPI.ts

  Line 373 in 1cdf428

  getPullRequestDiff = async (): Promise<string> => {

  .

Your Environment

software	version
danger.js	12.3.4

[orta]

That's both reasonable from Github's side and a bit of a shame, perhaps there's a higher level API route for that which could be used in those cases

[nhanvt-eh]

That's both reasonable from Github's side and a bit of a shame, perhaps there's a higher level API route for that which could be used in those cases

The API also returns additions, deletions, changes. Could we adapt danger.git.linesOfCode to somehow work with these instead of just patch? 🤔

[fbartho]

If I remember correctly (am not at desk) Danger.git is a platform-independent abstraction.

I definitely think your suggestion would be a useful utility. Platform specific abstractions should live in the relevant platform or provider adapter files. Happy to review a PR.

If you implement it in the one for GitHub, as a follow-up, I’d be open to the discussion of whether this should be an extension point for all platforms/providers. (Does this kind of thing sound useful for GitLab?)

[kimroen]

I ran into this today too, but on Gitlab. Any change that appears as collapsed in the diff view in merge requests are not counted by linesOfCode() (normally something like a lockfile). Gitlab also appears to have similar functionality in their api, providing added_lines and removed_lines both for the MR as a whole (in diffs_metadata) and each file (in diffs_batch).