For unrelated reasons, we decided that S1172 will no longer be included in the “Sonar Way” profile. Starting from the next update coming in the following weeks to SonarCloud, to SonarQube 9.6, and to SonarLint upcoming release. This means most users will stop seeing this false positive.
If you are using TypeScript, you could benefit from TS6133, which is a check built-in to the TypeScript compiler. This will avoid any false positives, but you might need to manually enable it.
Thanks for your reply. Still want to give some replies to this.
Can you perhaps provide a bit more information about why it would be removed from the profile? To me it seems like a perfectly legitimate rule to be honest. The fact that it can be done with a TypeScript configuration option doesn’t say that projects will actually do that obviously. If you use SonarCloud as a “let’s-make-sure-that-the-quality-of-the-projects-in-our-organization-at-least-adheres-to-X” kind of tool, it’s nice to have these kinds of rules in I would say? Otherwise (for TypeScript specifically) you’d need some global check in an org to figure out if everybody is using noUnusedParameters in their TS configuration?
For a few TypeScript options I kind of dislike having them in my configuration while developing. Let’s say I’m creating some Cucumber test (first thing that comes to mind). Not sure how familiar you are with the framework, but if I will just write some initial “step”, the framework will generate some code for me to actually implement. I.e. something like:
It’s really annoying if while running the tests with something like ts-node that will start barking at you because you have not used the parameter amountOfCookiesyet. Same goes for something like noUnusedLocals. If I comment out some code temporarily because it will help me debug an issue faster somehow, but then I run the tests (which invoke tsc) and it starts barking at me because now I’m not using some local variable somewhere that is annoying. However I definitely don’t want to end up in production with these issues though. Running SonarCloud in CI to prevent those kind of errors I still find useful.
As a more generic remark about the above: I can fix it probably with a CI specific tsconfig.json, but I like the fact that these rules are noted by SonarQube, giving me proper feedback on my pull request, rather than having a build step first fail on compilation (because of that flag in the CI specific configuration). git commit --amend; git push that, and only then running SonarQube which would give me another set of errors.
I’m not sure if you had a look at my PR, but it seems the fix is (seems?) really quite trivial. There already is an exception for a (TypeScript) ParameterProperty, the implementation was just a bit off for parameter properties that had a default value assigned as well. If you keep the rule, i.e. don’t deprecate it, don’t you still want to have this fix in?
Thanks for sharing your workflow and compromises, I can relate.
Sorry I was not clear, the reasoning is not about this particular FP, it is about the ecosystem not settling on renaming parameters with underscores when subsequent parameters are being used. We were considering accommodating that case.
However, instead of diluting the rule, we chose to make it opt-in by default, since some users do like its current behaviour.