Fix S1144: Nested type constructor accessibility is wrong in the rule message #9108
Conversation
| public void UnusedPrivateMember_NonPrivateConstructorInPrivateClass(string accessModifier, string expectedMessage) => | ||
| builder.AddSnippet($$$""" | ||
| public class Some | ||
| { | ||
| private class Foo // Noncompliant | ||
| { | ||
| {{{accessModifier}}} Foo() // Noncompliant {{{{{expectedMessage}}}}} | ||
| { | ||
| var a = 1; | ||
| } | ||
| } | ||
| } | ||
| """).Verify(); |
There was a problem hiding this comment.
The indentation can be improved.
| public void UnusedPrivateMember_NonPrivateConstructorInPrivateClass(string accessModifier, string expectedMessage) => | |
| builder.AddSnippet($$$""" | |
| public class Some | |
| { | |
| private class Foo // Noncompliant | |
| { | |
| {{{accessModifier}}} Foo() // Noncompliant {{{{{expectedMessage}}}}} | |
| { | |
| var a = 1; | |
| } | |
| } | |
| } | |
| """).Verify(); | |
| public void UnusedPrivateMember_NonPrivateConstructorInPrivateClass(string accessModifier, string expectedMessage) => | |
| builder.AddSnippet($$$""" | |
| public class Some | |
| { | |
| private class Foo // Noncompliant | |
| { | |
| {{{accessModifier}}} Foo() // Noncompliant {{{{{expectedMessage}}}}} | |
| { | |
| var a = 1; | |
| } | |
| } | |
| } | |
| """).Verify(); |
There was a problem hiding this comment.
All the test classes:
UnusedPrivateMemberTest.Constructors.csUnusedPrivateMemberTest.Fields.csUnusedPrivateMemberTest.Methods.csUnusedPrivateMemberTest.Properties.csUnusedPrivateMemberTest.Types.cs
are following this indentation, I don't like it either, but I like it even less refactoring all the above :D.
Given this context, are you ok with merging as is?
There was a problem hiding this comment.
There are three options:
- Leave as is
- Refactor everything
- Clean as you code
I'd go with 3, but if you think it's too much of a sore on the eye, sticking with 1 is also fine.
|
Ah... one of my comments did not make it from my brain to the keyboard: |
| internal class MyClass | ||
| { | ||
| protected MyClass() // Compliant | ||
| { | ||
| var a = 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Good test! I actually meant one with a private constructor (should be Noncompliant with a slightly different message).
There was a problem hiding this comment.
The message will be the same (the default) as the first DataRow in UnusedPrivateMember_NonPrivateConstructorInPrivateClass: Remove the unused private constructor 'blabla'.
I'll add this as well anyway 👍
There was a problem hiding this comment.
You are absolutely right, of course. Still a good case to test!
|
|
Fixes #7774