9

The MSDN documentation for the Flag attribute says that you should:

Define enumeration constants in powers of two, that is, 1, 2, 4, 8, and so on. This means the individual flags in combined enumeration constants do not overlap.

...and of course I always try to remember to do that. However, nothing enforce that and if you just create an enumeration the 'basic' way like...

[Flags]
public enum BrokenEnum
{
    None,
    FirstOption,
    SecondOption,
    ThirdOption
}

...it won't behave as expected. To combat this, I'm looking for some kind of static code analysis (like FxCop) that can warn me when an enum like the one above exists in my code. The closest such warning I could find was 'CA1008: Enums should have zero value' - which is also helpful for designing flags enumeration correctly but isn't enough.

What is the best way to find incorrectly designed flags enums in my code? The more automated the solution, the better.

3 Answers 3

5

Sometimes you want to have a flags enum that represents multiple options; in cases like that, it's not an error. Here's a common example:

[Flags]
public enum FilePermissions
{
    None = 0,
    Read = 1,
    Write = 2,
    Execute = 4,

    ReadWrite = 3, // Read | Write,
    ReadWriteExecute = 7 // Read | Write | Execute
}

Perhaps because of the need to support cases like these, that's why a compiler doesn't cause a warning or error.

2
  • I can see reasons for why the compiler doesn't do it (also thinking backwards compatibility, unjustified complexity, etc.) so I'm fine that you don't get a compiler warning. But I'm still looking for a way to be warned. In cases like yours were the use is valid, you could disable inspection with something like the SuppressMessage attribute. Commented Aug 20, 2011 at 6:24
  • You can also have more elaborate flags. For example { View = 1, Alter = 2, Action = 4 | View | Alter }: thus to action an item you need View and Alter; in addition to the right itself. Commented Aug 20, 2011 at 10:21
3

As Jacob says, it can be useful to have mixtures of flags... but possibly you could indicate that somehow so that your detection doesn't mind.

It shouldn't be too hard to write a unit test which goes through every enum in an assembly decorated with [Flags] and checks that there's a value for 0 (possibly ensuring it's called None or Default) and that every other defined value (from Enum.GetValues()) is a power of two. You can check that using if ((x & (x - 1)) == 0).

You could potentially have something like an attribute [Combination] to indicate values which are designed to be combinations... they could even indicate what flag names they're meant to be combinations of, so you could check that too.

I know this isn't quite as good as a compile-time check, but assuming you're already running tests regularly, it's pretty close.

1
  • 1
    I guess I shouldn't have been thinking 'inside the box' of static code analysis. A unit test like this should be pretty easy and give me nearly the same end result. Commented Aug 20, 2011 at 20:21
3

I never tried it myself, but maybe you could write a custom rule for FxCop.

Check FxCop and Code Analysis: Writing Your Own Custom Rules.

1
  • Thanks, that seems very promising. I might stick with Jon Skeet's answer just because I think it will be faster in the short term to write a unit test instead of learning how to write a custom FxCop rule. Commented Aug 20, 2011 at 20:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.