Code Analysis rule CA1062 behaviour

Question

I have following extension method for strings:

public static bool IsNullOrEmpty(this string target)
{
    return string.IsNullOrEmpty(target);
}

... and in the code I use it as follows:

public static string DoSomethingOnString(this string target)
{
    if (target.IsNullOrEmpty())
        return target;

    target = target.Trim();  //This line causes CA1062 violation

    return target;
}

Now, if I run code analysis on this, I get a violation of rule CA1062. But if I change the code to:

public static string DoSomethingOnString(this string target)
{
    if (string.IsNullOrEmpty(target))  //CHANGED LINE
        return target;

    target = target.Trim();  //This line DOES NOT cause CA1062 violation anymore

    return target;
}

... then it is fine.

Why it thinks that I'm not checking for null condition in the first example? Does it check only for string.IsNullOrEmpty or string.IsNullOrWhiteSpace? Is there a way to make CA recognize my extension method, or I'll need to suppress this rule?

UPDATE: If you have the same issue you can vote on feedback item I submitted on MS Connect: Code Analysis rule CA1062 raises false alarm

Answer 1

Why it thinks that I'm not checking for null condition in the first example?

Quite simply, FxCop doesn't understand that if your IsNullOrEmpty extension method does the same thing as string.IsNullOrEmpty. It doesn't realize that if target is null, IsNullOrEmpty will return true and your method will exit. Basically I suspect it has in-built knowledge of string.IsNullOrEmpty. Code Contracts is more likely to have success here, as I believe FxCop only performs a relatively shallow check on what your code does, compared with the deep reasoning of Code Contracts. You could decorate your IsNullOrEmpty method with ValidatedNotNullAttribute to inform FxCop what's going on.

public static bool IsNullOrEmpty([ValidatedNotNullAttribute] this string target)
{
    return string.IsNullOrEmpty(target);
}
//The naming is important to inform FxCop
sealed class ValidatedNotNullAttribute : Attribute { }

This is just an example of where code analysis can sometimes be a little too eager to criticize. It's something I've seen with pretty much every code analysis tool I've used. Your choices are usually along the lines of:

• Change your code to work around the code analysis tool, even if it was fine before
• Suppress the rules at specific sites, after manually checking each of them
• Suppress whole rules if they frequently give false positives
• Abandon the code analysis tool entirely

You should also log a bug or feature request, of course...

Answer 2

It looks like they finally got around to fixing this in the roslyn analyzers.

Bug report here: https://github.com/dotnet/roslyn-analyzers/issues/2369