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

Mosify rule S2198: Add C# #1534

Merged
merged 6 commits into from Feb 15, 2023
Merged

Conversation

gregory-paidis-sonarsource
Copy link
Contributor

Rule implementation: SonarSource/sonar-dotnet#6658

@sonarsource-next
Copy link

SonarQube Quality Gate for 'rspec-tools'

Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarsource-next
Copy link

SonarQube Quality Gate for 'rspec-frontend'

Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

Comment on lines 3 to 8
These comparisons will always return ``++false++``:

* comparing a ``++char++`` with a numeric constant that's outside the ``++char++`` range
* comparing a ``++float++`` with a numeric constant that's outside the ``++float++`` range
* comparing a ``++long++`` with a numeric constant that's outside the ``++long++`` range
* comparing a ``++ulong++`` with a numeric constant that's outside the ``++ulong++`` range

Choose a reason for hiding this comment

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

Suggested change
These comparisons will always return ``++false++``:
* comparing a ``++char++`` with a numeric constant that's outside the ``++char++`` range
* comparing a ``++float++`` with a numeric constant that's outside the ``++float++`` range
* comparing a ``++long++`` with a numeric constant that's outside the ``++long++`` range
* comparing a ``++ulong++`` with a numeric constant that's outside the ``++ulong++`` range
These comparisons will always return ``++false++``:
* comparing a `char` with a numeric constant that's outside the `char` range
* comparing a `float` with a numeric constant that's outside the `float` range
* comparing a `long` with a numeric constant that's outside the `long` range
* comparing a `ulong` with a numeric constant that's outside the `ulong` range

include::../rule.adoc[]
Certain mathematical comparisons will always return the same value, and should simply not be made.

These comparisons will always return ``++false++``:

Choose a reason for hiding this comment

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

Suggested change
These comparisons will always return ``++false++``:
These comparisons will always return `false`:


These comparisons will always return ``++false++``:

* comparing a `char` with a numeric constant that's outside the `char` range

Choose a reason for hiding this comment

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

The convention is to start with an upper letter and to end with a dot:

Suggested change
* comparing a `char` with a numeric constant that's outside the `char` range
* Comparing a `char` with a numeric constant that's outside the `char` range.

Comment on lines 14 to 16
float f = 42.0f;
const double d = double.MaxValue;
if (f <= d) { } // Noncompliant

Choose a reason for hiding this comment

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

Simpler:

Suggested change
float f = 42.0f;
const double d = double.MaxValue;
if (f <= d) { } // Noncompliant
float f = 42.0f;
if (f <= double.MaxValue) { } // Noncompliant


These comparisons will always return ``++false++``:

* comparing a `char` with a numeric constant that's outside the `char` range

Choose a reason for hiding this comment

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

Maybe better like so?

Suggested change
* comparing a `char` with a numeric constant that's outside the `char` range
* Comparing a `char` with a numeric constant that is outside of the range of `char`.

Copy link
Contributor

Choose a reason for hiding this comment

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

"always false" is not correct.

include::../rule.adoc[]
Certain mathematical comparisons will always return the same value, and should simply not be made.

These comparisons will always return `false`:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
These comparisons will always return `false`:
These comparisons will return either always `true` or always `false` depending on the kind of comparison:

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. Please merge once 🟢 .

@sonarsource-next
Copy link

SonarQube Quality Gate for 'rspec-tools'

Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarsource-next
Copy link

SonarQube Quality Gate for 'rspec-frontend'

Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@gregory-paidis-sonarsource gregory-paidis-sonarsource merged commit 972f4dc into master Feb 15, 2023
@gregory-paidis-sonarsource gregory-paidis-sonarsource deleted the greg/edit-S2198-csharp branch February 15, 2023 14:00
@arseniy-sonar
Copy link
Contributor

Hi @gregory-paidis-sonarsource
Please, follow the guideline for the commit and the PR summary next time.

@arseniy-sonar arseniy-sonar changed the title S2198: Add C# Mosify rule S2198: Add C# Feb 15, 2023
@gregory-paidis-sonarsource
Copy link
Contributor Author

Hi @gregory-paidis-sonarsource
Please, follow the guideline for the commit and the PR summary next time.

Hey @arseniy-sonar , thanks for reminding me, I will follow the procedure from now on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants