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:
SonarQube Project - there is a marking of line 216
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
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. )
Is this something that could be fixed in a future version or is it by design?
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.
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.
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.
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.
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.