Skip to content

Commit d22386f

Browse files
bjornhellanderbjornhellander
committed
Added new test case for SA1205 to verify propagation of access modifiers between multiple parts of a nested partial type. Updated code fix accordingly.
1 parent 92e453f commit d22386f

3 files changed

Lines changed: 108 additions & 23 deletions

File tree

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

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
namespace StyleCop.Analyzers.OrderingRules
55
{
6-
using System;
76
using System.Collections.Immutable;
87
using System.Composition;
98
using System.Threading;
@@ -22,6 +21,13 @@ namespace StyleCop.Analyzers.OrderingRules
2221
[Shared]
2322
internal class SA1205CodeFixProvider : CodeFixProvider
2423
{
24+
private static readonly ImmutableArray<SyntaxKind> PublicAccessibilityKeywords = ImmutableArray.Create(SyntaxKind.PublicKeyword);
25+
private static readonly ImmutableArray<SyntaxKind> InternalAccessibilityKeywords = ImmutableArray.Create(SyntaxKind.InternalKeyword);
26+
private static readonly ImmutableArray<SyntaxKind> ProtectedAccessibilityKeywords = ImmutableArray.Create(SyntaxKind.ProtectedKeyword);
27+
private static readonly ImmutableArray<SyntaxKind> ProtectedOrInternalAccessibilityKeywords = ImmutableArray.Create(SyntaxKind.ProtectedKeyword, SyntaxKind.InternalKeyword);
28+
private static readonly ImmutableArray<SyntaxKind> PrivateAccessibilityKeywords = ImmutableArray.Create(SyntaxKind.PrivateKeyword);
29+
private static readonly ImmutableArray<SyntaxKind> UnexpectedAccessibilityKeywords = ImmutableArray.Create<SyntaxKind>();
30+
2531
/// <inheritdoc/>
2632
public override ImmutableArray<string> FixableDiagnosticIds { get; } =
2733
ImmutableArray.Create(SA1205PartialElementsMustDeclareAccess.DiagnosticId);
@@ -60,29 +66,33 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
6066
}
6167

6268
var symbol = semanticModel.GetDeclaredSymbol(typeDeclarationNode);
63-
var accessModifierKind = GetMissingAccessModifier(symbol.DeclaredAccessibility);
69+
var accessModifierKinds = GetMissingAccessModifiers(symbol.DeclaredAccessibility);
6470

6571
var keywordToken = typeDeclarationNode.Keyword;
6672

67-
var replacementModifiers = DeclarationModifiersHelper.AddModifier(typeDeclarationNode.Modifiers, ref keywordToken, accessModifierKind);
73+
var replacementModifiers = DeclarationModifiersHelper.AddModifiers(typeDeclarationNode.Modifiers, ref keywordToken, accessModifierKinds);
6874
var replacementNode = ReplaceModifiers(typeDeclarationNode, replacementModifiers);
6975
replacementNode = ReplaceKeyword(replacementNode, keywordToken);
7076
var newSyntaxRoot = syntaxRoot.ReplaceNode(typeDeclarationNode, replacementNode);
7177
return document.WithSyntaxRoot(newSyntaxRoot);
7278
}
7379

74-
private static SyntaxKind GetMissingAccessModifier(Accessibility accessibility)
80+
private static ImmutableArray<SyntaxKind> GetMissingAccessModifiers(Accessibility accessibility)
7581
{
7682
switch (accessibility)
7783
{
7884
case Accessibility.Public:
79-
return SyntaxKind.PublicKeyword;
85+
return PublicAccessibilityKeywords;
8086
case Accessibility.Internal:
81-
return SyntaxKind.InternalKeyword;
87+
return InternalAccessibilityKeywords;
88+
case Accessibility.Protected:
89+
return ProtectedAccessibilityKeywords;
90+
case Accessibility.ProtectedOrInternal:
91+
return ProtectedOrInternalAccessibilityKeywords;
8292
case Accessibility.Private:
83-
return SyntaxKind.PrivateKeyword;
93+
return PrivateAccessibilityKeywords;
8494
default:
85-
throw new InvalidOperationException("Unexpected accessibility " + accessibility);
95+
return UnexpectedAccessibilityKeywords; // This should not happen!
8696
}
8797
}
8898

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

Lines changed: 66 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,30 @@ public static IEnumerable<object[]> InvalidDeclarations
5959
}
6060
}
6161

62+
public static IEnumerable<object[]> ValidNestedDeclarations
63+
{
64+
get
65+
{
66+
yield return new object[] { "public", "class" };
67+
yield return new object[] { "protected", "class" };
68+
yield return new object[] { "internal", "class" };
69+
yield return new object[] { "protected internal", "class" };
70+
yield return new object[] { "private", "class" };
71+
72+
yield return new object[] { "public", "struct" };
73+
yield return new object[] { "protected", "struct" };
74+
yield return new object[] { "internal", "struct" };
75+
yield return new object[] { "protected internal", "struct" };
76+
yield return new object[] { "private", "struct" };
77+
78+
yield return new object[] { "public", "interface" };
79+
yield return new object[] { "protected", "interface" };
80+
yield return new object[] { "internal", "interface" };
81+
yield return new object[] { "protected internal", "interface" };
82+
yield return new object[] { "private", "interface" };
83+
}
84+
}
85+
6286
/// <summary>
6387
/// Verifies that a valid declaration (with an access modifier or not a partial type) will not produce a diagnostic.
6488
/// </summary>
@@ -171,21 +195,7 @@ public partial class Foo
171195
/// <param name="typeKeyword">The type keyword to use.</param>
172196
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
173197
[Theory]
174-
[InlineData("public", "class")]
175-
[InlineData("protected", "class")]
176-
[InlineData("internal", "class")]
177-
[InlineData("protected internal", "class")]
178-
[InlineData("private", "class")]
179-
[InlineData("public", "struct")]
180-
[InlineData("protected", "struct")]
181-
[InlineData("internal", "struct")]
182-
[InlineData("protected internal", "struct")]
183-
[InlineData("private", "struct")]
184-
[InlineData("public", "interface")]
185-
[InlineData("protected", "interface")]
186-
[InlineData("internal", "interface")]
187-
[InlineData("protected internal", "interface")]
188-
[InlineData("private", "interface")]
198+
[MemberData(nameof(ValidNestedDeclarations))]
189199
public async Task TestNestedTypeAccessModifiersAsync(string accessModifier, string typeKeyword)
190200
{
191201
var testCode = $@"
@@ -232,6 +242,47 @@ public class Foo
232242
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
233243
}
234244

245+
/// <summary>
246+
/// Verifies that the code fix will properly copy over the access modifier defined in another fragment of the nested partial element.
247+
/// </summary>
248+
/// <param name="accessModifier">The access modifier to use for the nested type.</param>
249+
/// <param name="typeKeyword">The type keyword to use.</param>
250+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
251+
[Theory]
252+
[MemberData(nameof(ValidNestedDeclarations))]
253+
public async Task TestProperNestedAccessModifierPropagationAsync(string accessModifier, string typeKeyword)
254+
{
255+
var testCode = $@"
256+
public class Foo
257+
{{
258+
{accessModifier} partial {typeKeyword} Bar
259+
{{
260+
}}
261+
262+
partial {typeKeyword} Bar
263+
{{
264+
}}
265+
}}
266+
";
267+
268+
var fixedTestCode = $@"
269+
public class Foo
270+
{{
271+
{accessModifier} partial {typeKeyword} Bar
272+
{{
273+
}}
274+
275+
{accessModifier} partial {typeKeyword} Bar
276+
{{
277+
}}
278+
}}
279+
";
280+
281+
await this.VerifyCSharpDiagnosticAsync(testCode, this.CSharpDiagnostic().WithLocation(8, 14 + typeKeyword.Length), CancellationToken.None).ConfigureAwait(false);
282+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
283+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
284+
}
285+
235286
/// <inheritdoc/>
236287
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
237288
{

‎StyleCop.Analyzers/StyleCop.Analyzers/Helpers/DeclarationModifiersHelper.cs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
namespace StyleCop.Analyzers.Helpers
55
{
6+
using System.Collections.Generic;
7+
using System.Linq;
68
using Microsoft.CodeAnalysis;
79
using Microsoft.CodeAnalysis.CSharp;
810
using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -71,6 +73,28 @@ internal static SyntaxTokenList AddModifier(SyntaxTokenList modifiers, ref Synta
7173
return modifiers;
7274
}
7375

76+
/// <summary>
77+
/// Adds a number of modifier tokens for <paramref name="modifierKeywords"/> to the beginning of
78+
/// <paramref name="modifiers"/>. The trivia for the new modifier and the trivia for the token that follows it
79+
/// are updated to ensure that the new modifier is placed immediately before the syntax token that follows it,
80+
/// separated by exactly one space.
81+
/// </summary>
82+
/// <param name="modifiers">The existing modifiers. This may be empty if no modifiers are present.</param>
83+
/// <param name="leadingTriviaToken">The syntax token which follows the modifiers. The trivia for this token is
84+
/// updated if (and only if) the existing <paramref name="modifiers"/> list is empty.</param>
85+
/// <param name="modifierKeywords">The modifier keywords to add.</param>
86+
/// <returns>A <see cref="SyntaxTokenList"/> representing the original modifiers (if any) with the addition of a
87+
/// modifier of the specified <paramref name="modifierKeywords"/> at the beginning of the list.</returns>
88+
internal static SyntaxTokenList AddModifiers(SyntaxTokenList modifiers, ref SyntaxToken leadingTriviaToken, IEnumerable<SyntaxKind> modifierKeywords)
89+
{
90+
foreach (var modifierKeyword in modifierKeywords.Reverse())
91+
{
92+
modifiers = AddModifier(modifiers, ref leadingTriviaToken, modifierKeyword);
93+
}
94+
95+
return modifiers;
96+
}
97+
7498
internal static SyntaxTokenList GetModifiers(this MemberDeclarationSyntax syntax)
7599
{
76100
if (syntax is BaseMethodDeclarationSyntax)

0 commit comments

Comments
 (0)