Skip to content

Commit bceaf44

Browse files
sharwellsharwell
committed
Merge pull request #1486 from vweijsters/fix-fileheader-issues
2 parents a09e1b2 + bfadd4e commit bceaf44

5 files changed

Lines changed: 274 additions & 37 deletions

File tree

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@
44
namespace StyleCop.Analyzers.Test.DocumentationRules
55
{
66
using System.Collections.Generic;
7-
using System.Linq;
87
using System.Threading;
98
using System.Threading.Tasks;
109
using Analyzers.DocumentationRules;
11-
using Microsoft.CodeAnalysis;
1210
using Microsoft.CodeAnalysis.Diagnostics;
1311
using TestHelper;
1412
using Xunit;

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,41 @@ public async Task TestValidFileHeaderNoContentAsync()
4747
}
4848

4949
/// <summary>
50-
/// Verifies that a valid file header with leading directives will not produce a diagnostic message.
50+
/// Verifies that a file with a valid header and no other content will not produce a diagnostic message.
51+
/// </summary>
52+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
53+
[Fact]
54+
public async Task TestValidMultilineCommentFileHeadersAsync()
55+
{
56+
var testCode1 = @"/* <copyright file=""Test0.cs"" company=""FooCorp"">
57+
Copyright (c) FooCorp. All rights reserved.
58+
</copyright> */
59+
";
60+
61+
var testCode2 = @"/*
62+
<copyright file=""Test1.cs"" company=""FooCorp"">
63+
Copyright (c) FooCorp. All rights reserved.
64+
</copyright>
65+
*/
66+
";
67+
68+
var testCode3 = @"/*<copyright file=""Test2.cs"" company=""FooCorp"">
69+
Copyright (c) FooCorp. All rights reserved.
70+
</copyright>*/
71+
";
72+
73+
var testCode4 = @"/*
74+
* <copyright file=""Test3.cs"" company=""FooCorp"">
75+
* Copyright (c) FooCorp. All rights reserved.
76+
* </copyright>
77+
*/
78+
";
79+
80+
await this.VerifyCSharpDiagnosticAsync(new[] { testCode1, testCode2, testCode3, testCode4 }, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
81+
}
82+
83+
/// <summary>
84+
/// Verifies that a valid file header with leading directives will produce the correct diagnostic message.
5185
/// </summary>
5286
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
5387
[Fact]
@@ -66,7 +100,8 @@ namespace Bar
66100
}
67101
";
68102

69-
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
103+
var expected = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1633DescriptorMissing).WithLocation(1, 1);
104+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
70105
}
71106

72107
/// <summary>

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

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,32 @@ namespace StyleCop.Analyzers.Test.DocumentationRules
1414
/// </summary>
1515
public class SA1636UnitTests : FileHeaderTestBase
1616
{
17+
private const string MultiLineHeaderTestSettings = @"
18+
{
19+
""settings"": {
20+
""documentationRules"": {
21+
""companyName"": ""FooCorp"",
22+
""copyrightText"": ""copyright (c) {companyName}. All rights reserved.\n\nLine #3""
23+
}
24+
}
25+
}
26+
";
27+
28+
private const string NoXmlMultiLineHeaderTestSettings = @"
29+
{
30+
""settings"": {
31+
""documentationRules"": {
32+
""companyName"": ""FooCorp"",
33+
""copyrightText"": ""copyright (c) {companyName}. All rights reserved.\n\nLine #3"",
34+
""xmlHeader"": false
35+
}
36+
}
37+
}
38+
";
39+
40+
private bool useMultiLineHeaderTestSettings;
41+
private bool useNoXmlMultiLineHeaderTestSettings;
42+
1743
/// <summary>
1844
/// Verifies that a file header with a copyright message that is different than in the settings will produce the expected diagnostic message.
1945
/// </summary>
@@ -74,9 +100,118 @@ namespace Bar
74100
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
75101
}
76102

103+
/// <summary>
104+
/// Verifies that a file header will ignore spurious leading / trailing whitespaces (for multiple line comments)
105+
/// This is a regression for #1356
106+
/// </summary>
107+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
108+
[Fact]
109+
public async Task TestFileHeaderWillIgnoreLeadingAndTrailingWhitespaceAroundCopyrightMessageAsync()
110+
{
111+
this.useMultiLineHeaderTestSettings = true;
112+
113+
var testCode1 = @"// <copyright file=""Test0.cs"" company=""FooCorp"">
114+
// copyright (c) FooCorp. All rights reserved.
115+
//
116+
// Line #3
117+
// </copyright>
118+
119+
namespace Bar
120+
{
121+
}
122+
";
123+
124+
var testCode2 = @"/* <copyright file=""Test1.cs"" company=""FooCorp"">
125+
copyright (c) FooCorp. All rights reserved.
126+
127+
Line #3
128+
</copyright> */
129+
130+
namespace Bar
131+
{
132+
}
133+
";
134+
135+
var testCode3 = @"/*
136+
* <copyright file=""Test2.cs"" company=""FooCorp"">
137+
* copyright (c) FooCorp. All rights reserved.
138+
*
139+
* Line #3
140+
* </copyright>
141+
*/
142+
143+
namespace Bar
144+
{
145+
}
146+
";
147+
148+
await this.VerifyCSharpDiagnosticAsync(new[] { testCode1, testCode2, testCode3 }, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
149+
}
150+
151+
/// <summary>
152+
/// Verifies that a file header without XML header will ignore spurious leading / trailing whitespaces (for multiple line comments)
153+
/// This is a regression for #1356
154+
/// </summary>
155+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
156+
[Fact]
157+
public async Task TestNoXmlFileHeaderWillIgnoreLeadingAndTrailingWhitespaceAroundCopyrightMessageAsync()
158+
{
159+
this.useNoXmlMultiLineHeaderTestSettings = true;
160+
161+
var testCode1 = @"// copyright (c) FooCorp. All rights reserved.
162+
//
163+
// Line #3
164+
165+
namespace Bar
166+
{
167+
}
168+
";
169+
170+
var testCode2 = @"/*
171+
* copyright (c) FooCorp. All rights reserved.
172+
*
173+
* Line #3
174+
*/
175+
176+
namespace Bar
177+
{
178+
}
179+
";
180+
181+
var testCode3 = @"/*
182+
copyright (c) FooCorp. All rights reserved.
183+
184+
Line #3
185+
*/
186+
187+
namespace Bar
188+
{
189+
}
190+
";
191+
192+
await this.VerifyCSharpDiagnosticAsync(new[] { testCode1, testCode2, testCode3 }, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
193+
}
194+
195+
/// <inheritdoc/>
77196
protected override CodeFixProvider GetCSharpCodeFixProvider()
78197
{
79198
return new FileHeaderCodeFixProvider();
80199
}
200+
201+
/// <inheritdoc/>
202+
protected override string GetSettings()
203+
{
204+
if (this.useMultiLineHeaderTestSettings)
205+
{
206+
return MultiLineHeaderTestSettings;
207+
}
208+
209+
if (this.useNoXmlMultiLineHeaderTestSettings)
210+
{
211+
return NoXmlMultiLineHeaderTestSettings;
212+
}
213+
214+
return base.GetSettings();
215+
}
81216
}
82217
}

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/FileHeaderAnalyzers.cs‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,10 @@ private static void HandleSyntaxTreeAxtion(SyntaxTreeAnalysisContext context, Co
242242
return;
243243
}
244244

245-
// make sure that both \n and \r\n are accepted from the settings.
246-
var reformattedCopyrightText = settings.DocumentationRules.CopyrightText.Replace("\r\n", "\n").Replace("\n", Environment.NewLine);
247-
if (string.CompareOrdinal(fileHeader.CopyrightText, reformattedCopyrightText) != 0)
245+
if (!CompareCopyrightText(fileHeader.CopyrightText, settings))
248246
{
249247
context.ReportDiagnostic(Diagnostic.Create(SA1636Descriptor, fileHeader.GetLocation(context.Tree)));
248+
return;
250249
}
251250
}
252251
}
@@ -321,7 +320,8 @@ private static void CheckCopyrightText(SyntaxTreeAnalysisContext context, Compil
321320
return;
322321
}
323322

324-
if (string.CompareOrdinal(copyrightText.Trim(' ', '\r', '\n'), settings.DocumentationRules.CopyrightText) != 0)
323+
// trim any leading / trailing new line or whitespace characters (those are a result of the XML formatting)
324+
if (!CompareCopyrightText(copyrightText.Trim('\r', '\n', ' ', '\t'), settings))
325325
{
326326
var location = fileHeader.GetElementLocation(context.Tree, copyrightElement);
327327
context.ReportDiagnostic(Diagnostic.Create(SA1636Descriptor, location));
@@ -371,5 +371,28 @@ private static void CheckSummaryHeader(SyntaxTreeAnalysisContext context, Compil
371371
context.ReportDiagnostic(Diagnostic.Create(SA1639Descriptor, location));
372372
}
373373
}
374+
375+
private static bool CompareCopyrightText(string copyrightText, StyleCopSettings settings)
376+
{
377+
// make sure that both \n and \r\n are accepted from the settings.
378+
var reformattedCopyrightTextParts = settings.DocumentationRules.CopyrightText.Replace("\r\n", "\n").Split('\n');
379+
var fileHeaderCopyrightTextParts = copyrightText.Replace("\r\n", "\n").Split('\n');
380+
381+
if (reformattedCopyrightTextParts.Length != fileHeaderCopyrightTextParts.Length)
382+
{
383+
return false;
384+
}
385+
386+
// compare line by line, ignoring leading and trailing whitespace on each line.
387+
for (var i = 0; i < reformattedCopyrightTextParts.Length; i++)
388+
{
389+
if (string.CompareOrdinal(reformattedCopyrightTextParts[i].Trim(), fileHeaderCopyrightTextParts[i].Trim()) != 0)
390+
{
391+
return false;
392+
}
393+
}
394+
395+
return true;
396+
}
374397
}
375398
}

0 commit comments

Comments
 (0)