PR scan ignores violation of: Functions should not have too many parameters (typescript:S107)

  • SonarQube Server: Data Center Edition v10.6 (92116)
  • how is SonarQube deployed: Not sure

I ran into an interesting bug in which a method with 8 parameters does not raise the Functions should not have too many parameters (typescript:S107) issue, when running a PR scan. A full scan will definitely raise the issue.
It took me a while to reproduce this, since it requires a very particular scenario:
target branch:

const foo = async (val1: string, val2: string, val3: string,
                   val4: string, val5: string, val6: string,
): Promise<void> =>
    console.log('params: ', val1, val2, val3, val4, val5, val6, val7, val8);

pr branch:

const foo = async (val1: string, val2: string, val3: string,
                   val4: string, val5: string, val6: string,
                   val7: string, val8: string,
): Promise<void> =>
    console.log('params: ', val1, val2, val3, val4, val5, val6, val7, val8);

So basically we’re adding params 7 and 8 (and in doing so violating typescript:S107), but the issue is not raised in the PR scan.
Adding the new params in an already existing line causes the issue to be raised as expected:

const foo = async (val1: string, val2: string, val3: string, val4: string, val5: string, val6: string, val7: string, val8: string): Promise<void> =>
    console.log('params: ', val1, val2, val3, val4, val5, val6, val7, val8);

Hi,

You’re right; this is a very particular scenario.

Issues are only reported on changed lines. In this case, the issue would be reported on the variable/function declaration, const foo .... And since that line isn’t changed in the PR, any issues found there are repressed in the PR analysis reporting.

This is a known issue, and not an easy one which is why we haven’t tackled it.

 
:frowning:
Ann

Thanks for getting back to me Ann.
This issue then, along with unused import, field or method is another one that gets missed in PR scans. I know that it’s not an easy bug to fix, but do you have any idea as to when this may be resolved?
In the meantime, can we disable the diff scan in PRs and run a full scan? I seem to remember running into some flag that controls this behavior.

Hi,

Sorry, no clue. As far as I know, tackling it is not on any short-term roadmaps. And I’ve flagged this for the PMs so they can count “traction”.

There’s no flag for this. If you want the full analysis, you’ll need to analyze the PR’s underlying branch.

 
Ann

Thanks Ann,
What about sonar.java.skipUnchanged, mentioned here?

Hi,

I believe all that will do is increase your analysis time. PR analysis results are still filtered to only reflect the changed files.

 
Ann

1 Like