Should these rules be mandatory or is it just to decide a code style?

[davide.angelillo]

Hi, my company is having an hard time deciding which rules should be strictly followed, so I wanted to askif these specific rules are meant to apport actual benefit or is just code-style.
Remember this is for Angular/React/Typescript project, strictly following documentation code

Remove parentheses around the parameter of this arrow function. → in Typescript, I’m used to always have parentheses even in the rarest case where i cannot type a variable, for consistency. Wouldn’t it be more consistent not to remove them?

Ternary operator user → I just don’t get this. Totally forbidding ternary operator is not a good option in my opinion. Does this apport actual benefit? Of course other rules like “no nested ternary operator” are good

Unexpected any. Specify a different type. → Even though I totally understand this rule, there are some times where you can’t use generics but still need to accept any type of data. Should this just be a warning?

Interactive elements should not be assigned non-interactive roles and other accessibility rules → This is a correct guideline, but because accessibility is most human test, shouldn’t this just be a warning? I often get these kind of errors even on totally accessible pages (other tools and human tests confirm that)

Use either a literal or “Array.from()” instead of the “Array” constructor. → there are some times when this does not give the same result. So it might result in a false positive

Explicitly export the specific member needed → this is not what the barrel development pattern wants, also it is often referred to files with only one export (like components), so why is this rule so important (marked as high)

Type string trivially inferred from a string literal, remove type annotation → Since when having “too much types” is a problem? Wouldn’t this rule create less readable code?

Forbidden non-null assertion → Shouldn’t this be a unit test concern? Not always this error should be blocking (some times we delegate these checks to functions on top of the function’s body, so that would never happen

Add a “default” clause to this “switch” statement → What if the default shouldn’t to nothing?

Unexpected console statement. → not always a good idea, am i wrong?

These are some rules that I do not understand quite well. My company is trying to decide if either remove them or not from our sonar scans, so what’s your opinion on that? Should they be just warnings, or are there some of these rules that should remain mandatory?

[gab (Gabriel Vivas)]

Hi Davide,

Welcome to the Sonar Community.

Our general recommendation is to use the rules in the Sonar Way profile. We have carefully picked them based on decades long industry experience and static analysis expertise.

To avoid dealing with too many issues being raised, we recommend focusing on code that is being added and modified, and applying the highest standard possible to that code. We believe this gives the most benefit both in the short term and in the long term. Sonar helps you do this with New Code definition and Quality Gates on New Code.

It would be helpful to have the rule keys (e.g., S100) for the specific rules you mentioned. Usually, the rule descriptions provide good insight into why they matter.

That being said, here’s a quick take on the rules you mentioned:

Remove parentheses around the parameter of this arrow function

Indeed this can be seen as mostly style. Particularly if you have auto-formatting tools, you can disable this one.

Ternary operator use

We discourage the use of nested ternaries, but even single ternaries are avoided by some teams. Your choice.

Unexpected any. Specify a different type

This is a good practice to prevent issues from spreading in your codebase. You can “Accept” individual issues if you need to, and Sonar will help you keep track of them.

Interactive elements should not be assigned non-interactive roles

These are best practices for accessibility. It is up to you whether you choose to ignore them and let them accumulate.

Use either a literal or “Array.from()” instead of the “Array” constructor

The Array constructor should be avoided because of its complex and sometimes unintuitive behavior. It is a common source of bugs. Array.from covers most use cases since it was made for this purpose. Feel free to share specific examples if you have doubts.

Explicitly export the specific member needed

Barrel files are super convenient but also very problematic, see this article: https://laniewski.me/blog/pitfalls-of-barrel-files-in-javascript-modules/

Type string trivially inferred from a string literal, remove type annotation

The annotation does not provide more information. Feel free to disable this rule if you disagree.

Forbidden non-null assertion

We believe it is better to shift left. Fixing this structurally when coding is faster and less expensive than fixing it when it breaks, or compensating by adding tests.

Add a “default” clause to this “switch” statement

Being explicit is better in this case and can save your from common bugs.

Unexpected console statement

This is a good practice since you should be using a logging system and devs sometimes add these while working and forget to remove them.