Skip to content

Commit 988ef21

Browse files
sharwellsharwell
committed
Merge pull request #1440 from sharwell/fix-1437
Fix handling of line endings in FileHeaderCodeFixProvider + SA1635 code fix
2 parents b946558 + a53719a commit 988ef21

3 files changed

Lines changed: 95 additions & 31 deletions

File tree

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

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ namespace StyleCop.Analyzers.Test.DocumentationRules
77
using System.Threading;
88
using System.Threading.Tasks;
99
using Analyzers.DocumentationRules;
10-
using Microsoft.CodeAnalysis;
10+
using Microsoft.CodeAnalysis.CodeFixes;
1111
using Microsoft.CodeAnalysis.Diagnostics;
1212
using TestHelper;
1313
using Xunit;
1414

1515
/// <summary>
1616
/// Unit tests for file header that do not follow the XML syntax.
1717
/// </summary>
18-
public class NoXmlFileHeaderUnitTests : DiagnosticVerifier
18+
public class NoXmlFileHeaderUnitTests : CodeFixVerifier
1919
{
2020
private const string SettingsFileName = "stylecop.json";
2121
private const string TestSettings = @"
@@ -45,10 +45,19 @@ public virtual async Task TestNoFileHeaderAsync()
4545
var testCode = @"namespace Foo
4646
{
4747
}
48+
";
49+
var fixedCode = @"// Copyright (c) FooCorp. All rights reserved.
50+
// Licensed under the ??? license. See LICENSE file in the project root for full license information.
51+
52+
namespace Foo
53+
{
54+
}
4855
";
4956

5057
var expectedDiagnostic = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1633DescriptorMissing).WithLocation(1, 1);
5158
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
59+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
60+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
5261
}
5362

5463
/// <summary>
@@ -87,13 +96,13 @@ namespace Bar
8796
}
8897

8998
/// <summary>
90-
/// Verifies that a valid file header built using multi line comments will not produce a diagnostic message.
99+
/// Verifies that a valid file header built using multi-line comments will not produce a diagnostic message.
91100
/// </summary>
92101
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
93102
[Fact]
94-
public async Task TestValidFileHeaderWithMultiLineCommentsAsync()
103+
public async Task TestValidFileHeaderWithMultiLineComments1Async()
95104
{
96-
var testCodeFormat1 = @"/* Copyright (c) FooCorp. All rights reserved.
105+
var testCode = @"/* Copyright (c) FooCorp. All rights reserved.
97106
* Licensed under the ??? license. See LICENSE file in the project root for full license information.
98107
*/
99108
@@ -102,42 +111,55 @@ namespace Bar
102111
}
103112
";
104113

105-
var testCodeFormat2 = @"/* Copyright (c) FooCorp. All rights reserved.
114+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
115+
}
116+
117+
/// <summary>
118+
/// Verifies that a valid file header built using multi-line comments will not produce a diagnostic message.
119+
/// </summary>
120+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
121+
[Fact]
122+
public async Task TestValidFileHeaderWithMultiLineComments2Async()
123+
{
124+
var testCode = @"/* Copyright (c) FooCorp. All rights reserved.
106125
Licensed under the ??? license. See LICENSE file in the project root for full license information. */
107126
108127
namespace Bar
109128
{
110129
}
111130
";
112131

113-
await this.VerifyCSharpDiagnosticAsync(testCodeFormat1, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
114-
await this.VerifyCSharpDiagnosticAsync(testCodeFormat2, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
132+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
115133
}
116134

117135
/// <summary>
118136
/// Verifies that a file header without text / only whitespace will produce the expected diagnostic message.
119137
/// </summary>
138+
/// <param name="comment">The comment text.</param>
120139
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
121-
[Fact]
122-
public async Task TestInvalidFileHeaderWithoutTextAsync()
140+
[Theory]
141+
[InlineData("//")]
142+
[InlineData("// ")]
143+
public async Task TestInvalidFileHeaderWithoutTextAsync(string comment)
123144
{
124-
var testCodeFormat1 = @"//
145+
var testCode = $@"{comment}
125146
126147
namespace Bar
127-
{
128-
}
148+
{{
149+
}}
129150
";
130-
131-
var testCodeFormat2 = "// " + @"
151+
var fixedCode = @"// Copyright (c) FooCorp. All rights reserved.
152+
// Licensed under the ??? license. See LICENSE file in the project root for full license information.
132153
133154
namespace Bar
134155
{
135156
}
136157
";
137158

138159
var expected = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1635Descriptor).WithLocation(1, 1);
139-
await this.VerifyCSharpDiagnosticAsync(testCodeFormat1, expected, CancellationToken.None).ConfigureAwait(false);
140-
await this.VerifyCSharpDiagnosticAsync(testCodeFormat2, expected, CancellationToken.None).ConfigureAwait(false);
160+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
161+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
162+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
141163
}
142164

143165
/// <summary>
@@ -150,12 +172,21 @@ public async Task TestInvalidFileHeaderWithWrongTextAsync()
150172
var testCode = @"// Copyright (c) BarCorp. All rights reserved.
151173
// Licensed under the ??? license. See LICENSE file in the project root for full license information.
152174
175+
namespace Bar
176+
{
177+
}
178+
";
179+
var fixedCode = @"// Copyright (c) FooCorp. All rights reserved.
180+
// Licensed under the ??? license. See LICENSE file in the project root for full license information.
181+
153182
namespace Bar
154183
{
155184
}
156185
";
157186
var expected = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1636Descriptor).WithLocation(1, 1);
158187
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
188+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
189+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
159190
}
160191

161192
/// <inheritdoc/>
@@ -169,5 +200,11 @@ protected sealed override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAna
169200
{
170201
yield return new FileHeaderAnalyzers();
171202
}
203+
204+
/// <inheritdoc/>
205+
protected override CodeFixProvider GetCSharpCodeFixProvider()
206+
{
207+
return new FileHeaderCodeFixProvider();
208+
}
172209
}
173210
}

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,23 @@ public async Task TestFileHeaderWithShorthandCopyrightAsync()
2323
{
2424
var testCode = @"// <copyright file=""Test0.cs"" company=""FooCorp""/>
2525
26+
namespace Bar
27+
{
28+
}
29+
";
30+
var fixedCode = @"// <copyright file=""Test0.cs"" company=""FooCorp"">
31+
// Copyright (c) FooCorp. All rights reserved.
32+
// </copyright>
33+
2634
namespace Bar
2735
{
2836
}
2937
";
3038

3139
var expectedDiagnostic = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1635Descriptor).WithLocation(1, 4);
3240
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
41+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
42+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
3343
}
3444

3545
/// <summary>
@@ -47,14 +57,25 @@ public async Task TestFileHeaderWithWhitespaceOnlyCopyrightAsync()
4757
"namespace Bar\r\n" +
4858
"{\r\n" +
4959
"}\r\n";
60+
string fixedCode =
61+
"// <copyright file=\"Test0.cs\" company=\"FooCorp\">\r\n" +
62+
"// Copyright (c) FooCorp. All rights reserved.\r\n" +
63+
"// </copyright>\r\n" +
64+
"\r\n" +
65+
"namespace Bar\r\n" +
66+
"{\r\n" +
67+
"}\r\n";
5068

5169
var expectedDiagnostic = this.CSharpDiagnostic(FileHeaderAnalyzers.SA1635Descriptor).WithLocation(1, 4);
5270
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
71+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
72+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
5373
}
5474

75+
/// <inheritdoc/>
5576
protected override CodeFixProvider GetCSharpCodeFixProvider()
5677
{
57-
throw new System.NotImplementedException();
78+
return new FileHeaderCodeFixProvider();
5879
}
5980
}
6081
}

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/FileHeaderCodeFixProvider.cs‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ namespace StyleCop.Analyzers.DocumentationRules
1212
using Microsoft.CodeAnalysis.CodeActions;
1313
using Microsoft.CodeAnalysis.CodeFixes;
1414
using Microsoft.CodeAnalysis.CSharp;
15+
using Microsoft.CodeAnalysis.Formatting;
1516
using StyleCop.Analyzers.Helpers;
1617
using StyleCop.Analyzers.Settings.ObjectModel;
1718

@@ -29,6 +30,7 @@ public class FileHeaderCodeFixProvider : CodeFixProvider
2930
public override ImmutableArray<string> FixableDiagnosticIds { get; }
3031
= ImmutableArray.Create(
3132
FileHeaderAnalyzers.SA1633DescriptorMissing.Id,
33+
FileHeaderAnalyzers.SA1635Descriptor.Id,
3234
FileHeaderAnalyzers.SA1636Descriptor.Id);
3335

3436
/// <inheritdoc/>
@@ -54,7 +56,7 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
5456
var settings = document.Project.AnalyzerOptions.GetStyleCopSettings();
5557

5658
var fileHeader = FileHeaderHelpers.ParseFileHeader(root);
57-
var newSyntaxRoot = fileHeader.IsMissing ? AddHeader(root, document.Name, settings) : ReplaceHeader(document, root, settings);
59+
var newSyntaxRoot = fileHeader.IsMissing ? AddHeader(document, root, document.Name, settings) : ReplaceHeader(document, root, settings);
5860

5961
return document.WithSyntaxRoot(newSyntaxRoot);
6062
}
@@ -104,36 +106,40 @@ private static SyntaxNode ReplaceHeader(Document document, SyntaxNode root, Styl
104106
}
105107
}
106108

107-
return root.WithLeadingTrivia(CreateNewHeader(document.Name, settings).Add(SyntaxFactory.CarriageReturnLineFeed).Add(SyntaxFactory.CarriageReturnLineFeed).AddRange(trivia));
109+
string newLineText = document.Project.Solution.Workspace.Options.GetOption(FormattingOptions.NewLine, LanguageNames.CSharp);
110+
return root.WithLeadingTrivia(CreateNewHeader(document.Name, settings, newLineText).Add(SyntaxFactory.CarriageReturnLineFeed).Add(SyntaxFactory.CarriageReturnLineFeed).AddRange(trivia));
108111
}
109112

110-
private static SyntaxNode AddHeader(SyntaxNode root, string name, StyleCopSettings settings)
113+
private static SyntaxNode AddHeader(Document document, SyntaxNode root, string name, StyleCopSettings settings)
111114
{
112-
var newTrivia = CreateNewHeader(name, settings).Add(SyntaxFactory.CarriageReturnLineFeed).Add(SyntaxFactory.CarriageReturnLineFeed);
115+
string newLineText = document.Project.Solution.Workspace.Options.GetOption(FormattingOptions.NewLine, LanguageNames.CSharp);
116+
var newTrivia = CreateNewHeader(name, settings, newLineText).Add(SyntaxFactory.CarriageReturnLineFeed).Add(SyntaxFactory.CarriageReturnLineFeed);
113117
newTrivia = newTrivia.AddRange(root.GetLeadingTrivia());
114118

115119
return root.WithLeadingTrivia(newTrivia);
116120
}
117121

118-
private static SyntaxTriviaList CreateNewHeader(string filename, StyleCopSettings settings)
122+
private static SyntaxTriviaList CreateNewHeader(string filename, StyleCopSettings settings, string newLineText)
119123
{
120-
var copyrightText = "// " + GetCopyrightText(settings.DocumentationRules.CopyrightText);
124+
var copyrightText = "// " + GetCopyrightText(settings.DocumentationRules.CopyrightText, newLineText);
121125
var newHeader = settings.DocumentationRules.XmlHeader
122-
? WrapInXmlComment(copyrightText, filename, settings)
126+
? WrapInXmlComment(copyrightText, filename, settings, newLineText)
123127
: copyrightText;
124128
return SyntaxFactory.ParseLeadingTrivia(newHeader);
125129
}
126130

127-
private static string WrapInXmlComment(string copyrightText, string filename, StyleCopSettings settings)
131+
private static string WrapInXmlComment(string copyrightText, string filename, StyleCopSettings settings, string newLineText)
128132
{
129-
return $@"// <copyright file=""{filename}"" company=""{settings.DocumentationRules.CompanyName}"">
130-
{copyrightText}
131-
// </copyright>";
133+
return
134+
$"// <copyright file=\"{filename}\" company=\"{settings.DocumentationRules.CompanyName}\">" + newLineText
135+
+ copyrightText + newLineText
136+
+ "// </copyright>";
132137
}
133138

134-
private static string GetCopyrightText(string copyrightText)
139+
private static string GetCopyrightText(string copyrightText, string newLineText)
135140
{
136-
return string.Join("\n// ", copyrightText.Split('\n'));
141+
copyrightText = copyrightText.Replace("\r\n", "\n");
142+
return string.Join(newLineText + "// ", copyrightText.Split('\n'));
137143
}
138144
}
139145
}

0 commit comments

Comments
 (0)