EPIC: SonarCloud error reduction

[CDellaGiusta]

Short description

Current status of the overall issues and errors detected by the SonarCloud instance running on the Uyuni project (https://sonarcloud.io/project/overview?id=uyuni-project_uyuni) shows more than 7000 issues/errors.
With this number of issues, the usability of the tool becomes much less effective.
The overall idea is to tackle the problems to reduce them over time at a manageable number, so that any issue due to new code can be immediately spotted.

How to tackle the problem

A possible approach should be "one slice at a time". Here below are some proposed steps after a first analysis:

1. The security hotspots should be tackled as soon as possible. Some of them are trivial and could be solved very easily. Some can even be marked as non-issues.
2. The number of issues per programming language is the following:
   • Java 3900+
   • Python 2800+
   • Typescript 549
   • HTML 42 (all in one old license file)
   • CSS 11 (all of 1 type only )
   • JavaScript 8 (all in 2 files)

It is worth fixing the last 3 languages at the start, and we should focus on tackling the Java issues as first step.

3. There are 11 rules in Java that could be proposed as to be removed, since their adoption could possibly not be feasible or the work to fix them is not worth the effort in terms of being somehow useful.
   These 11 rules should be put under public discussion and a poll for removal should be issued. The rules that receive the majority of votes to be removed, will be deactivated on SonarCloud analysis for Uyuni.
4. The other issues have been classified by rule. Fixing a problem "by rule" allows a simplest fix (one knows how to fix it and does it methodically) and it is simple to review.
5. Some of the fixes are classified as a "good first issue" since the kind of issue to be resolved is very easily fixable, without dig deep into the code meaning or behaviour.

Analysis by rule/removal candidate/good first issue

A spreadsheet with a first analysis of the affected rules, classified by number of issues, rule, removal candidate, good first issue, can be downloaded here: #9881

Polls and discussions on potential removal candidate rules:

1. SonarCloud remove candidate rule: java:S1611 Parentheses should be removed from a single lambda parameter when its type is inferred #9880
2. SonarCloud remove candidate rule: java:S1172 Unused method parameters should be removed #9922
3. SonarCloud remove candidate rule: java:S1066 Mergeable "if" statements should be combined #9924
4. SonarCloud remove candidate rule: java:S1168 Empty arrays and collections should be returned instead of null #9925
5. SonarCloud remove candidate rule: java:S1450 Private fields only used as local variables in methods should become local variables #9926
6. SonarCloud remove candidate rule: java:S2925 Thread.sleep should not be used in tests #9927
7. SonarCloud remove candidate rule: java:S1075 URIs should not be hardcoded #9928
8. SonarCloud remove candidate rule: java:S1602 Lambdas containing only one statement should not nest this statement in a block #9929
9. SonarCloud remove candidate rule: java:S135 Loops should not contain more than a single "break" or "continue" statement #9930
10. SonarCloud remove candidate rule: java:S1643 Strings should not be concatenated using '+' in a loop #9931
11. SonarCloud remove candidate rule: java:S2589 Boolean expressions should not be gratuitous #9932

Issues and PRs fixing SonarCloud errors

#9884 SonarCloud error reduction: HTML issues
#9885 SonarCloud security hotspot fix: using https instead of http
#612 Fix unchecked types warnings

All open pull requests
https://github.com/uyuni-project/uyuni/pulls?q=is%3Aopen+is%3Apr+label%3Asonar-cloud-error-reduction

All open issues
https://github.com/uyuni-project/uyuni/issues?q=is%3Aissue%20state%3Aopen%20label%3Asonar-cloud-error-reduction%20no%3Atype

[cbosdo]

#612 should probably be pointed out from here.

[Etheryte]

I would strongly suggest reviewing the rules for Typescript files before starting any refactoring work based on it. From what I've seen so far, Sonarcloud has a bunch of arbitrary rules that don't align with our code style and the suggestions are not applicable. If possible, we would like to only use security related rules from Sonar, we mostly use ESLint and Prettier to enforce coding style on the frontend.

[CDellaGiusta]

I would strongly suggest reviewing the rules for Typescript files before starting any refactoring work based on it. From what I've seen so far, Sonarcloud has a bunch of arbitrary rules that don't align with our code style and the suggestions are not applicable. If possible, we would like to only use security related rules from Sonar, we mostly use ESLint and Prettier to enforce coding style on the frontend.

Absolutely! I will start considering Java rules at the moment, then we'll tackle Typescript and Python with the same approach: find and discuss what rules should be removed, then fix the others.

[Bhavya031]

I would strongly suggest reviewing the rules for Typescript files before starting any refactoring work based on it. From what I've seen so far, Sonarcloud has a bunch of arbitrary rules that don't align with our code style and the suggestions are not applicable. If possible, we would like to only use security related rules from Sonar, we mostly use ESLint and Prettier to enforce coding style on the frontend.

Absolutely! I will start considering Java rules at the moment, then we'll tackle Typescript and Python with the same approach: find and discuss what rules should be removed, then fix the others.

I already opened two PR about this issue, but I am thinking about completing this issue as soon as possible. can i take the lead in java?

[CDellaGiusta]

I already opened two PR about this issue, but I am thinking about completing this issue as soon as possible. can i take the lead in java?

No, please wait. I have still to complete a lot of stuff there. We must proceed in an orderly manner, avoiding overlapping.
Also, first we have to discuss what rules have to be removed, if any. I'll be having a presentation at the Uyuni community hours as soon as I finish my initial work. Thanks for understanding

[Bhavya031]

I already opened two PR about this issue, but I am thinking about completing this issue as soon as possible. can i take the lead in java?

No, please wait. I have still to complete a lot of stuff there. We must proceed in an orderly manner, avoiding overlapping. Also, first we have to discuss what rules have to be removed, if any. I'll be having a presentation at the Uyuni community hours as soon as I finish my initial work. Thanks for understanding

I think I quoted the wrong comment. I want to quote @cbosdo’s comment about #612, but I will wait until your presentation.

[cbosdo]

I already opened two PR about this issue, but I am thinking about completing this issue as soon as possible. can i take the lead in java?
I think I quoted the wrong comment. I want to quote @cbosdo’s comment about #612, but I will wait until your presentation.

Taking the lead and completely remove them all is a huge task. I would rather recommend that you pick a class with issues and fix them. Once the PR is ready, move to another one. Small bites are the only way to go here. I believe there are other persons interested in this one. Better continue the discussion on the corresponding issue to avoid polluting this general one.