Substitution code suggested by java:S2912 is not equivalent

SQ 8.6 (yeah yeah I know :slight_smile: )

Rule java:S2912 recommends explicitly specifying the start index of a string search. I got this message:
image
The two blacked out identifiers are the same String variable. The ! is followed by parentheses enclosing the two tests anded together which is why the 4 is followed by )) rather than ).

A couple of points here:

  1. The polarity of the recommended replacement (even assuming I replace n with 4) is inverted. It should be “== -1” or “< 0” since if the period is only at the start of the string then the second half of my condition will be true but indexOf(".",4)>-1 will be false.

  2. The replacement only makes sense if the first check is performed. (IDK if your checker takes that into account.)

  3. Even then, the code is still not equivalent, if the string being tested is, for instance, “0.00000.1”.

  4. And I could quibble over whether the replacement code is more understandable. The meaning of the original code is pretty clear: is there a period within the first 4 characters of the string?

I think I would only have this rule trigger when the check uses > or >=, but not < or <=.

And more on point 3: the code example in the popup:
image

shows two code fragments that are not equivalent. The “non-compliant” code asks: is the first occurrence of “ae” after position 2? The “compliant” code asks: is there an occurrence of “ae” at position 2 or greater? So to make them closer to equivalent, shouldn’t the first case use >= 2 or the second case use start index 3? Even after doing that, these are not the same thing if the string “aemael” is tested.

Hello @MisterPi

I agree with you, this rule is not only arguable and confusing but according to the last part of your message, it seems fundamentally wrong! I created a ticket to remove the rule:
SONARJAVA-4116.

Thanks for spotting this surprising problem.
Best,
Quentin

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.