Issues get assigned to wrong user (squid:S00107, squid:S18202)


(Marius Klezok) #1

Hello,

I have a question regarding the rules “squid:S00107” (method has too many parameters) and “squid:S18202” (class has too many fields).
In both cases, when one additional parameter/field is added and then the threshold is exceeded, the rules do not actually look in the line of the added field/parameter.

For example (in the parenthesis is the actual scm info like it is shown in SonarQube):

L35 (comitter1)  public void methodName( param1,
L36 (comitter1)                  param2,
L37 (comitter2)                  param3){
...}

Here commiter1 creates the method with one parameter and commits his code.
Afterwards commiter2 adds another parameter to the method -> exceeded threshold -> SonarQube searches for the assignee in the line of the method declaration instead of where the last added parameter is -> The issue is assigned to commiter1 instead of comitter2.
Is there any way to change that behaviour?

We already tried to customize the rule, so that it reports on the FieldLevel instead of the ClassLevel. But then every field gets marked and we are not sure, which line it would use then to find the assignee.

Thank you,
Marius

Server Version: 6.7.2.37468
SonarJava Version : 5.9.1


(Loïc Joly) #2

Hello Marius,

I think the situation can be even more complex than what you describe. For instance:

L35 (committer1)  public void methodName( param1,
L36 (committer3)                  param3,
L37 (committer2)                  param2){
...}

In this example, the extra parameter is the one added by committer2, but when committer2 added it, it was only the second parameter of the function. Later, committer3 added param3, which is the just the second parameter, and there is no reason whatsoever to highlight it as the issue when analyzing the code.

I think the question boils down to “why would you like to associate the issue with the accurate committer”? I think there are two situations:

  • The commit is very recent, and you want to assign the issue to the guy who is currently working on the subject. For that, if you put in place pull request analysis, you don’t even need to know who did the commit: The pull request is blocked, and the guy in charge of it will have to correct the issue before it can be merged.
  • You a cleaning-up old issues. In that case, does it really matter if the issue is assigned to committer1 instead of committer3? What matters, IMO, is that the commit is assigned to someone who is knowledgeable enough about the code to be able to take the right decisions to correct it. All 3 committers look like good match to me in this situation.

(Marius Klezok) #3

Hi Loic,

thank you for your reply. In our case we integrate the SonarQube Issues via the REST API into our own tool.
Here we only consider the new Issues. What i forgot to mention is, that we use SVN and while I’m not sure,
if there is something like a pull request there, we don’t use it.
So it would be the first case, that you mentioned but we can’t use the pull request.

Greetings