GitHub annotations not working. Strange filter in plugin code with annotations <= 50

[OneCyrus]

I can't get GitHub annotations to work at all. I can't even see GraphQL-requests relating to annotations with debug-logs. So I started to go through the codebase.

one thing which stands out is this early backing-out in the reportRemainingAnnotations method. This basically skips all annotation reporting when there are not more than 50 annotations to report. So i tried to find a second location in the code where the first 50 annotations could be pushed but wasn't able to find something. Either this code is perfectly hidden or the current implementation will expect more than 50 annotations and never reports to remaining 50 annotations.

https://github.com/mc1arke/sonarqube-community-branch-plugin/blob/895a89d75176c40f95bf5c14fd238d5d3f5761b9/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/github/v4/GraphqlGithubClient.java#L256C9-L258C10

all i could find is this batching PR from a long time ago (#167). So i think the intention was to handle > 50 but it introduced a bug to handle the last 50 annotations.

[OneCyrus]

@mc1arke i guess the intention was to gate recursive call instead of backing out early.

something like this:

       ...

       executeRequest((r, t) -> graphqlProvider.createGraphQLTemplate().mutate(r, t), graphQLRequestEntity, CreateCheckRun.class);

        if (outstandingIssues.size() > 50) {
           reportRemainingIssues(issues, checkRunId, repositoryInputArguments, outputObjectBuilder, graphQLRequestEntityBuilder);
        }

after reading even deeper into the code i think this whole method is completely broken, isn't it? I'm no Java dev but to me it looks like sublist is returning the remaining items and not the first 50 annotations which should be posted. but the code uses this list as the 50 items to annotate.

this here seems to be the wrong part of the splitted list:

https://github.com/mc1arke/sonarqube-community-branch-plugin/blob/895a89d75176c40f95bf5c14fd238d5d3f5761b9/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/github/v4/GraphqlGithubClient.java#L260-L264

[mc1arke]

The commit message on the PR you've mentioned does explain the intended setup pretty well and has test that generate 120 issues then checks that 120 issues were created for Github reporting, so I don't think that's the issue here. I'd certainly expect you to see at least 50 issues since they're reported as part of the initial batch of annotations with the initial CreateCheckRun request.

Can you confirm what you are seeing? Is the check run being shown with no annotations, or just the summary comment, or even only 50 annotations. Can you also confirm what setup you're running (Sonarqube version, plugin version, Github version etc)

[OneCyrus]

the actual issue is/was that only the summary is shown. no actual check with annotations. but you are correct that the first 50 annotations are in the create check method. this investigation lead to the conclusion that the create check graphql request didn't post to the correct commit SHA value. not sure how the correct SHA is identified. But I can override the value with a sonar scanner argument and it started to create checks and annotations. so it looks like there is either a bug with the sonar scanner not reading the proper commit sha or something else is strange.

but the annotation code still looks strange to me. not sure why it passes the same annotations list to createAnnotations and the recursive reportRemainingAnnotations.

List<Annotation> annotations = outstandingAnnotations.subList(50, outstandingAnnotations.size());
                      ^^

InputObject<Object> outputObject = outputObjectBuilder
                .put("annotations", createAnnotations(annotations))
                          ^^
                .build();

reportRemainingAnnotations(annotations, checkRunId, repositoryInputArguments, outputObjectBuilder, graphQLRequestEntityBuilder);
                               ^^

[mc1arke]

The hash is picked up form the git metadata on your local filesystem. It's worth checking if your build system is performing anything like merging or alteration of the commit as part of the checkout.

I honestly can't remember why I wrote the recursion in the way I did rather than dropping the first 50 items from the list before the first call and then checking the list wasn't empty on each entry to the method. We were all young and foolish once!