Skip to content

Commit 0f42e1f

Browse files
sharwellsharwell
committed
Update SA1207 to handle 'private protected' accessibility
1 parent 62bd77a commit 0f42e1f

5 files changed

Lines changed: 102 additions & 15 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/SA1207CodeFixProvider.cs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace StyleCop.Analyzers.OrderingRules
55
{
66
using System.Collections.Immutable;
77
using System.Composition;
8+
using System.Linq;
89
using System.Threading;
910
using System.Threading.Tasks;
1011
using Microsoft.CodeAnalysis;
@@ -59,21 +60,23 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
5960
return document;
6061
}
6162

62-
var newDeclarationNode = originalDeclarationNode.ReplaceTokens(childTokens, ComputeReplacementToken);
63+
bool hasInternalKeyword = childTokens.Any(token => token.IsKind(SyntaxKind.InternalKeyword));
64+
var newDeclarationNode = originalDeclarationNode.ReplaceTokens(childTokens, (originalToken, rewrittenToken) => ComputeReplacementToken(originalToken, rewrittenToken, hasInternalKeyword));
6365

6466
var newSyntaxRoot = syntaxRoot.ReplaceNode(originalDeclarationNode, newDeclarationNode);
6567
return document.WithSyntaxRoot(newSyntaxRoot);
6668
}
6769

68-
private static SyntaxToken ComputeReplacementToken(SyntaxToken originalToken, SyntaxToken rewrittenToken)
70+
private static SyntaxToken ComputeReplacementToken(SyntaxToken originalToken, SyntaxToken rewrittenToken, bool hasInternalKeyword)
6971
{
70-
if (originalToken.IsKind(SyntaxKind.InternalKeyword))
72+
if (originalToken.IsKind(SyntaxKind.InternalKeyword)
73+
|| originalToken.IsKind(SyntaxKind.PrivateKeyword))
7174
{
7275
return SyntaxFactory.Token(SyntaxKind.ProtectedKeyword).WithTriviaFrom(rewrittenToken);
7376
}
7477
else if (originalToken.IsKind(SyntaxKind.ProtectedKeyword))
7578
{
76-
return SyntaxFactory.Token(SyntaxKind.InternalKeyword).WithTriviaFrom(rewrittenToken);
79+
return SyntaxFactory.Token(hasInternalKeyword ? SyntaxKind.InternalKeyword : SyntaxKind.PrivateKeyword).WithTriviaFrom(rewrittenToken);
7780
}
7881
else
7982
{

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/OrderingRules/SA1207CSharp7UnitTests.cs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,78 @@
33

44
namespace StyleCop.Analyzers.Test.CSharp7.OrderingRules
55
{
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Microsoft.CodeAnalysis.CSharp;
9+
using Microsoft.CodeAnalysis.Testing;
10+
using StyleCop.Analyzers.OrderingRules;
611
using StyleCop.Analyzers.Test.OrderingRules;
12+
using StyleCop.Analyzers.Test.Verifiers;
13+
using Xunit;
14+
using static StyleCop.Analyzers.Test.Verifiers.CustomDiagnosticVerifier<StyleCop.Analyzers.OrderingRules.SA1207ProtectedMustComeBeforeInternal>;
715

816
public class SA1207CSharp7UnitTests : SA1207UnitTests
917
{
18+
[Fact]
19+
public async Task TestPrivateProtectedAsync()
20+
{
21+
var testCode = @"namespace TestNamespace
22+
{
23+
public class TestClass
24+
{
25+
protected private int testField;
26+
}
27+
}
28+
";
29+
30+
var fixedTestCode = @"namespace TestNamespace
31+
{
32+
public class TestClass
33+
{
34+
private protected int testField;
35+
}
36+
}
37+
";
38+
39+
var expectedDiagnostic = Diagnostic().WithArguments("private", "protected").WithLocation(5, 19);
40+
await VerifyCSharpFixAsync(LanguageVersion.CSharp7_2, testCode, expectedDiagnostic, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
41+
}
42+
43+
private static Task VerifyCSharpFixAsync(LanguageVersion languageVersion, string source, DiagnosticResult expected, string fixedSource, CancellationToken cancellationToken)
44+
{
45+
return VerifyCSharpFixAsync(languageVersion, source, new[] { expected }, fixedSource, cancellationToken);
46+
}
47+
48+
private static Task VerifyCSharpFixAsync(LanguageVersion languageVersion, string source, DiagnosticResult[] expected, string fixedSource, CancellationToken cancellationToken)
49+
{
50+
var test = new CSharpTest(languageVersion)
51+
{
52+
TestCode = source,
53+
FixedCode = fixedSource,
54+
};
55+
56+
if (source == fixedSource)
57+
{
58+
test.FixedState.InheritanceMode = StateInheritanceMode.AutoInheritAll;
59+
test.FixedState.MarkupHandling = MarkupMode.Allow;
60+
test.BatchFixedState.InheritanceMode = StateInheritanceMode.AutoInheritAll;
61+
test.BatchFixedState.MarkupHandling = MarkupMode.Allow;
62+
}
63+
64+
test.ExpectedDiagnostics.AddRange(expected);
65+
return test.RunAsync(cancellationToken);
66+
}
67+
68+
private class CSharpTest : StyleCopCodeFixVerifier<SA1207ProtectedMustComeBeforeInternal, SA1207CodeFixProvider>.CSharpTest
69+
{
70+
public CSharpTest(LanguageVersion languageVersion)
71+
{
72+
this.SolutionTransforms.Add((solution, projectId) =>
73+
{
74+
var parseOptions = (CSharpParseOptions)solution.GetProject(projectId).ParseOptions;
75+
return solution.WithProjectParseOptions(projectId, parseOptions.WithLanguageVersion(languageVersion));
76+
});
77+
}
78+
}
1079
}
1180
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1207UnitTests.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ public async Task TestInvalidDeclarationAsync(string invalidDeclaration, int dia
129129
var testCode = TestCodeTemplate.Replace("$$", invalidDeclaration);
130130
var fixedTestCode = TestCodeTemplate.Replace("$$", fixedDeclaration);
131131

132-
await VerifyCSharpFixAsync(testCode, Diagnostic().WithLocation(4, 4 + diagnosticColumn), fixedTestCode, CancellationToken.None).ConfigureAwait(false);
132+
DiagnosticResult expected = Diagnostic().WithArguments("protected", "internal").WithLocation(4, 4 + diagnosticColumn);
133+
await VerifyCSharpFixAsync(testCode, expected, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
133134
}
134135
}
135136
}

‎StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1207ProtectedMustComeBeforeInternal.cs‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ namespace StyleCop.Analyzers.OrderingRules
1111

1212
/// <summary>
1313
/// The keyword <c>protected</c> is positioned after the keyword <c>internal</c> within the declaration of a
14-
/// protected internal C# element.
14+
/// protected internal C# element, or the keyword <c>private</c> is positioned after the keyword <c>protected</c>.
1515
/// </summary>
1616
/// <remarks>
1717
/// <para>A violation of this rule occurs when a protected internal element's access modifiers are written as
18-
/// <c>internal protected</c>. In reality, an element with the keywords <c>protected internal</c> will have the same
18+
/// <c>internal protected</c>, or when a private protected element's access modifiers are written as
19+
/// <c>protected private</c>. In reality, an element with the keywords <c>protected internal</c> will have the same
1920
/// access level as an element with the keywords <c>internal protected</c>. To make the code easier to read and more
2021
/// consistent, StyleCop standardizes the ordering of these keywords, so that a protected internal element will
2122
/// always be described as such, and never as internal protected. This can help to reduce confusion about whether
@@ -29,8 +30,8 @@ internal class SA1207ProtectedMustComeBeforeInternal : DiagnosticAnalyzer
2930
/// </summary>
3031
public const string DiagnosticId = "SA1207";
3132
private const string Title = "Protected should come before internal";
32-
private const string MessageFormat = "The keyword 'protected' should come before 'internal'.";
33-
private const string Description = "The keyword 'protected' is positioned after the keyword 'internal' within the declaration of a protected internal C# element.";
33+
private const string MessageFormat = "The keyword '{0}' should come before '{1}'.";
34+
private const string Description = "The keyword '{0}' is positioned after the keyword '{1}' within the declaration of a {0} {1} C# element.";
3435
private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1207.md";
3536

3637
private static readonly DiagnosticDescriptor Descriptor =
@@ -72,6 +73,7 @@ private static void HandleDeclaration(SyntaxNodeAnalysisContext context)
7273
return;
7374
}
7475

76+
bool protectedKeywordFound = false;
7577
bool internalKeywordFound = false;
7678
foreach (var childToken in childTokens)
7779
{
@@ -80,10 +82,22 @@ private static void HandleDeclaration(SyntaxNodeAnalysisContext context)
8082
internalKeywordFound = true;
8183
continue;
8284
}
83-
84-
if (internalKeywordFound && childToken.IsKind(SyntaxKind.ProtectedKeyword))
85+
else if (childToken.IsKind(SyntaxKind.ProtectedKeyword))
86+
{
87+
if (internalKeywordFound)
88+
{
89+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, childToken.GetLocation(), "protected", "internal"));
90+
break;
91+
}
92+
else
93+
{
94+
protectedKeywordFound = true;
95+
continue;
96+
}
97+
}
98+
else if (protectedKeywordFound && childToken.IsKind(SyntaxKind.PrivateKeyword))
8599
{
86-
context.ReportDiagnostic(Diagnostic.Create(Descriptor, childToken.GetLocation()));
100+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, childToken.GetLocation(), "private", "protected"));
87101
break;
88102
}
89103
}

‎documentation/SA1207.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@
1717

1818
## Cause
1919

20-
The keyword *protected* is positioned after the keyword *internal* within the declaration of a protected internal C# element.
20+
The keyword *protected* is positioned after the keyword *internal* within the declaration of a protected internal C# element, or the keyword *private* is positioned after the keyword *protected*.
2121

2222
## Rule description
2323

24-
A violation of this rule occurs when a protected internal element's access modifiers are written as *internal protected*. In reality, an element with the keywords *protected internal* will have the same access level as an element with the keywords *internal protected*. To make the code easier to read and more consistent, StyleCop standardizes the ordering of these keywords, so that a protected internal element will always be described as such, and never as internal protected. This can help to reduce confusion about whether these access levels are indeed the same.
24+
A violation of this rule occurs when a protected internal element's access modifiers are written as *internal protected*, or when a private protected element's access modifiers are written as *protected private*. In reality, an element with the keywords *protected internal* will have the same access level as an element with the keywords *internal protected*. To make the code easier to read and more consistent, StyleCop standardizes the ordering of these keywords, so that a protected internal element will always be described as such, and never as internal protected. This can help to reduce confusion about whether these access levels are indeed the same.
2525

2626
## How to fix violations
2727

28-
To fix an instance of this violation, place the *protected* keyword before the *internal* keyword.
28+
To fix an instance of this violation, place the *protected* keyword before the *internal* keyword, or place the *private* keyword before the *protected* keyword.
2929

3030
## How to suppress violations
3131

0 commit comments

Comments
 (0)