hi, @JaedenRuiner , and thanks for the time you invested in this topic.
S3265 Non-flags enums should not be used in bitwise operations
Exactly. This Code Smell rule says that, for the developers’ benefit, an enum
which is used for Bitwise operations should have the FlagsAttribute
.
Looking at MemberAttributes Enum, this is a pretty good example of a poorly designed enum
(smelly code).
System.CodeDom.MemberAttributes abstractFinal = System.CodeDom.MemberAttributes.Abstract | System.CodeDom.MemberAttributes.Final;
Console.WriteLine(abstractFinal == System.CodeDom.MemberAttributes.Static); // true!
Console.WriteLine((abstractFinal & System.CodeDom.MemberAttributes.Static) == System.CodeDom.MemberAttributes.Static); // true!
This sort of enum will trigger the S3265 , and once it will be a Flags
enum, it will trigger S4070 because it has a poor design. That’s life, it is technical debt however you put it.
This is a conflict between the designer of an API and the ones that use it. In this case, you should talk with the developers that use your enum
to use it correctly, as it’s not intended to be a Flags
enum. Or if it is intended to be a Flags
enum, mark it as such.
You should never write bad code to please the tool. If it makes sense, you can always mark an issue as Won't fix
and write a message in SQ for the reason why you’re not doing it.
S2346 Flags enumerations zero-value members should be named “None”
First of all, Microsoft’s Enum Design guideline clearly specifies:
✓ DO provide a value of zero on simple enums.
Consider calling the value something like “None.” If such a value is not appropriate for this particular enum, the most common default value for the enum should be assigned the underlying value of zero.
and
✓ DO name the zero value of flag enums
None
. For a flag enum, the value must always mean “all flags are cleared.”
Indeed, there are exceptions from this rule, like the TypeEnum
you give, which is rather a corner case. In my experience as a software engineer, I’ve seen the atrocities enums without a None
default value can do. For example, I’ve seen in product code a Result
enum which had the default value Error
- you couldn’t tell if the enum hasn’t been set yet, or if there was an error in the call. You say it yourself:
I agree with you, however please read my example above where someone can give the Error
name to a Result
enum default value. Making it None
is avoiding this problem.
S4070 Non-flags enums should not be marked with “FlagsAttribute”
First of all, Microsoft’s Enum Design guideline clearly specifies:
✓ DO apply the System.FlagsAttribute to flag enums. Do not apply this attribute to simple enums.
✓ DO use powers of two for the flag enum values so they can be freely combined using the bitwise OR operation.
X AVOID creating flag enums where certain combinations of values are invalid.
X AVOID using flag enum values of zero unless the value represents “all flags are cleared” and is named appropriately, as prescribed by the next guideline.
Please read my code sample from above on MemberAttributes and it should be pretty clear why this rule is necessary, especially to fix problems before they exist (during development and before release).
Please read Microsoft’s Enum Design guideline.