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

C# CC0013 CC0014 ternary operator fix removes comments #921

Open
KristianWedberg opened this issue May 24, 2017 · 3 comments
Open

C# CC0013 CC0014 ternary operator fix removes comments #921

KristianWedberg opened this issue May 24, 2017 · 3 comments

Comments

@KristianWedberg
Copy link

KristianWedberg commented May 24, 2017

Bug

C# CC0013 and CC0014 ternary operator fixes remove comments in code-cracker v1.0.3.

This CC0014 code:

int a;
if (Environment.MachineName == "Bilbo")
{
    a = 0; // Success
}
else
{
    a = 1; // Failure
}

Currently becomes this after fix applied:

a = Environment.MachineName == "Bilbo" ? 0 : 1;

And this code:

int a;
if (Environment.MachineName == "Bilbo")
    a = 0; // Success
else
    a = 1; // Failure

Currently becomes this after fix applied:

int a;
a = Environment.MachineName == "Bilbo" ? 0 : 1; // Failure

Expected output in both above cases after fix applied:

int a;
a = Environment.MachineName == "Bilbo" 
    ? 0 // Success
    : 1; // Failure

CC0013 behaves in the exact same (incorrect) way as CC0014.

As noted on other VB ternary operator fix issues, true and false comments should not be combined, since each comment almost always only apply to one of the branches.

If 'fixing the fix' properly is non-trivial, I would suggest not creating any fix suggestion at all if there are comments, rather than silently removing them.

@carloscds
Copy link
Contributor

@KristianWedberg Really is non-trivial fix. My suggestion is disable this fix if you use this kind of comment.

@KristianWedberg
Copy link
Author

This issue will certainly end up eating users comments, since few will know to look out for it. Doing this silently is... unfortunate. I would still recommend making CC0014 hidden until this issue is addressed.

@giggio
Copy link
Member

giggio commented Jun 13, 2017

I agree with you both, if there comments, we do not offer a diagnostic, for both CC0013 and CC0014.

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

No branches or pull requests

3 participants