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

New rule S2198: Silly mathematical comparisons should not be made #6658

Closed
cristian-ambrosini-sonarsource opened this issue Jan 23, 2023 · 3 comments · Fixed by #6695
Closed
Assignees
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: New Feature This hasn't been here before.
Projects
Milestone

Comments

@cristian-ambrosini-sonarsource
Copy link
Contributor

cristian-ambrosini-sonarsource commented Jan 23, 2023

S2198
RSPEC PR: SonarSource/rspec#1534

@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource added Area: VB.NET VB.NET rules related issues. Area: C# C# rules related issues. labels Jan 23, 2023
@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource added this to the 8.52 milestone Jan 23, 2023
@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource added the Type: New Feature This hasn't been here before. label Jan 23, 2023
@gregory-paidis-sonarsource
Copy link
Contributor

Maybe this could be a part of S2437.

Or even better, the other way around, we could merge the rules and have different diagnostics.

@gregory-paidis-sonarsource
Copy link
Contributor

Looking a bit at what this rule covers, as well as some testing:

  • comparing a byte with an int or long constant that’s outside the byte range

This is covered by CS0652 for all types of of comparisons (<, <=, >, >=, ==)

  • comparing an int with a long constant that’s outside the int range

This is covered by CS0652 for all types of of comparisons (<, <=, >, >=, ==)

  • comparing a value guaranteed to be negative with a one that’s guaranteed to be non-negative

This is covered by CS0652 for comparing a uint with a constant negative int. I cannot think of any other usecases.

  • comparing aByte <= Byte.MAX_VALUE and aByte >= Byte.MIN_VALUE

Not covered.

  • comparing anInt <= Integer.MAX_VALUE and anInt >= Integer.MIN_VALUE

Not covered.

  • comparing aLong <= Long.MAX_VALUE and aLong >= Long.MIN_VALLUE

Not covered.

  • Additional notes

Maybe we can implement a comparison between float and constant double that is > float.MaxValue.

@gregory-paidis-sonarsource
Copy link
Contributor

We decided to implement only the parts that are NOT covered by CS0652, meaning:

  • comparing aByte <= byte.MaxValue and aByte >= byte.MinValue
  • comparing anInt <= int.MaxValue and anInt >= int.MinValue
  • comparing aLong <= long.MaxValue and aLong >= long.MinValue
  • comparing a float with a constant double outside of float's range

Also, to cut branches we will only implement the C# part of the rule, at least at first.

Also, it will indeed be part of S2437.

@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource moved this from To do to In progress in Best Kanban Feb 2, 2023
@github-actions github-actions bot moved this from In progress to Review in progress in Best Kanban Feb 3, 2023
@github-actions github-actions bot moved this from Review in progress to In progress in Best Kanban Feb 3, 2023
@github-actions github-actions bot moved this from In progress to Review in progress in Best Kanban Feb 6, 2023
@github-actions github-actions bot moved this from Review in progress to In progress in Best Kanban Feb 7, 2023
@github-actions github-actions bot moved this from In progress to Review in progress in Best Kanban Feb 7, 2023
@github-actions github-actions bot moved this from Review in progress to In progress in Best Kanban Feb 7, 2023
@github-actions github-actions bot moved this from In progress to Review in progress in Best Kanban Feb 7, 2023
@github-actions github-actions bot moved this from Review in progress to In progress in Best Kanban Feb 7, 2023
@github-actions github-actions bot moved this from In progress to Review in progress in Best Kanban Feb 8, 2023
@github-actions github-actions bot moved this from Review in progress to In progress in Best Kanban Feb 9, 2023
@github-actions github-actions bot moved this from In progress to Review in progress in Best Kanban Feb 9, 2023
@github-actions github-actions bot moved this from Review in progress to Review approved in Best Kanban Feb 10, 2023
Best Kanban automation moved this from Review approved to Validate Peach Feb 10, 2023
@gregory-paidis-sonarsource gregory-paidis-sonarsource moved this from Validate Peach to Done in Best Kanban Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: New Feature This hasn't been here before.
Projects
Best Kanban
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants