Add "includeSwitchBlocks" rule property to S1142 - MethodWithExcessiveReturnsCheck

[arms...@gmail.com]

I would like to contribute to the existing S1142 - MethodWithExcessiveReturnsCheck rule. On my team we like this rule but with one exception -- we would like to ignore return statements inside switch blocks. We feel that

case
  return
case
  return
case
  return

is more readable than

init var
case
  set var
  break
case
  set var
  break
case
  set var
  break
return

I propose we add a "includeSwitchBlocks" rule property to the existing rule, and have it default to true for backwards compatibility.

I've already made the change on my fork. Just creating this post so if there's discussion it can happen in the right place. :)

[arms...@gmail.com]

PR: https://github.com/SonarSource/sonar-java/pull/1904

[arms...@gmail.com]

Hello,

Just checking back as it's been a month since I submitted the pull request with no action. Am I following the process correctly? If somebody could let me know if this can be considered as a SonarJava contribution, I would appreciate it.

Thanks,

Adam

On Friday, February 9, 2018 at 1:35:30 PM UTC-8, arms...@gmail.com wrote:

[arms...@gmail.com]

Hi,

Could someone from the Sonar team please have a look at this? I am interested to know if the contribution can be considered for SonarJava.

Thanks,

Adam

On Friday, February 9, 2018 at 1:35:30 PM UTC-8, arms...@gmail.com wrote:

[alexandr...@gmail.com]

Hi Adam,

First, sorry for the delay to get back to you. 

We try to limit the number of parameter to the minimum on each rule so that when you activate it, you get the best of the rule without having to think twice what is the best configuration possible for the rule.

There is a potential debate about should we count or not the returns of a "switch". We decided to count them because we think it's a problem to have too much returns in a method even if they are part of a switch.

The other option could have been to simply ignore them and count only the other returns.

We don't want to make S1142 more complex and so we will not consider your suggestion. 

I suggest creating a custom rule (https://github.com/SonarSource/sonar-custom-rules-examples/tree/master/java-custom-rules) without the option but directly ignoring the switch and why not publishing it on a GitHub repo, it may help some other Java developers who want to apply the same logic as the one you suggest.

Regards

Alex