Skip to content

Commit e41ae68

Browse files
sharwellsharwell
committed
Merge pull request #1517 from pdelvo/SA1642CFImprov
2 parents dac9f93 + 2febf0b commit e41ae68

3 files changed

Lines changed: 332 additions & 18 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1642UnitTests.cs‎

Lines changed: 189 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,102 @@ public async Task TestStaticConstructorMissingDocumentationGenericAsync(string t
219219
await this.TestConstructorMissingDocumentationAsync(typeKind, "static", StaticConstructorStandardText, $" {typeKind}.", true).ConfigureAwait(false);
220220
}
221221

222+
[Theory]
223+
[InlineData("class")]
224+
[InlineData("struct")]
225+
public async Task TestNonPrivateConstructorSimpleDocumentationAsync(string typeKind)
226+
{
227+
await this.TestConstructorSimpleDocumentationAsync(typeKind, "public", NonPrivateConstructorStandardText, $" {typeKind}", false).ConfigureAwait(false);
228+
}
229+
230+
[Theory]
231+
[InlineData("class")]
232+
[InlineData("struct")]
233+
public async Task TestNonPrivateConstructorSimpleDocumentationGenericAsync(string typeKind)
234+
{
235+
await this.TestConstructorSimpleDocumentationAsync(typeKind, "public", NonPrivateConstructorStandardText, $" {typeKind}", true).ConfigureAwait(false);
236+
}
237+
238+
[Theory]
239+
[InlineData("class")]
240+
[InlineData("struct")]
241+
public async Task TestPrivateConstructorSimpleDocumentationAsync(string typeKind)
242+
{
243+
await this.TestConstructorSimpleDocumentationAsync(typeKind, "private", NonPrivateConstructorStandardText, $" {typeKind}", false).ConfigureAwait(false);
244+
}
245+
246+
[Theory]
247+
[InlineData("class")]
248+
[InlineData("struct")]
249+
public async Task TestPrivateConstructorSimpleDocumentationGenericAsync(string typeKind)
250+
{
251+
await this.TestConstructorSimpleDocumentationAsync(typeKind, "private", NonPrivateConstructorStandardText, $" {typeKind}", true).ConfigureAwait(false);
252+
}
253+
254+
[Theory]
255+
[InlineData("class")]
256+
[InlineData("struct")]
257+
public async Task TestStaticConstructorSimpleDocumentationAsync(string typeKind)
258+
{
259+
await this.TestConstructorSimpleDocumentationAsync(typeKind, "static", StaticConstructorStandardText, $" {typeKind}.", false).ConfigureAwait(false);
260+
}
261+
262+
[Theory]
263+
[InlineData("class")]
264+
[InlineData("struct")]
265+
public async Task TestStaticConstructorSimpleDocumentationGenericAsync(string typeKind)
266+
{
267+
await this.TestConstructorSimpleDocumentationAsync(typeKind, "static", StaticConstructorStandardText, $" {typeKind}.", true).ConfigureAwait(false);
268+
}
269+
270+
[Theory]
271+
[InlineData("class")]
272+
[InlineData("struct")]
273+
public async Task TestNonPrivateConstructorSimpleDocumentationWrongTypeNameAsync(string typeKind)
274+
{
275+
await this.TestConstructorSimpleDocumentationWrongTypeNameAsync(typeKind, "public", NonPrivateConstructorStandardText, $" {typeKind}", false).ConfigureAwait(false);
276+
}
277+
278+
[Theory]
279+
[InlineData("class")]
280+
[InlineData("struct")]
281+
public async Task TestNonPrivateConstructorSimpleDocumentationGenericWrongTypeNameAsync(string typeKind)
282+
{
283+
await this.TestConstructorSimpleDocumentationWrongTypeNameAsync(typeKind, "public", NonPrivateConstructorStandardText, $" {typeKind}", true).ConfigureAwait(false);
284+
}
285+
286+
[Theory]
287+
[InlineData("class")]
288+
[InlineData("struct")]
289+
public async Task TestPrivateConstructorSimpleDocumentationWrongTypeNameAsync(string typeKind)
290+
{
291+
await this.TestConstructorSimpleDocumentationWrongTypeNameAsync(typeKind, "private", NonPrivateConstructorStandardText, $" {typeKind}", false).ConfigureAwait(false);
292+
}
293+
294+
[Theory]
295+
[InlineData("class")]
296+
[InlineData("struct")]
297+
public async Task TestPrivateConstructorSimpleDocumentationGenericWrongTypeNameAsync(string typeKind)
298+
{
299+
await this.TestConstructorSimpleDocumentationWrongTypeNameAsync(typeKind, "private", NonPrivateConstructorStandardText, $" {typeKind}", true).ConfigureAwait(false);
300+
}
301+
302+
[Theory]
303+
[InlineData("class")]
304+
[InlineData("struct")]
305+
public async Task TestStaticConstructorSimpleDocumentationWrongTypeNameAsync(string typeKind)
306+
{
307+
await this.TestConstructorSimpleDocumentationWrongTypeNameAsync(typeKind, "static", StaticConstructorStandardText, $" {typeKind}.", false).ConfigureAwait(false);
308+
}
309+
310+
[Theory]
311+
[InlineData("class")]
312+
[InlineData("struct")]
313+
public async Task TestStaticConstructorSimpleDocumentationGenericWrongTypeNameAsync(string typeKind)
314+
{
315+
await this.TestConstructorSimpleDocumentationWrongTypeNameAsync(typeKind, "static", StaticConstructorStandardText, $" {typeKind}.", true).ConfigureAwait(false);
316+
}
317+
222318
/// <summary>
223319
/// This is a regression test for DotNetAnalyzers/StyleCopAnalyzers#676 "SA1642 misfires on nested structs,
224320
/// requiring text describing the outer type"
@@ -315,15 +411,14 @@ internal abstract class CustomizableBlockSubscriberBase<TSource, TTarget, TSubsc
315411
{
316412
/// <summary>
317413
/// Initializes a new instance of the <see cref=""CustomizableBlockSubscriberBase{TSource, TTarget, TSubscribedElement}""/> class.
318-
/// Initializes a new instance of the <see cref=""ProjectBuildSnapshotService""/> class.
319414
/// </summary>
320415
protected CustomizableBlockSubscriberBase()
321416
{
322417
}
323418
}
324419
";
325420

326-
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(6, 9);
421+
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(7, 43);
327422
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
328423
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
329424
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
@@ -470,5 +565,97 @@ await this.VerifyCSharpDiagnosticAsync(
470565
fixedCode = string.Format(fixedCode, typeKind, generic ? "<T1, T2>" : string.Empty, generic ? "{T1, T2}" : string.Empty, part1, part2, part3, modifiers, arguments);
471566
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
472567
}
568+
569+
private async Task TestConstructorSimpleDocumentationAsync(string typeKind, string modifiers, string part1, string part2, bool generic)
570+
{
571+
string part3 = part2.EndsWith(".") ? string.Empty : ".";
572+
573+
var testCode = @"namespace FooNamespace
574+
{{
575+
public {0} Foo{1}
576+
{{
577+
/// <summary>
578+
/// {3}Foo{4}{5}
579+
/// </summary>
580+
{6}
581+
Foo({7})
582+
{{
583+
584+
}}
585+
}}
586+
}}";
587+
string arguments = typeKind == "struct" && modifiers != "static" ? "int argument" : null;
588+
testCode = string.Format(testCode, typeKind, generic ? "<T1, T2>" : string.Empty, generic ? "{T1, T2}" : string.Empty, part1, part2, part3, modifiers, arguments);
589+
590+
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(5, 13);
591+
592+
await this.VerifyCSharpDiagnosticAsync(
593+
testCode,
594+
expected,
595+
CancellationToken.None).ConfigureAwait(false);
596+
597+
var fixedCode = @"namespace FooNamespace
598+
{{
599+
public {0} Foo{1}
600+
{{
601+
/// <summary>
602+
/// {3}<see cref=""Foo{2}""/>{4}{5}
603+
/// </summary>
604+
{6}
605+
Foo({7})
606+
{{
607+
608+
}}
609+
}}
610+
}}";
611+
fixedCode = string.Format(fixedCode, typeKind, generic ? "<T1, T2>" : string.Empty, generic ? "{T1, T2}" : string.Empty, part1, part2, part3, modifiers, arguments);
612+
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
613+
}
614+
615+
private async Task TestConstructorSimpleDocumentationWrongTypeNameAsync(string typeKind, string modifiers, string part1, string part2, bool generic)
616+
{
617+
string part3 = part2.EndsWith(".") ? string.Empty : ".";
618+
619+
var testCode = @"namespace FooNamespace
620+
{{
621+
public {0} Foo{1}
622+
{{
623+
/// <summary>
624+
/// {3}Bar{4}{5}
625+
/// </summary>
626+
{6}
627+
Foo({7})
628+
{{
629+
630+
}}
631+
}}
632+
}}";
633+
string arguments = typeKind == "struct" && modifiers != "static" ? "int argument" : null;
634+
testCode = string.Format(testCode, typeKind, generic ? "<T1, T2>" : string.Empty, generic ? "{T1, T2}" : string.Empty, part1, part2, part3, modifiers, arguments);
635+
636+
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(5, 13);
637+
638+
await this.VerifyCSharpDiagnosticAsync(
639+
testCode,
640+
expected,
641+
CancellationToken.None).ConfigureAwait(false);
642+
643+
var fixedCode = @"namespace FooNamespace
644+
{{
645+
public {0} Foo{1}
646+
{{
647+
/// <summary>
648+
/// {3}<see cref=""Foo{2}""/>{4}{5}
649+
/// </summary>
650+
{6}
651+
Foo({7})
652+
{{
653+
654+
}}
655+
}}
656+
}}";
657+
fixedCode = string.Format(fixedCode, typeKind, generic ? "<T1, T2>" : string.Empty, generic ? "{T1, T2}" : string.Empty, part1, part2, part3, modifiers, arguments);
658+
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
659+
}
473660
}
474661
}

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1642SA1643CodeFixProvider.cs‎

Lines changed: 123 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ namespace StyleCop.Analyzers.DocumentationRules
77
using System.Collections.Immutable;
88
using System.Composition;
99
using System.Linq;
10+
using System.Text.RegularExpressions;
1011
using System.Threading.Tasks;
1112
using Helpers;
1213
using Microsoft.CodeAnalysis;
@@ -53,13 +54,19 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
5354
continue;
5455
}
5556

56-
var node = root.FindNode(diagnostic.Location.SourceSpan, findInsideTrivia: true) as XmlElementSyntax;
57-
if (node == null)
57+
var node = root.FindNode(diagnostic.Location.SourceSpan, findInsideTrivia: true, getInnermostNodeForTie: true);
58+
59+
var xmlElementSyntax = node as XmlElementSyntax;
60+
61+
if (xmlElementSyntax != null)
5862
{
59-
continue;
63+
context.RegisterCodeFix(CodeAction.Create(DocumentationResources.SA1642SA1643CodeFix, token => GetTransformedDocumentAsync(context.Document, root, xmlElementSyntax), equivalenceKey: nameof(SA1642SA1643CodeFixProvider)), diagnostic);
64+
}
65+
else
66+
{
67+
var xmlEmptyElementSyntax = (XmlEmptyElementSyntax)node;
68+
context.RegisterCodeFix(CodeAction.Create(DocumentationResources.SA1642SA1643CodeFix, token => GetTransformedDocumentAsync(context.Document, root, xmlEmptyElementSyntax), equivalenceKey: nameof(SA1642SA1643CodeFixProvider)), diagnostic);
6069
}
61-
62-
context.RegisterCodeFix(CodeAction.Create(DocumentationResources.SA1642SA1643CodeFix, token => GetTransformedDocumentAsync(context.Document, root, node), equivalenceKey: nameof(SA1642SA1643CodeFixProvider)), diagnostic);
6370
}
6471
}
6572

@@ -119,9 +126,13 @@ private static Task<Document> GetTransformedDocumentAsync(Document document, Syn
119126
}
120127

121128
string newLineText = document.Project.Solution.Workspace.Options.GetOption(FormattingOptions.NewLine, LanguageNames.CSharp);
122-
var list = BuildStandardText(typeDeclaration.Identifier, typeParameterList, newLineText, standardText[0], standardText[1]);
123129

124-
var newContent = node.Content.InsertRange(0, list);
130+
string trailingString = string.Empty;
131+
132+
var newContent = RemoveMalformattedStandardText(node.Content, typeDeclaration.Identifier, standardText[0], standardText[1], ref trailingString);
133+
134+
var list = BuildStandardText(typeDeclaration.Identifier, typeParameterList, newLineText, standardText[0], standardText[1] + trailingString);
135+
newContent = newContent.InsertRange(0, list);
125136
var newNode = node.WithContent(newContent).AdjustDocumentationCommentNewLineTrivia();
126137

127138
var newRoot = root.ReplaceNode(node, newNode);
@@ -131,7 +142,111 @@ private static Task<Document> GetTransformedDocumentAsync(Document document, Syn
131142
return Task.FromResult(newDocument);
132143
}
133144

145+
private static Task<Document> GetTransformedDocumentAsync(Document document, SyntaxNode root, XmlEmptyElementSyntax node)
146+
{
147+
var typeDeclaration = node.FirstAncestorOrSelf<BaseTypeDeclarationSyntax>();
148+
var declarationSyntax = node.FirstAncestorOrSelf<BaseMethodDeclarationSyntax>();
149+
bool isStruct = typeDeclaration.IsKind(SyntaxKind.StructDeclaration);
150+
151+
TypeParameterListSyntax typeParameterList;
152+
ClassDeclarationSyntax classDeclaration = typeDeclaration as ClassDeclarationSyntax;
153+
if (classDeclaration != null)
154+
{
155+
typeParameterList = classDeclaration.TypeParameterList;
156+
}
157+
else
158+
{
159+
typeParameterList = (typeDeclaration as StructDeclarationSyntax)?.TypeParameterList;
160+
}
161+
162+
var newRoot = root.ReplaceNode(node, BuildSeeElement(typeDeclaration.Identifier, typeParameterList));
163+
164+
var newDocument = document.WithSyntaxRoot(newRoot);
165+
166+
return Task.FromResult(newDocument);
167+
}
168+
169+
private static SyntaxList<XmlNodeSyntax> RemoveMalformattedStandardText(SyntaxList<XmlNodeSyntax> content, SyntaxToken identifier, string preText, string postText, ref string trailingString)
170+
{
171+
var regex = new Regex(@"^\s*" + Regex.Escape(preText) + "[^ ]+" + Regex.Escape(postText));
172+
var item = content.OfType<XmlTextSyntax>().FirstOrDefault();
173+
174+
if (item == null)
175+
{
176+
return content;
177+
}
178+
179+
int index = -1;
180+
foreach (var token in item.TextTokens)
181+
{
182+
index++;
183+
184+
if (token.IsKind(SyntaxKind.XmlTextLiteralNewLineToken))
185+
{
186+
continue;
187+
}
188+
else if (token.IsKind(SyntaxKind.XmlTextLiteralToken))
189+
{
190+
string value = token.ValueText.Trim(null);
191+
192+
Match match = regex.Match(value);
193+
194+
if (!match.Success)
195+
{
196+
return content;
197+
}
198+
else if (match.Length == value.Length)
199+
{
200+
// Remove the token
201+
var tokens = item.TextTokens;
202+
203+
while (index >= 0)
204+
{
205+
tokens = tokens.RemoveAt(0);
206+
index--;
207+
}
208+
209+
var newContent = item.WithTextTokens(tokens);
210+
211+
return content.Replace(item, newContent);
212+
}
213+
else
214+
{
215+
// Remove the tokens before
216+
var tokens = item.TextTokens;
217+
218+
while (index >= 0)
219+
{
220+
tokens = tokens.RemoveAt(0);
221+
index--;
222+
}
223+
224+
trailingString = value.Substring(match.Length);
225+
226+
var newContent = item.WithTextTokens(tokens);
227+
228+
return content.Replace(item, newContent);
229+
}
230+
}
231+
else
232+
{
233+
return content;
234+
}
235+
}
236+
237+
return content;
238+
}
239+
134240
private static SyntaxList<XmlNodeSyntax> BuildStandardText(SyntaxToken identifier, TypeParameterListSyntax typeParameters, string newLineText, string preText, string postText)
241+
{
242+
return XmlSyntaxFactory.List(
243+
XmlSyntaxFactory.NewLine(newLineText),
244+
XmlSyntaxFactory.Text(preText),
245+
BuildSeeElement(identifier, typeParameters),
246+
XmlSyntaxFactory.Text(postText.EndsWith(".") ? postText : (postText + ".")));
247+
}
248+
249+
private static XmlEmptyElementSyntax BuildSeeElement(SyntaxToken identifier, TypeParameterListSyntax typeParameters)
135250
{
136251
TypeSyntax identifierName;
137252

@@ -145,11 +260,7 @@ private static SyntaxList<XmlNodeSyntax> BuildStandardText(SyntaxToken identifie
145260
identifierName = SyntaxFactory.GenericName(identifier.WithoutTrivia(), ParameterToArgumentListSyntax(typeParameters));
146261
}
147262

148-
return XmlSyntaxFactory.List(
149-
XmlSyntaxFactory.NewLine(newLineText),
150-
XmlSyntaxFactory.Text(preText),
151-
XmlSyntaxFactory.SeeElement(SyntaxFactory.TypeCref(identifierName)),
152-
XmlSyntaxFactory.Text(postText.EndsWith(".") ? postText : (postText + ".")));
263+
return XmlSyntaxFactory.SeeElement(SyntaxFactory.TypeCref(identifierName));
153264
}
154265

155266
private static TypeArgumentListSyntax ParameterToArgumentListSyntax(TypeParameterListSyntax typeParameters)

0 commit comments

Comments
 (0)