fix: GHPullRequestReviewComment.getLine() returning -1

[Anonycoders]

Description

• Fixes GHPullRequestReviewComment.getLine() always returns -1 #2106

Problem

When fetching review comments through review.listReviewComments(), the getLine() method always returns -1. This is because the GitHub API endpoint /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/comments does not return line, original_line, side, and related fields.

Solution

Introduced a new GHPullRequestReview.ReviewComment type that accurately represents the limited data returned by the review-specific comments endpoint, making the API limitation visible through the type system.

1. New GHPullRequestReview.ReviewComment class — contains only the fields present in the review comments endpoint response (body, path, position, etc.). Users cannot accidentally call getLine() on this type.

2. readPullRequestReviewComment() method — allows fetching the full GHPullRequestReviewComment with all fields when needed:

for (GHPullRequestReview.ReviewComment comment : review.listReviewComments()) {
      GHPullRequestReviewComment full = comment.readPullRequestReviewComment();
      int line = full.getLine(); // Returns actual value
}

3. Breaking change to GHPullRequestReview#listReviewComments() — return type changed from PagedIterable<GHPullRequestReviewComment> to PagedIterable<GHPullRequestReview.ReviewComment>.
4. Javadoc — documented the API limitation on getLine(), getSide(), and related methods on GHPullRequestReviewComment, and cross-referenced both listReviewComments() methods.

Alternative: use GHPullRequest#listReviewComments()

Users who need line data for all comments can use the pull request-level endpoint which returns full GHPullRequestReviewComment objects:

  for (GHPullRequestReviewComment comment : pullRequest.listReviewComments()) {
      int line = comment.getLine(); // Returns actual value
      Long reviewId = comment.getPullRequestReviewId(); // Filter by review if needed
  }

Before submitting a PR:

• Changes must not break binary backwards compatibility. If you are unclear on how to make the change you think is needed while maintaining backward compatibility, CONTRIBUTING.md for details.
• Add JavaDocs and other comments explaining the behavior.
• When adding or updating methods that fetch entities, add @link JavaDoc entries to the relevant documentation on https://docs.github.com/en/rest .
• Add tests that cover any added or changed code. This generally requires capturing snapshot test data. See CONTRIBUTING.md for details.
• Run mvn -D enable-ci clean install site "-Dsurefire.argLine=--add-opens java.base/java.net=ALL-UNNAMED" locally. If this command doesn't succeed, your change will not pass CI.
• Push your changes to a branch other than main. You will create your PR from that branch.

When creating a PR:

• Fill in the "Description" above with clear summary of the changes. This includes:
  • If this PR fixes one or more issues, include "Fixes #" lines for each issue.
  • Provide links to relevant documentation on https://docs.github.com/en/rest where possible. If not including links, explain why not.
• All lines of new code should be covered by tests as reported by code coverage. Any lines that are not covered must have PR comments explaining why they cannot be covered. For example, "Reaching this particular exception is hard and is not a particular common scenario."
• Enable "Allow edits from maintainers".

[codecov]

Codecov Report

❌ Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.06%. Comparing base (9152b37) to head (daf7583).

Files with missing lines	Patch %	Lines
...n/java/org/kohsuke/github/GHPullRequestReview.java	92.30%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2188      +/-   ##
============================================
+ Coverage     85.03%   85.06%   +0.02%     
  Complexity     2499     2499              
============================================
  Files           237      237              
  Lines          7366     7391      +25     
  Branches        388      389       +1     
============================================
+ Hits           6264     6287      +23     
- Misses          871      872       +1     
- Partials        231      232       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bitwiseman]

@Anonycoders Great catch here!

The comments on the methods and the test showing working behavior are good.

However, your comments also make it clear this is a behavior is truly bizarre and likely to cause confusion downstream (even with your comments).

I'd like you to consider a couple other options:

Option 1: Return classes that match the records

• Add a new class GHPullRequestReview#CommentListItem that contains only the fields present on the records returned by GHPullRequestReview#listReviewComments()
• Add a method to GHPullRequestReview#CommentListItem named pubilc GHPullRequestReviewComment readPullRequestReviewComment() that fetches the full GHPullRequestReviewComment
• Change GHPullRequestReview#listReviewComments() return PagedIterable<GHPullRequestReview#CommentListItem>
  ** This would make it instantly clear that the endpoint is returning limited information
  ** This is a breaking change, but 2.0 is still not final. We'd need to do an added step or two to merge the change.
• Keep your comments with some modification, people will still need them

Option 2: Auto-refresh incomplete records, hiding the bad behavior

• add a field (internal) bool incomplete; to GHPullRequestReviewComment.
• add a method private void refreshIncomplete() { if (incomplete) { refresh(); incomplete = false; }
• add call to refreshIncomplete() to getters for all fields not returned as part of GHPullRequestReview#listReviewComments()
  ** Pro: Non-breaking change and quietly works around confusing behavior
  ** Con: Adds stealth REST API calls, allows users to implement potentially expensive/suboptimal behavior without being aware of the expense
• Keep your comments with some modification, people will still need them

I think I'd prefer Option 1, it's clear and makes expensive behavior visible.

Here's the GHPullRequestReview#CommentListItem records:
https://github.com/Anonycoders/github-api/blob/f1983f3b51849ad707d5c35ad12b3b9719df91e7/src/test/resources/org/kohsuke/github/GHPullRequestTest/wiremock/pullRequestReviews/__files/9-r_h_g_pulls_482_reviews_2121304234_comments.json

And here's the full GHPullRequestReviewComment:
https://github.com/Anonycoders/github-api/blob/f1983f3b51849ad707d5c35ad12b3b9719df91e7/src/test/resources/org/kohsuke/github/GHPullRequestTest/wiremock/pullRequestReviews/__files/13-r_h_g_pulls_comments_1641771497.json

[Anonycoders]

3-r_h_g_pulls_comments_1641771497.json

I'd also prefer option No. 1. let me see what I can do about it and shape the PR(s).

[Anonycoders]

@bitwiseman I pushed the revised version; let me know what you think about it.