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

Public Static Readonly Fields Trigger SA1401 #1320

Closed
lumberjack4 opened this issue Aug 29, 2015 · 8 comments
Closed

Public Static Readonly Fields Trigger SA1401 #1320

lumberjack4 opened this issue Aug 29, 2015 · 8 comments
Assignees
Milestone

Comments

@lumberjack4
Copy link

public static readonly MyClass Empty = new MyClass();

SA1401 is triggered telling me that the field must be private. Recommend ignoring the rule for public static readonly fields, as this is the .NET paradigm for creating empty objects.

@sharwell
Copy link
Member

Considering that C# 6 made it extremely easy to expose these as properties, my vote is that we not change the current behavior. The framework contains as many examples of properties being used for this purpose (e.g. Encoding.UTF8). To resolve the issue, you can actually update your code to the following equally-succinct form, and improve your exposed API at the same time:

public static MyClass Empty { get; } = new MyClass();

I'm leaving this marked needs discussion for now to get some more feedback.

By the way, welcome to the project @lumberjack4. 😄 Thanks for taking the time to try out the product and file all these issues! 👍

@lumberjack4
Copy link
Author

Cool! I'm still trying to remember all the new changes.

@ghost
Copy link

ghost commented Aug 29, 2015

See #1308

@bezysoftware
Copy link

What about dependency properties? Typically when you use the "propdp" snippet you get this generated:

public static readonly DependencyProperty FooProperty =
    DependencyProperty.Register("Foo", typeof(string), typeof(Class1), new PropertyMetadata(null));

which triggers SA1401.

@sharwell
Copy link
Member

@oatkins Can you check the behavior of StyleCop "classic" with code like the previous comment?

@oatkins
Copy link
Contributor

oatkins commented Sep 28, 2015

None of the following raises SA1401 in classic SC:

public static readonly DependencyProperty MyPropertyProperty =
    DependencyProperty.Register("MyProperty", typeof(int), typeof(HasDependencyProperty), new PropertyMetadata(0));

public static readonly string MyString = "fish";

public static readonly StringBuilder Builder = new StringBuilder();

Interestingly, SC is being cautious here. My last example exposes a mutable object as a static field, which isn't good practice... but I would think that someone who's explicitly made something public static readonly probably knows what they want and why!

I currently disable SA1401 for WPF projects; I would support never reporting this case so I can turn it back on.

@vweijsters
Copy link
Contributor

I would prefer that SA1401 will not be reported for public static readonly fields. The main reason being that we don't want to force people to change (a lot of) their code when migrating to StyleCop.Analyzers. A secondary reason to change the current behavior would be the dependency properties, which would be a major case for quite a few people I imagine.

@sharwell
Copy link
Member

Relabeling this as a bug.

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

5 participants