Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WTG3012 needs to restore leading trivia #190

Open
andrewhongnsw opened this issue Nov 9, 2022 · 2 comments
Open

WTG3012 needs to restore leading trivia #190

andrewhongnsw opened this issue Nov 9, 2022 · 2 comments

Comments

@andrewhongnsw
Copy link

In the following situations, WTG3012 removes the first preprocessor directive entirely, leading to CS1027 errors:

Example 1

return
#if DEBUG
	 something.HasValue ? something.Value :
#endif
true;

Example 2

return false
#if DEBUG
|| (
	value0 &&
	value1 &&
	value2
)
#endif
;

Example 3

protected virtual bool ShouldDoSomething => true
#if DEBUG
	&& !(value0 && value1)
#endif
;
@brian-reichle
Copy link
Contributor

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

@yaakov-h
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants