Skip to content

Commit bd617d0

Browse files
sharwellsharwell
committed
Merge remote-tracking branch 'DotNetAnalyzers/pr/2119'
Corrected SA1205 code fix for nested partial types
2 parents 7a2006a + d22386f commit bd617d0

3 files changed

Lines changed: 152 additions & 16 deletions

File tree

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ namespace StyleCop.Analyzers.OrderingRules
2121
[Shared]
2222
internal class SA1205CodeFixProvider : CodeFixProvider
2323
{
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+
2431
/// <inheritdoc/>
2532
public override ImmutableArray<string> FixableDiagnosticIds { get; } =
2633
ImmutableArray.Create(SA1205PartialElementsMustDeclareAccess.DiagnosticId);
@@ -59,17 +66,37 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
5966
}
6067

6168
var symbol = semanticModel.GetDeclaredSymbol(typeDeclarationNode);
62-
var accessModifierKind = (symbol.DeclaredAccessibility == Accessibility.Public) ? SyntaxKind.PublicKeyword : SyntaxKind.InternalKeyword;
69+
var accessModifierKinds = GetMissingAccessModifiers(symbol.DeclaredAccessibility);
6370

6471
var keywordToken = typeDeclarationNode.Keyword;
6572

66-
var replacementModifiers = DeclarationModifiersHelper.AddModifier(typeDeclarationNode.Modifiers, ref keywordToken, accessModifierKind);
73+
var replacementModifiers = DeclarationModifiersHelper.AddModifiers(typeDeclarationNode.Modifiers, ref keywordToken, accessModifierKinds);
6774
var replacementNode = ReplaceModifiers(typeDeclarationNode, replacementModifiers);
6875
replacementNode = ReplaceKeyword(replacementNode, keywordToken);
6976
var newSyntaxRoot = syntaxRoot.ReplaceNode(typeDeclarationNode, replacementNode);
7077
return document.WithSyntaxRoot(newSyntaxRoot);
7178
}
7279

80+
private static ImmutableArray<SyntaxKind> GetMissingAccessModifiers(Accessibility accessibility)
81+
{
82+
switch (accessibility)
83+
{
84+
case Accessibility.Public:
85+
return PublicAccessibilityKeywords;
86+
case Accessibility.Internal:
87+
return InternalAccessibilityKeywords;
88+
case Accessibility.Protected:
89+
return ProtectedAccessibilityKeywords;
90+
case Accessibility.ProtectedOrInternal:
91+
return ProtectedOrInternalAccessibilityKeywords;
92+
case Accessibility.Private:
93+
return PrivateAccessibilityKeywords;
94+
default:
95+
// This should not happen!
96+
return UnexpectedAccessibilityKeywords;
97+
}
98+
}
99+
73100
// This code was copied from the Roslyn code base (and slightly modified). It can be removed if
74101
// TypeDeclarationSyntaxExtensions.WithModifiers is made public (Roslyn issue #2186)
75102
private static TypeDeclarationSyntax ReplaceModifiers(TypeDeclarationSyntax node, SyntaxTokenList modifiers)

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

Lines changed: 99 additions & 14 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>
@@ -86,6 +110,7 @@ public async Task TestInvalidDeclarationAsync(string declaration)
86110

87111
await this.VerifyCSharpDiagnosticAsync(testCode, this.CSharpDiagnostic().WithLocation(1, 2 + declaration.Length), CancellationToken.None).ConfigureAwait(false);
88112
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
113+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
89114
}
90115

91116
/// <summary>
@@ -170,34 +195,94 @@ public partial class Foo
170195
/// <param name="typeKeyword">The type keyword to use.</param>
171196
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
172197
[Theory]
173-
[InlineData("public", "class")]
174-
[InlineData("protected", "class")]
175-
[InlineData("internal", "class")]
176-
[InlineData("protected internal", "class")]
177-
[InlineData("private", "class")]
178-
[InlineData("public", "struct")]
179-
[InlineData("protected", "struct")]
180-
[InlineData("internal", "struct")]
181-
[InlineData("protected internal", "struct")]
182-
[InlineData("private", "struct")]
198+
[MemberData(nameof(ValidNestedDeclarations))]
183199
public async Task TestNestedTypeAccessModifiersAsync(string accessModifier, string typeKeyword)
184200
{
185201
var testCode = $@"
186202
internal static partial class TestPartial
187203
{{
188204
{accessModifier} partial {typeKeyword} PartialInner
189205
{{
190-
public int Do()
191-
{{
192-
return 2;
193-
}}
194206
}}
195207
}}
196208
";
197209

198210
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
199211
}
200212

213+
/// <summary>
214+
/// Verifies that a nested type without access modifiers will produce a diagnostic and can be fixed correctly.
215+
/// </summary>
216+
/// <param name="declaration">The declaration to verify.</param>
217+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
218+
[Theory]
219+
[MemberData(nameof(InvalidDeclarations))]
220+
public async Task TestNestedTypeWithoutAccessModifierAsync(string declaration)
221+
{
222+
var testCode = $@"
223+
public class Foo
224+
{{
225+
{declaration} Bar
226+
{{
227+
}}
228+
}}
229+
";
230+
231+
var fixedTestCode = $@"
232+
public class Foo
233+
{{
234+
private {declaration} Bar
235+
{{
236+
}}
237+
}}
238+
";
239+
240+
await this.VerifyCSharpDiagnosticAsync(testCode, this.CSharpDiagnostic().WithLocation(4, 6 + declaration.Length), CancellationToken.None).ConfigureAwait(false);
241+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
242+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
243+
}
244+
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+
201286
/// <inheritdoc/>
202287
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
203288
{

‎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)