I’d like to suggest make the rule S2360 “Optional parameters should not be used” more configurable (in a similar way like, for example, S134 " Control flow statements should not be nested too deeply"). Instead of fully prohibiting usage of optional parameters it should allow users to set their own max allowed number.
The rule description explicitly mentions 3 points behind this rule:
Optional parameter values are baked into the method call site code, thus, if a default value has been changed, all referencing assemblies need to be rebuilt, otherwise the original values will be used.
The Common Language Specification (CLS) allows compilers to ignore default parameter values, and thus require the caller to explicitly specify the values. For example, if you want to consume a method with default argument from another .NET compatible language (for instance C++/CLI), you will have to provide all arguments. When using method overloads, you could achieve similar behavior as default arguments.
Optional parameters prevent muddying the definition of the function contract. Here is a simple example: if there are two optional parameters, when one is defined, is the second one still optional or mandatory?
I think that the point #2 is the one that most unlikely to an average developer (and probably should be triggered only if class is explicitly marked with CLSCompliant attribute). 1st point is more common, but it is relevant only if you’re distributing resulting library and consume it from another project as dll. In our case, we’re developing large solution that consists of several dozens of projects that are built together, therefore making this point irrelevant to us.
In the end, in our case only 3rd point stays relevant, and I can argue that having method with 1 optional parameter is fine, it shortens the code and increases readability, while leaving no place to ambiguity at the callsite. However, having two or even more arguments make code less readable at the callsite, especially if optional parameters are of the same time.
So, I’d like to have an ability to enforce allowed number of optional parameters to be <= 1. Default configuration value could stay = 0 for backward compatibility.
Hi @utin.as , thanks for the well described suggestion, we’ll take this for an internal discussion.
Indeed, so if we parameterize the rule by default with 0, this would cover cases where people don’t want them. And it also allows your case, to allow one optional param.
Regarding freedom: for example parameterizing the rule with 5 might be similar to disabling the rule, from a Clean Code perspective (but it’s the same with all the other parameterized rules, I guess) - do you have any thoughts on that?
Yes, large number as the limit would act as effectively disabling of the rule, with only difference being a small performance penalty since the rule still has to be checked on all methods, even though it’s unlikely to ever be triggered. Probably, allowed range for values of the parameter should be from 0 to the value set in rule “S107:Methods should not have too many parameters”. But IMO, the simpler - the better, and no limits to the allowed parameter range should be set at all. Moreover, looks like there are no limits for parameter values in configurable sonarqube rules right now - for example I can set parameter value of S107 to any value from -999999999 to 999999999
Thanks a lot for your insight on this.
About the request, I opened an issue on our dedicated backlog of the dotnet analyzer, where you can track the progress.
About the -999… to 999… unlimited range on Sonarqube, that is indeed an interesting observation.
I will raise it internally with my team, to see how doable it is to limit the ranges between more sensible values.
Thanks a lot for the suggestions and the conversation!
