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

SA1002 doesn't handle inline comments well #1426

Closed
Tinister opened this issue Sep 9, 2015 · 5 comments
Closed

SA1002 doesn't handle inline comments well #1426

Tinister opened this issue Sep 9, 2015 · 5 comments
Assignees
Milestone

Comments

@Tinister
Copy link

Tinister commented Sep 9, 2015

Example:

public static int GetBitPattern(PType ptype)
{
    return IsSpecial(ptype) ? 0x11001 /*legacy*/ : 0x100001 /*since april 2014*/;
}

It wants to remove the space before the inline comment (starting with /*since).

@sharwell
Copy link
Member

sharwell commented Sep 9, 2015

💡 This method should probably be rewritten.

public static int GetBitPattern(PType ptype)
{
    // Since April, 2014
    const int CurrentBitPattern = 0x100001;

    // Before April, 2014
    const int LegacyBitPattern = 0x11001;

    return IsSpecial(ptype) ? LegacyBitPattern : CurrentBitPattern;
}

@Tinister
Copy link
Author

Tinister commented Sep 9, 2015

Sure, but if teams wanted to discourage that behavior wouldn't that be more appropriate for a separate "don't use delimited comments in the middle of a line of code" rule instead of having rules like SA1002 misflag in those situations?

@sharwell
Copy link
Member

sharwell commented Sep 9, 2015

I think it makes sense to not report SA1002 for this specific example shown above. I would expect it to be reported if a space appeared between the comment and the ;.

@Noryoko what do you think? Should we only look for immediately preceding spaces?

@oatkins What does StyleCop "classic" do for this?

@Noryoko
Copy link
Contributor

Noryoko commented Sep 9, 2015

I agree. We should only check immediately preceding spaces. 👍

@oatkins
Copy link
Contributor

oatkins commented Sep 9, 2015

@sharwell I was (pleasantly) surprised to find that classic SC doesn't report anything for the example at the top of this file. You were correct in thinking that you'd only get an error if you were to insert a space before the semi-colon.

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

No branches or pull requests

4 participants