SonarQube does not find new bugs in PRs, until they are merged to master

  • SonarQube Developer Edition, version 9.6.1 (build 59531)
  • SonarScanner 4.7.0.2747
  • Default New Code behaviour - Previous version
  • Sonar scanner options for PRs are:
  - sonar.pullrequest.base=master
  - sonar.pullrequest.branch=DEV-some-branch
  - sonar.pullrequest.key=some-key

Hi,

We faced the issue when SonarQube for PRs found no new bugs in code and was merged to master, but after this merge the nightly job on master found a new bug in code and failed by the Quality Gate.
That bug was related to code that was changed in 11/10/2020, but was raised as New Code issue in the master branch.


This is a real issue (we use the wrong way to access the static class member). But I don’t see any reason why sonar should complain about it now, this is old tech debt. We changed SikluEthtool::GetPortParameters to be static since the unit tests code was not included in the diff (it was not changed) the Sonar did not find the issue on a PR build. When the master build starts, the full source code was scanned by Sonar and the issue in the unit tests was detected.
The expected behaviour was to fail the PR job before it was merged to master or not to fail the master with new bug in code.
What is a proper solution for avoiding such issues in future?

Hi,

As I understand it, a change in one line caused a new issue to be raised on old code at another line.

It’s normal and expected that the new-old issue wasn’t raised in PR analysis; PR analysis only raises issues on code changed in the PR.

And you’re not the only one to complain about this situation. I’ll pass this on to the Product Managers.

 
Ann

1 Like

Hi Ann,

The issue has occurred again with the updated SonarQube to Version 9.7 (build 61563). The scenario was the same, merged PR with no new bugs in code and then failed nightly build job on master.
The “New Bug” in master branch appeared in lines that were not changed, only lines number differs (See screenshots above)
sonarNewBug

We need a working solution how to avoid such behaviour for our production environment. Is it possible to perform full code scan on PR jobs? Maybe, there is a workaround how we could stay within PR scans, and continue to benefit from its features, but to perform full scan before merging to master?

Hi,

The workaround would be to analyze the PR’s underlying branch. You don’t get the benefit of PR decoration, but you would get a “whole code” analysis.

 
Ann

Thank you for a quick response,

as I understood from your answer, the only available solution to perform a full code review is fall back to branch analysis? Will Sonar team prepare a feature update in future versions to fix such unwanted behaviour?

Hi,

We’ve talked in the past about raising more issues in PR analysis, but it hasn’t bubbled to the top of the list. I’ve pinged the Product Managers on the topic, though.

 
Ann

Hi,

Is there any recommended workflow for PRs to avoid such issues besides full code analysis?

Hi,

I’ve already listed the options I’m aware of.

 
Ann

Hi Ann,

I saw several issues like this that were reported by different people. For example SonarQube feature branch checks for new code does not show failures but it does on master - #6 by ganncamp

May I ask you what is the conclusion here?

Hi Dmitry,

We consider addressing this need in the future and have created a portal card here where other users can share their needs. You can find the link to it here.
If we move forward with this we will update the card and if you left your email you will receive a notification.

Chris

1 Like

Hi Chris,

This is definitely a bug in SonarQube. Why you are considering it as a new feature?
This bug causes instability in our CI/CD system. As a temporary solution, can you propose us a workaround or something like that?

Thanks,
Dmitry

1 Like

Hi Dmitry,

As explained by @ganncamp, this is currently the normal behavior for PRs: PR analysis currently raises issues on changed code only.
The main workaround I can suggest is to analyze your feature branch if you want to be sure to catch these issues before the merge.

We made big improvements recently to speed up the PR analysis, analyzing only the code that has changed. Out of curiosity, would you be ready to sacrifice feedback time on your PR for stricter checks?

Chris

Hi Chris,

As explained by ganncamp, this is currently the normal behavior for PRs: PR analysis currently raises issues on changed code only.

If, for example, in PR someone changes the type of some variable that is accessible in many places in the code, it may lead to additional issues in the code that were not changed by PR.

The main workaround I can suggest is to analyze your feature branch if you want to be sure to catch these issues before the merge.

Yes, this is what we do, but it leads to additional problems. For example, if an issue is detected by Sonar during scanning the feature branch and if the SW engineer will mark this issue as a false positive, after the PR merge the same false positive issue will not be marked in the main branch and next Sonar scan of the main branch will fail.

We made big improvements recently to speed up the PR analysis, analyzing only the code that has changed. Out of curiosity, would you be ready to sacrifice feedback time on your PR for stricter checks?

For us, the time required for PR analysis is not critical at all. Our CI/CD system runs tests at the same time as Sonar analysis PR.
If you can add a flag that will force Sonar to do a full scan of the entire codebase during PR analysis (instead of analyzing only changed code), it will be a great solution for us.
We will much appreciate it if you will be able to add such a flag.

Thanks,
Dmitry