SonarQube are marking the wrong line in our Azure DevOps Pullrequest (PR)

SonarQube Server / Community Build
, , ,
1

Must-share information (formatted with Markdown):

  • which versions are you using: SonarQube Enterprise 10.5.1
  • how is SonarQube deployed: zip
  • what are you trying to achieve: Pull Request comment from SonarQube on the wrong linenumber in Azure DevOps Pullrequest (C++/C code), but not in SonarQube where the comment is on the correct line
  • what have you tried so far to achieve this: Configured the SonarQube Project to comment on Azure DevOps Pullrequests. we have also tried re-executing the SonarQube analysis, but are unsure if this is a coverage.xml problem or other problem. Do note that there is a lot of changes in this PR and also lots and lots of comments. From experience this might be an Azure DevOps problem - I personally think its an cosmetical problem, but I understand that our developers find it annoying. Any tips here are welcome.

Azure DevOps PR - there is a marking of line 216:
image

SonarQube Project - there is a marking of line 216
image

SonarQube Project - there is a warning, not sure its correct, but may be the reason if the coverage.xml is the problem. we are not sure

image

We beleive line 216 is the correct line.

2

Hey there.

What I first find striking is that the PR comment is from August 21st. Has SonarQube analysis run on the most recent commit of the PR?

What I’m thinking is:

  • Analysis was executed, the PR was decorated
  • The file got updated again, and naturally line numbers changed in Azure DevOps representation of the PR
  • Analysis hasn’t be executed since the file was updated

Another way to ask this is: Does the file’s current state in the PR (outside the PR view, just in the repo view) match what is shown in SonarCloud?

3

Allright, I think our guys have figured out why this issue arises.

  • The problem arises because: SonarQube makes the analysis on the merge-commit that Azure DevOps creates, but Azure DevOps doesn’t show the changes from that in the pull-request.

  • SonarQube GUI shows the code as it looks in the merge-commit. Thats why the comment looks ok in SonarQube.

So, the conclusion is; if there are changes on the main-branch, on top of the pull-request, then SonarQube will miscalculate the line numbers sligtly (in the PR), depending on the amount of added or removed lines of code.

(Hopefully I’ve translated this correctly - I hope that make sense. :slight_smile: )

Is this something that could be fixed in a future version or is it by design?

5

Thanks for looking deeper into it @jensmadsen! If that’s the case, it sure seems we need to look into it a bit closer… I’ve flagged this for attention.

1 Like
8

Hi Collin

Thankyou for this. Is there any SonarSource case number or link that we can refer to or have a look at?

Best regards
Jens Madsen

9

Hello Jens,

No, not yet.

I flagged your topic internally for a deeper review.

Our reasoning so far is:

I’m afraid we won’t be able to do much indeed.

Essentially you are analyzing a different version of the code (post merge) than you are decorating on Azure (pre merge). If the PR is recently rebased from master, it shouldn’t cause any problem because changes from the PR are the only one affecting the post merge version (ie. the PR commit is the only difference from master). I’m assuming that here it’s not the case.

We will check internally, but in the meantime I’d simply say avoid doing that.

Out of curiosity:

Why Azure makes a merge-commit before running the pipeline, can’t you simply checkout the PR commit and run the pipeline on it? It seems odd to me to do this, but I might be missing the benefits.

Cheers

1 Like
10

I’m not entirely sure if this merge-commit correctly described by me, I think your explanation with pre-merge and post-merge is a better explanation of whats happening.

But yes, SonarQube analyses the branch/merge, I think thats correctly described, and also by design I think. It is analyzing the pull-request.

Allright, I’m not exactly sure how to move on from here, as I cant force our developers to work on only one branch at a time. I’m going to check with our developers if this is a satisfactory solution.

Thanks everyone for your efforts so far.

11

Indeed, of course there will be multiple branches targeting master at any single time. You can’t solve that.
However, and this is usually what happens: your pipeline could be configured to analyze the PR commit directly (ie. pre-merge), not a post-merge or what we could call a “local merge”. Hence you’re ensuring that the code analyzed is exactly the code being decorated (literally the same commit) and you have no code discrepancy.

It is a general good practises to do it. Check with your developers as they might have a very good reason to run the pipeline on the local merge, but if not then I would just avoid it.

Cheers,
Antoine

12

I assume this is a bit much to ask: I assume this is a configuration problem then?

  • Is there any configuration article (Azure DevOps) that you can recommend, which will solve this?
  • I assume you may have done this with other customers?
13

To be honest, I don’t know much about it. I can say that Azure pipelines do that by default (I could see it in our own repos).

I think the simplest approach is to rebase the PR and relaunch the pipeline in those cases, I assume that’s what most people do.
About editing the pipeline so it doesn’t do this merge commit, I don’t know, I searched a bit but it doesn’t look trivial like just a flag to use.