Chore: Code hygiene - warn-numeric-widen #2588
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2588 +/- ##
============================================
+ Coverage 56.12% 59.30% +3.18%
- Complexity 976 1444 +468
============================================
Files 119 146 +27
Lines 11743 13747 +2004
Branches 2251 2353 +102
============================================
+ Hits 6591 8153 +1562
- Misses 4012 4373 +361
- Partials 1140 1221 +81 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| record.add(0, i % 2 == 0) | ||
| record.add(1, i.toByte) | ||
| record.add(2, i.toShort) | ||
| record.add(1, i.toByte.toInt) |
There was a problem hiding this comment.
its probably can be directly casted to int?
There was a problem hiding this comment.
I believe overflow behaviour will be different and lead to unexpected and incorrect results
There was a problem hiding this comment.
Actually I think if the test suite is mean to examine the behaviour when Byte or Short being passed, then we should keep it as it is.
I have reverted some .toInt on test suites for this purpose and add a suppression annotation instead.
@comphead @EmilyMatt
| record.add(0, i % 2 == 0) | ||
| record.add(1, i.toByte) | ||
| record.add(2, i.toShort) | ||
| record.add(1, i.toByte.toInt) |
There was a problem hiding this comment.
same, can be probably converted directly
There was a problem hiding this comment.
see above reply, I believe in this case it's more appropriate to keep it as it is, so that we can assert the implicit conversation behaviour on test suites.
andy-hf-kwok
force-pushed
the
ft-hf-chore-numeric-widen
branch
from
a1f1b3c to
c1b4b86
Compare
Signed-off-by: Andy HF Kwok <[email protected]>
Signed-off-by: Andy HF Kwok <[email protected]>
Signed-off-by: Andy HF Kwok <[email protected]>
Signed-off-by: Andy HF Kwok <[email protected]>
andy-hf-kwok
force-pushed
the
ft-hf-chore-numeric-widen
branch
from
c047bd0 to
8712bde
Compare
|
@comphead @mbutrovich I have removed some explicit conversions and added a suppression instead, as it seems more appropriate. |
5 participants
Which issue does this PR close?
This PR aims to address the
-Wnumeric-widenwarning in the codebase.As the overall scope of strict-warning compliance is quite large, this change focuses on resolving this specific warning type as a first step. Future PRs will continue addressing other warning incrementally.
Partially closes #. #2255
Rationale for this change
To clean up the code as per static check with the hope to resolve all warning and promote the restrict warning profile for the default CI build to uphold Scala code quality.
What changes are included in this PR?
To perform explicit type conversion instead of relying on runtime implicit cast.
How are these changes tested?
-Wnumeric-widenoption on.