Questions from our Development Team

Below is a question on of our Dev Teams has about how SonarQube is interpreting a code change.

Yes! Here is an example that Aaron collected for us of the difficulty we are encountering:

Take this PR as an example – only one line was changed. Aaron changed the url from “/checkout/cart” to “/checkout/cart/test”.

Sonarqube failed this PR, indicating that the cognitive complexity of the method is too large (56, when it should be 15). Aaron’s PR did not change the cognitive complexity of the method at all; he just changed a URL, but now he is responsible for rewriting the surrounding code.

Our question is: is there a way to have Sonarqube only apply rules to the new code written by our developers? Or will we have to rework pre-existing code with every small change? I can think of arguments for both ways, but the fact of the matter is when we begin this initiative we agreed to only be responsible for new code, not rewrite old code, to avoid potentially blowing our estimates out of the water (we won’t know ahead of time which code needs to change - and what Sonarqube would complain about - until we have completed the work).

For the time being, we have disabled the rules which cause this to happen (generally things that measure method/class statistics) – We don’t intend to re-enable them without understading how we can restrict it to new code only.

Let me know if you have any questions! Thanks,

Hi,

Welcome to the community!

This sounds like a problem with detecting which code is new in the PR and which is not. Could we have

  • a screenshot of the issue in SonarQube
  • the log of the PR’s analysis

both redacted as necessary, of course.

The analysis / scanner log is what’s output from the analysis command. Hopefully, the log you provide - redacted as necessary - will include that command as well.

This guide will help you find them.

 
Ann

Yes Ann.

Here is further definition below…

Yes! Here is an example that Aaron collected for us of the difficulty we are encountering:

Take this PR as an example – only one line was changed. Aaron changed the url from “/checkout/cart” to “/checkout/cart/test”.

[ Screenshot redacted: Pull request showing a change to one if statement. ]

Sonarqube failed this PR, indicating that the cognitive complexity of the method is too large (56, when it should be 15). Aaron’s PR did not change the cognitive complexity of the method at all; he just changed a URL, but now he is responsible for rewriting the surrounding code.

[ Screenshot reacted: One Cognitive Complexity issue shown in the Issues list. Issue message: Refactor this function to reduce its Cognitive Complexity from 56 to the 15 allowed ]

Our question is: is there a way to have Sonarqube only apply rules to the new code written by our developers? Or will we have to rework pre-existing code with every small change? I can think of arguments for both ways, but the fact of the matter is when we begin this initiative we agreed to only be responsible for new code, not rewrite old code, to avoid potentially blowing our estimates out of the water (we won’t know ahead of time which code needs to change - and what Sonarqube would complain about - until we have completed the work).

For the time being, we have disabled the rules which cause this to happen (generally things that measure method/class statistics) – We don’t intend to re-enable them without understading how we can restrict it to new code only.

Motion_Logo_NEW_100x12_5258cfde-0b2d-43b7-af8f-13662c8d11d4.png

Hi,

Could I have a screenshot of the issue in the context of the code? I wanted to see what lines were marked new.

And again, could you post the analysis log?

 
Thx,
Ann

Could someone please get this information for Anna?

Take this PR as an example – only one line was changed. Aaron changed the url from “/checkout/cart” to “/checkout/cart/test”.

[ Screenshot redacted: Pull request showing a change to one if statement. ]

Sonarqube failed this PR, indicating that the cognitive complexity of the method is too large (56, when it should be 15). Aaron’s PR did not change the cognitive complexity of the method at all; he just changed a URL, but now he is responsible for rewriting the surrounding code.

[ Screenshot reacted: One Cognitive Complexity issue shown in the Issues list. Issue message: Refactor this function to reduce its Cognitive Complexity from 56 to the 15 allowed ]

Our question is: is there a way to have Sonarqube only apply rules to the new code written by our developers? Or will we have to rework pre-existing code with every small change? I can think of arguments for both ways, but the fact of the matter is when we begin this initiative we agreed to only be responsible for new code, not rewrite old code, to avoid potentially blowing our estimates out of the water (we won’t know ahead of time which code needs to change - and what Sonarqube would complain about - until we have completed the work).

For the time being, we have disabled the rules which cause this to happen (generally things that measure method/class statistics) – We don’t intend to re-enable them without understading how we can restrict it to new code only.

Motion_Logo_NEW_100x12_5258cfde-0b2d-43b7-af8f-13662c8d11d4.png

Hi,

It’s not clear to me whether you intended to reply to the thread, but again:

Which is why I’m asking for a screenshot of the issue in the context of the code, so I can see which lines are marked new, and for the analysis log.

 
Ann

Hi Ann.

Yes, I am trying to get you the information you are requesting.

I am not in possession of what you need which is why I am reaching out to other Team members to get it for you.

Thanks,
Rick

Motion_Logo_NEW_100x12_5258cfde-0b2d-43b7-af8f-13662c8d11d4.png

Hi,

Having received your logs and screenshot privately, I can say that your log looks normal. It’s the screenshot that tells the story. What’s marked ‘new’ in it is an if statement. It looks like your developer edited this line, which marked it new.

That line would have been previously included in the secondary locations from the (old) Cognitive Complexity issue, but now some part of that issue was “new”, thus the whole issue moved into the “new” realm. The idea with Clean as You Code - which all the “New Code” stuff was built to enable - is that you clean as you go along. So presumably, rather than editing the existing mess but leaving the mess in place, your developer should have cleaned up that little part of the mess.

Now, given the probable scope of the change (I’m going to guess the if statement was already in place and it was just the condition that was altered) I can see why that - reasonably - didn’t happen.

But that’s what’s going on.

 
HTH,
Ann

Hi again,

I understand - again through backchannels - that you’d like your screenshots redacted from this thread.

I suppose that means my explanation makes sense?

 
Ann

That is correct, Ann. We want to redact the sensitive information/

Can you please do that Aaron or do you want me to.

Motion_Logo_NEW_100x12_5258cfde-0b2d-43b7-af8f-13662c8d11d4.png

Hi,

I’ve already removed your screenshots…?

 
Ann

Awesome, and thank you Ann. :blush:

Motion_Logo_NEW_100x12_5258cfde-0b2d-43b7-af8f-13662c8d11d4.png