SONARJAVA-5754 : Increment and decrement operators (++/--) should not be used with floating point variables #5371
Conversation
- Missing rule asciidoc/metadata yet
java-checks/src/main/java/org/sonar/java/checks/IncDecOnFPCheck.java
Outdated
Show resolved
Hide resolved
java-checks/src/main/java/org/sonar/java/checks/IncDecOnFPCheck.java
Outdated
Show resolved
Hide resolved
java-checks/src/main/java/org/sonar/java/checks/IncDecOnFPCheck.java
Outdated
Show resolved
Hide resolved
| if ( | ||
| tree instanceof UnaryExpressionTree unaryExpr && | ||
| unaryExpr.expression() instanceof IdentifierTree identifier && | ||
| // above condition should always be true, just used to pattern match safely |
There was a problem hiding this comment.
Conditions that are always be true have an annoying habit of triggering low code coverage ;) Let me know if this happens and I'll show you the workaround.
There was a problem hiding this comment.
Yes it seems it has an impact on branch coverage, I'm interested in knowing more about this workaround
| class IncDecOnFPCheckTest { | ||
|
|
||
| @Test | ||
| void detected() { |
There was a problem hiding this comment.
We ideally want to add a test .withoutSemantic() as well. It tests our checks in the scenario where the code is not analyzed as thouroughly as we ideally would likt.
There was a problem hiding this comment.
Added it, runs exactly the same test (same java check file), and no test fail, I'm not sure to understand how
| // above condition should always be true, just used to pattern match safely | ||
| (identifier.symbolType().isPrimitive(Primitives.FLOAT) || identifier.symbolType().isPrimitive(Primitives.DOUBLE)) | ||
| ) { | ||
| reportIssue(unaryExpr, "Increment and decrement operators (++/--) should not be used with floating point variables"); |
There was a problem hiding this comment.
Do you think it is better to have one common error message or one for ++ and another for --? I have no particular opinion, but if you prefer just one, then I'd like to know why.
There was a problem hiding this comment.
It's an interesting question, I would say it yes we can distinguish between both in the error message
Cons :
- The code is a bit more complex.
Pros :
- Message is more natural : makes it easier to read for the customer
- Simplicity of implementation : it only requires a string formatting and don't require duplicating the message for both case
2 participants
Waiting on this PR to add asciidoc and metadata