Open andrewhongnsw opened 1 year ago
WTG3012 doesn't consider the pre-processor directives, maybe we could simply not offer a code-fix if the expression contains pre-processor directives; but all those examples represent dodgy practices that would fail on at least two other rules. I'm not sure if this is a defect worth fixing.
to me, they would be better written as (assuming the condition on DEBUG is unavoidable):
#if DEBUG
if (something.HasValue)
{
return something.Value;
}
#endif
return true;
#if DEBUG
if (value0 && value1 && value2)
{
return true;
}
#endif
return false;
#if DEBUG
protected virtual bool ShouldDoSomething => !(value0 && value1);
#else
protected virtual bool ShouldDoSomething => true;
#endif
All three of these examples already trigger WTG3103 (and WTG2002, but that's due to the name of the symbol).
That rule has been suppressed in a few repos due to a pre-existing habit of doing things like this.
I'd be tempted to leave it as is, but I think the more comprehensive approach is to suppress the Diagnostic (or just suppress the code-fix) if the expression contains preprocessor directives, and rely on WTG3103 to catch cases like this.
If a team then turns off WTG3103, they only have themselves to blame for tricky code like this.
In the following situations, WTG3012 removes the first preprocessor directive entirely, leading to CS1027 errors:
Example 1
Example 2
Example 3