False negative?

SonarCloud
1

How come SonarCloud doesn’t tag the following code? not even as Code Smell:

private static string PhoneNumberValidation(string phoneNumber)
{
    var rgx = new Regex(@"(?i)[^0-9\s]");
    var replacement = "";
    var newValue = rgx.Replace(phoneNumber, replacement);

    if (newValue.Length < 8 || newValue.Length > 9)
    {
        throw new ArgumentException("Value mismatch", nameof(phoneNumber));
    }

    return newValue;
}
  1. For starters, anythingValidation that returns a string should be a smell. Also, anythingValidation should not explicitly throw an exception.
  2. The guy created a regex to clean the string of non-number characters but ended up using another technique to check its length. IMHO should be a smell because (based on his tests) the regex match \d{4,5}.?\d{4} would suffice
  3. Regex.Replace is a very expensive operation, since the declaration of intention is validation (per method naming), this should be an issue.
2

Hey there.

Have you checked the rules available for Java to see if there’s a relevant rule? If there isn’t, feel free to Suggest a New Rule. If not, I’d suggest naming the explicit rule you expect an issue to be raised for.