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

update ordering diagnostics to proper StyleCop order #1128

Merged
merged 3 commits into from
Aug 6, 2015

Conversation

Noryoko
Copy link
Contributor

@Noryoko Noryoko commented Aug 6, 2015

Originally found in 1204. The also applies to 1202, 1203, 1214, and 1215. 1215 was updated to match the diagnostic location style of the other diagnostic rules.

fixes #1123

@sharwell sharwell added this to the 1.0.0 Beta 4 milestone Aug 6, 2015
@sharwell
Copy link
Member

sharwell commented Aug 6, 2015

@oatkins Can you review the tests included in this for accuracy?

@oatkins
Copy link
Contributor

oatkins commented Aug 6, 2015

The tests look good.

It looks as though we have a large block of correctly ordered code that is used in the tests for multiple rules. Perhaps this could be refactored into a common location? It also seems to be missing a nested static class, for completeness.

I'm going to merge this branch into the version I'll use on my machine today, so I should find any remaining gremlins pretty quickly.

@Noryoko
Copy link
Contributor Author

Noryoko commented Aug 6, 2015

Good catch. I'll get those added. Looks like the diagnostic messages should be updated on 1203, 1204, 1214, and 1215. They don't include the access level and could be misleading.

@Noryoko Noryoko mentioned this pull request Aug 6, 2015
@@ -0,0 +1,26 @@
namespace StyleCop.Analyzers.OrderingRules
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Consider moving this to StyleCop.Analyzers.Helpers.

@sharwell sharwell merged commit df6f548 into DotNetAnalyzers:master Aug 6, 2015
sharwell added a commit that referenced this pull request Aug 6, 2015
@sharwell
Copy link
Member

sharwell commented Aug 6, 2015

👍

@Noryoko Noryoko deleted the fix-1123 branch August 7, 2015 01:53
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

Successfully merging this pull request may close these issues.

SA1204 incorrectly reported when private static method follows non-private instance method
3 participants