Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full Sonar analysis #847

Merged
merged 18 commits into from
Sep 25, 2023
Merged

Full Sonar analysis #847

merged 18 commits into from
Sep 25, 2023

Conversation

So-Fras
Copy link
Member

@So-Fras So-Fras commented Sep 18, 2023

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines

What kind of change does this PR introduce?
Quality

What is the current behavior?
An old Sonar profile is used, it is not up-to-date.

What is the new behavior (if this is a feature change)?
An up-to-date Sonar profile is used in powsybl-open-loadflow CI. New code smells are reported and should therefore be corrected.

olperr1 and others added 5 commits August 24, 2023 15:44
Signed-off-by: Olivier Perrin <olivier.perrin@rte-france.com>
Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>
Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>
… case

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>
Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>
@So-Fras So-Fras requested a review from annetill September 18, 2023 11:00
@So-Fras So-Fras force-pushed the full-sonar-analysis-2023-08 branch from 09fbd37 to 79364bd Compare September 18, 2023 12:21
@So-Fras So-Fras requested a review from geofjamg September 20, 2023 14:18
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill annetill changed the title Full sonar analysis 2023 08 Full Sonar analysis Sep 21, 2023
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill
Copy link
Member

@geofjamg this is a proposal for making switch more lisible when needed (maybe not every time, as done in the PR). Just tell me what do think. Can we change the automatic indentation for mutliple case in a switch? Furthermore, I have an issue with default that are not covered when we switch on enum. Maybe I have to remove default in that case...

@geofjamg
Copy link
Member

@geofjamg this is a proposal for making switch more lisible when needed (maybe not every time, as done in the PR). Just tell me what do think. Can we change the automatic indentation for mutliple case in a switch? Furthermore, I have an issue with default that are not covered when we switch on enum. Maybe I have to remove default in that case...

When new switch is used as an expression switch (so when we get the returned value from the switch), if not all cases are handled we get a compile time error. So we should in such a case safely remove the default case.
BUT be careful that this is not true, even with new switch syntax when it is used a switch statement instead of a switch expression.

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
done = true;
}

if (List.of("v", "angle", "p", "q", "p1", "q1", "p2", "q2").contains(attribute)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change? this is a sequential search worst in term of performance a a switch/case

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sonar asks for an if loop here instead of a switch case. But I understand if we revert.

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
@sonarcloud
Copy link

sonarcloud bot commented Sep 25, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

88.8% 88.8% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@geofjamg geofjamg merged commit 2f4d33f into main Sep 25, 2023
8 of 9 checks passed
@geofjamg geofjamg deleted the full-sonar-analysis-2023-08 branch September 25, 2023 14:26
geofjamg pushed a commit that referenced this pull request Sep 27, 2023
Signed-off-by: Olivier Perrin <olivier.perrin@rte-france.com>
Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants