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

Code doesn't report SA1304 #1780

Closed
GregReddick opened this issue Nov 16, 2015 · 2 comments
Closed

Code doesn't report SA1304 #1780

GregReddick opened this issue Nov 16, 2015 · 2 comments
Assignees
Labels

Comments

@GregReddick
Copy link

First reported in #1766, Issue D11, with refinements here.

The code below would report SA1304, SA1307, and SA1311 in classic StyleCop. In the current analyzer, it reports only SA1307 and SA1311, not reporting SA1304. I think even two warnings may be too much, and we should at the least document the difference, and consider suppressing SA1307 too. If there is a large code base trying to be changed to the StyleCop style, the proliferation of warnings becomes daunting when you have 1000+ warnings that show up, when half of which are redundant reporting of "capitalize your field names".

namespace RuleViolations
{
    /// <summary>A rule SA1311 fail.</summary>
    public static class RuleSA1311Fail
    {
        /// <summary>The value.</summary>
        public static readonly int value = 1;
    }
}
@sharwell
Copy link
Member

Related to #574 and #1306 (which focused on instance fields).

I think only SA1311 should be reported here.

@sharwell
Copy link
Member

I remember why we did this now.

  • SA1307 affects accessible fields, which means the public API surface area.
  • SA1311 is a style choice which partially overlaps with SA1307.

It is reasonable to think some teams would disable SA1307 while others would disable SA1311. If SA1311 was a strict subset of SA1307, then it would make sense to not report SA1307 here, but that is not the case.

SA1304 is suppressed in this case because the emphasis for teams is some combination of SA1307 and SA1311.

@sharwell sharwell removed this from the Backlog milestone Nov 26, 2015
@sharwell sharwell self-assigned this Nov 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants