What's wrong with what FxCop does for globalization, Part 1

by Michael S. Kaplan, published on 2007/10/01 03:31 -04:00, original URI: http://blogs.msdn.com/b/michkap/archive/2007/10/01/5218976.aspx

It was way back in December of last year that I was talking about problems with FxCop and globalization (in posts like this one and this other one).

Then I sort of forgot about it.

Sorry, I have been busy. :-)

But then in the course of a month I was once again reminded of the problems in the form of three large projects that were (for lack of a better word) assaulted by these problems.

If you are a fan of the programs from the BuffyVerse (Buffy the Vampire Slayer and Angel) then you know that one of the rules with vampires is that they can't come in to your home unless you invite them in.

Well, FxCop is the same way -- you have to run it and then do something to follow its advice; if you don't, then nothing bad can befall you. :-)

Here is how the FxCop experience works:

1) You find the FxCop and run it.

2) You don't understand the errors at all as they seem wrong, then you discover that you are running the wrong version of FxCop for your project.

3) Repeat steps 1 and 2 another time or two while you get the right version.

4) Look at the errors and warnings, there are a bunch of level 1 and level 2 errors so you decide for this version to leave the level 3 and level 4 errors alone¹.

5) You ship your product.

6) In the space between ending development work on the version you just shipped and starting the development on the next version, someone decides to tackle all of the globalization errors.

Here is where the trouble happens, where some well-meaning developer has decided to let the vampires in, and indeed to help to extract the blood for them from your project.

The specific warnings are described here, there are really just three of them in particular, though they can easily leads to hundreds or even thousands of occurrences in a single project. They apply to every single method that either has an overload that takes a

The problems with the warnings are twofold:

1) FxCop assumes that if you are running the method that does not let you specify and IFormatProvider or a CultureInfo or you use the method with an override that dies not let you do so that your usage is wrong.

2) FxCop shuts up if you specify such a thing, even if it will return results that can be entirely wrong (like passing CurrentUICulture for formatting) or will be ignored )like passing a DateTimeFormatInfo style IFormatProvider when one is formatting numbers).

#1 can make the code less readable even if it was right before, and #2 the problems are I think self-explanatory though when one considers how easy it is to add CurrentUICulture to get rid of 90% of the warnings even though the answer is almost always wrong, you see where I start ascribing field-like attributes to our FxCop vampire we invited in.

In one such project, between the time that Vista was released and the beta 3 date of Server 2008, someone had updated the code in 250 different places, almost all of them wrong, and FxCop was happy with each mistake since it assumes that if you take the time to specify an IFormatProvider or a CultureInfo that you must have chosen one correctly, even if the choice is dead wrong.

Needless to say, the developer who was left holding the bag on this one (she was not the well-meaning developer who introduced the regressions) Kathy, once we walked through the very basic rules by which one could intelligently decide what the right fix should have been, was able to bring the product back to health in short order.

Unfortunately, not everyone is as skilled as she is, and not everyone has a team like the International Fundamentals team to ask for help.

But Kathy was kind enough to let me talk about the project as long as I didn't identify it and as long as I gave the rules that she was able to use to quickly bring the code back to a state of readiness. Here they are:

• String comparison:
• String comparison for equality -- Specify StringComparison.Ordinal or OrdinalIgnoreCase. This is true when comparing against string resources, and particularly important when comparing file names.
• String comparison for sorting – Specify StringComparison.CurrentCulture when sorting lists
• String.ToUpper – Use ToUpper rather than ToLower, and specify InvariantCulture in order to pick up OS casing rules
• Determining BiDi languages, for example to load correct icons – Continue to specify CurrentUICulture for these cases.
• String formatting including numbers, and int.ToString – Specify CurrentCulture
• String formatting of strings only (no numbers) – the Culture arg here is a no-op.  Use CurrentCulture.

But getting back to the problem with FxCop, it does not do enough real analysis of the usage. And as long as FxCop wants to warn that the code below is bad (switch to "English (United Kingdom)" in Regional Options to see the differences):

Console.WriteLine(DateTime.Now.ToString());

Even when it thinks the code below which returns identical results and is just completely wrong and weird is somehow fine to it:

Console.WriteLine(DateTime.Now.ToString(NumberFormatInfo.InvariantInfo));

When the intended code which will return different results would be:

Console.WriteLine(DateTime.Now.ToString(CultureInfo.InvariantCulture));

Then FxCop for globalization errors has a net effect of leaving the code in a possibly still incorrect and in all likelihood less readable form....

To spread the blame around a bit, the .NET Framework decision to have the IFormatProvider parameter be one that is implemented as "if the param applies then use it, otherwise treat is as null" is also slightly boneheaded, since that is the sort of thing that can easily lead to the wrong code being put in there in the first place.

But it is too late to fix all of them, so with our last defense as FxCop the tool's strength at defending against these scenarios is quite limited....

1 - The bulk of the globalization errors are there in those lower levels, I assume not because globalization isn't important but because there are a lot of false positives so if you average them together then they are less important per warning.

This post brought to you by ခ (U+1001, a.k.a. MYANMAR LETTER KHA)

# Mike Dimmick on 1 Oct 2007 8:19 AM:

# Michael S. Kaplan on 1 Oct 2007 9:39 AM:

# Arun Philip on 2 Oct 2007 2:50 AM:

# Dean Harding on 2 Oct 2007 5:21 AM:

# David M. Kean on 2 Oct 2007 11:30 PM:

# Michael S. Kaplan on 3 Oct 2007 12:59 AM:

referenced by

go to newer or older post, or back to index or month or day